Fix deadlettering in Grpc service

Azure · Oct 20, 2023 · 6531013 · 6531013
1 parent 327f162
commit 6531013
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 13 deletions.
diff --git a/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/CHANGELOG.md b/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/CHANGELOG.md
@@ -1,14 +1,11 @@
 # Release History
 
-## 5.14.0-beta.1 (Unreleased)
-
-### Features Added
-
-### Breaking Changes
+## 5.13.3 (2023-10-20)
 
 ### Bugs Fixed
 
-### Other Changes
+- Fixed issue where deadlettering a message without specifying properties to modify could throw 
+  an exception from out of proc extension.
 
 ## 5.13.2 (2023-10-18)
 

diff --git a/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/src/Grpc/SettlementService.cs b/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/src/Grpc/SettlementService.cs
@@ -71,12 +71,23 @@ public override async Task<Empty> Deadletter(DeadletterRequest request, ServerCa
         {
             if (_provider.ActionsCache.TryGetValue(request.Locktoken, out var tuple))
             {
-                await tuple.Actions.DeadLetterMessageAsync(
-                    tuple.Message,
-                    DeserializeAmqpMap(request.PropertiesToModify),
-                    request.DeadletterReason,
-                    request.DeadletterErrorDescription,
-                    context.CancellationToken).ConfigureAwait(false);
+                if (request.PropertiesToModify == null || request.PropertiesToModify == ByteString.Empty)
+                {
+                    await tuple.Actions.DeadLetterMessageAsync(
+                        tuple.Message,
+                        request.DeadletterReason,
+                        request.DeadletterErrorDescription,
+                        context.CancellationToken).ConfigureAwait(false);
+                }
+                else
+                {
+                    await tuple.Actions.DeadLetterMessageAsync(
+                        tuple.Message,
+                        DeserializeAmqpMap(request.PropertiesToModify),
+                        request.DeadletterReason,
+                        request.DeadletterErrorDescription,
+                        context.CancellationToken).ConfigureAwait(false);
+                }
                 return new Empty();
             }
             throw new RpcException (new Status(StatusCode.FailedPrecondition, $"LockToken {request.Locktoken} not found."));

diff --git a/...re.WebJobs.Extensions.ServiceBus/src/Microsoft.Azure.WebJobs.Extensions.ServiceBus.csproj b/...re.WebJobs.Extensions.ServiceBus/src/Microsoft.Azure.WebJobs.Extensions.ServiceBus.csproj
@@ -3,7 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;net6.0</TargetFrameworks>
     <Description>Microsoft Azure WebJobs SDK ServiceBus Extension</Description>
-    <Version>5.14.0-beta.1</Version>
+    <Version>5.13.3</Version>
     <!--The ApiCompatVersion is managed automatically and should not generally be modified manually.-->
     <!--Since we are adding a new target for net6.0, we need to temporarily condition on netstandard-->
     <ApiCompatVersion>5.13.2</ApiCompatVersion>

diff --git a/...s/Microsoft.Azure.WebJobs.Extensions.ServiceBus/tests/Grpc/ServiceBusGrpcEndToEndTests.cs b/...s/Microsoft.Azure.WebJobs.Extensions.ServiceBus/tests/Grpc/ServiceBusGrpcEndToEndTests.cs
@@ -93,6 +93,28 @@ public async Task BindToMessageAndDeadletter()
             Assert.IsEmpty(provider.ActionsCache);
         }
 
+        [Test]
+        public async Task BindToMessageAndDeadletterWithNoPropertiesToModify()
+        {
+            var host = BuildHost<ServiceBusBindToMessageAndDeadletterWithNoPropertiesToModify>();
+            var settlementImpl = host.Services.GetRequiredService<SettlementService>();
+            var provider = host.Services.GetRequiredService<MessagingProvider>();
+            ServiceBusBindToMessageAndDeadletterWithNoPropertiesToModify.SettlementService = settlementImpl;
+
+            using (host)
+            {
+                var message = new ServiceBusMessage("foobar");
+                await using ServiceBusClient client = new ServiceBusClient(ServiceBusTestEnvironment.Instance.ServiceBusConnectionString);
+                var sender = client.CreateSender(FirstQueueScope.QueueName);
+                await sender.SendMessageAsync(message);
+
+                bool result = _waitHandle1.WaitOne(SBTimeoutMills);
+                Assert.True(result);
+                await host.StopAsync();
+            }
+            Assert.IsEmpty(provider.ActionsCache);
+        }
+
         [Test]
         public async Task BindToBatchAndDeadletter()
         {
@@ -261,6 +283,31 @@ await SettlementService.Deadletter(
             }
         }
 
+        public class ServiceBusBindToMessageAndDeadletterWithNoPropertiesToModify
+        {
+            internal static SettlementService SettlementService { get; set; }
+            public static async Task BindToMessage(
+                [ServiceBusTrigger(FirstQueueNameKey)] ServiceBusReceivedMessage message, ServiceBusClient client)
+            {
+                Assert.AreEqual("foobar", message.Body.ToString());
+                await SettlementService.Deadletter(
+                    new DeadletterRequest()
+                    {
+                        Locktoken = message.LockToken,
+                        DeadletterErrorDescription = "description",
+                        DeadletterReason = "reason"
+                    },
+                    new MockServerCallContext());
+
+                var receiver = client.CreateReceiver(FirstQueueScope.QueueName, new ServiceBusReceiverOptions {SubQueue = SubQueue.DeadLetter});
+                var deadletterMessage = await receiver.ReceiveMessageAsync();
+                Assert.AreEqual("foobar", deadletterMessage.Body.ToString());
+                Assert.AreEqual("description", deadletterMessage.DeadLetterErrorDescription);
+                Assert.AreEqual("reason", deadletterMessage.DeadLetterReason);
+                _waitHandle1.Set();
+            }
+        }
+
         public class ServiceBusBindToBatchAndDeadletter
         {
             internal static SettlementService SettlementService { get; set; }