From 6531013320606eea2ce09be406026a5f1ba23536 Mon Sep 17 00:00:00 2001 From: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com> Date: Thu, 19 Oct 2023 21:43:07 -0700 Subject: [PATCH] Fix deadlettering in Grpc service --- .../CHANGELOG.md | 9 ++-- .../src/Grpc/SettlementService.cs | 23 ++++++--- ...Azure.WebJobs.Extensions.ServiceBus.csproj | 2 +- .../tests/Grpc/ServiceBusGrpcEndToEndTests.cs | 47 +++++++++++++++++++ 4 files changed, 68 insertions(+), 13 deletions(-) diff --git a/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/CHANGELOG.md b/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/CHANGELOG.md index aa988c9dd74dd..b5f32997db4e2 100644 --- 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 index d8d490cbb90a4..32b591d811fbb 100644 --- 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 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/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/src/Microsoft.Azure.WebJobs.Extensions.ServiceBus.csproj b/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/src/Microsoft.Azure.WebJobs.Extensions.ServiceBus.csproj index 6c9cf28e61814..a58f8fdefb82d 100644 --- a/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/src/Microsoft.Azure.WebJobs.Extensions.ServiceBus.csproj +++ b/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/src/Microsoft.Azure.WebJobs.Extensions.ServiceBus.csproj @@ -3,7 +3,7 @@ netstandard2.0;net6.0 Microsoft Azure WebJobs SDK ServiceBus Extension - 5.14.0-beta.1 + 5.13.3 5.13.2 diff --git a/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/tests/Grpc/ServiceBusGrpcEndToEndTests.cs b/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/tests/Grpc/ServiceBusGrpcEndToEndTests.cs index 3b0b48e597d06..4fcfd9f5df2d5 100644 --- a/sdk/servicebus/Microsoft.Azure.WebJobs.Extensions.ServiceBus/tests/Grpc/ServiceBusGrpcEndToEndTests.cs +++ b/sdk/servicebus/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(); + var settlementImpl = host.Services.GetRequiredService(); + var provider = host.Services.GetRequiredService(); + 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; }