From f5631aafb0e3496f51a122c249a0cc4e09ffd423 Mon Sep 17 00:00:00 2001 From: John Simons Date: Tue, 25 Feb 2025 16:18:09 +1000 Subject: [PATCH 1/5] Remove header instead Fixes https://github.com/Particular/ServiceControl/issues/4837 --- src/ServiceControl/Recoverability/Editing/EditHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl/Recoverability/Editing/EditHandler.cs b/src/ServiceControl/Recoverability/Editing/EditHandler.cs index 2a49ada79b..eb263bd3be 100644 --- a/src/ServiceControl/Recoverability/Editing/EditHandler.cs +++ b/src/ServiceControl/Recoverability/Editing/EditHandler.cs @@ -67,7 +67,7 @@ public async Task Handle(EditAndSend message, IMessageHandlerContext context) var outgoingMessage = BuildMessage(message); // mark the new message with a link to the original message id outgoingMessage.Headers.Add("ServiceControl.EditOf", message.FailedMessageId); - outgoingMessage.Headers["ServiceControl.Retry.AcknowledgementQueue"] = ""; + outgoingMessage.Headers.Remove("ServiceControl.Retry.AcknowledgementQueue"); var address = ApplyRedirect(attempt.FailureDetails.AddressOfFailingEndpoint, redirects); if (outgoingMessage.Headers.TryGetValue("ServiceControl.RetryTo", out var retryTo)) From d9601b25f68ae70db3be4cd32e304cbf4d3f8842 Mon Sep 17 00:00:00 2001 From: John Simons Date: Wed, 26 Feb 2025 18:39:58 +1000 Subject: [PATCH 2/5] Adding tests --- .../HttpExtensions.cs | 2 +- .../WhenRetryingSameMessageMultipleTimes.cs | 155 ++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs diff --git a/src/ServiceControl.AcceptanceTesting/HttpExtensions.cs b/src/ServiceControl.AcceptanceTesting/HttpExtensions.cs index 64c6203a30..7e5ebde17f 100644 --- a/src/ServiceControl.AcceptanceTesting/HttpExtensions.cs +++ b/src/ServiceControl.AcceptanceTesting/HttpExtensions.cs @@ -47,7 +47,7 @@ public static async Task> TryGetMany(this IAcceptanceTestInfras return ManyResult.Empty; } - return ManyResult.New(true, response); + return ManyResult.New(true, response.Where(m => condition(m)).ToList()); } public static async Task Patch(this IAcceptanceTestInfrastructureProvider provider, string url, T payload = null) where T : class diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs new file mode 100644 index 0000000000..c7e7ab4b79 --- /dev/null +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs @@ -0,0 +1,155 @@ +namespace ServiceControl.MultiInstance.AcceptanceTests.Recoverability +{ + using System; + using System.Linq; + using System.Threading.Tasks; + using AcceptanceTesting; + using AcceptanceTesting.EndpointTemplates; + using MessageFailures; + using MessageFailures.Api; + using NServiceBus; + using NServiceBus.AcceptanceTesting; + using NServiceBus.Settings; + using NUnit.Framework; + using ServiceControl.Infrastructure; + using TestSupport; + + class WhenRetryingSameMessageMultipleTimes : AcceptanceTest + { + [Test] + public async Task WithNoEdit() + { + await Define() + .WithEndpoint(b => b.When(bus => bus.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) + .Done(async c => + { + if (c.RetryCount < 3) + { + var result = await GetFailedMessage(c, ServiceControlInstanceName, FailedMessageStatus.Unresolved); + if (result.HasResult) + { + if (result.Item.ProcessingAttempts.Count == c.RetryCount + 1) + { + await this.Post($"/api/errors/{result.Item.UniqueMessageId}/retry", null, null, + ServiceControlInstanceName); + c.RetryCount++; + } + } + + return false; + } + + return await GetFailedMessage(c, ServiceControlInstanceName, FailedMessageStatus.Resolved); + }) + .Run(TimeSpan.FromMinutes(2)); + } + + [Test] + public async Task WithEdit() + { + CustomServiceControlPrimarySettings = s => { s.AllowMessageEditing = true; }; + + await Define() + .WithEndpoint(b => + b.When(bus => bus.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) + .Done(async c => + { + if (c.RetryCount < 3) + { + var results = await GetAllFailedMessage(ServiceControlInstanceName, FailedMessageStatus.Unresolved); + if (!results.HasResult) + { + return false; + } + + var result = results.Items.Single(); + + c.MessageId = result.MessageId; + + var failedMessage = await GetFailedMessage(c, ServiceControlInstanceName, FailedMessageStatus.Unresolved); + if (!failedMessage.HasResult) + { + return false; + } + + await this.Post($"/api/edit/{failedMessage.Item.UniqueMessageId}", + new + { + MessageBody = $"{{ \"Name\": \"John{c.RetryCount}\" }}", + MessageHeaders = failedMessage.Item.ProcessingAttempts[^1].Headers + }, null, + ServiceControlInstanceName); + c.RetryCount++; + + return false; + } + + return !(await GetAllFailedMessage(ServiceControlInstanceName, FailedMessageStatus.Unresolved)).HasResult; + }) + .Run(TimeSpan.FromMinutes(2)); + } + + Task> GetFailedMessage(MyContext c, string instance, FailedMessageStatus expectedStatus) + { + if (c.MessageId == null) + { + return Task.FromResult(SingleResult.Empty); + } + + return this.TryGet("/api/errors/" + c.UniqueMessageId, f => f.Status == expectedStatus, instance); + } + + Task> GetAllFailedMessage(string instance, FailedMessageStatus expectedStatus) + { + return this.TryGetMany("/api/errors", f => f.Status == expectedStatus, instance); + } + + public class FailureEndpoint : EndpointConfigurationBuilder + { + public FailureEndpoint() => EndpointSetup(c => { c.NoRetries(); }); + + public class MyMessageHandler : IHandleMessages + { + readonly MyContext testContext; + readonly IReadOnlySettings settings; + + public MyMessageHandler(MyContext testContext, IReadOnlySettings settings) + { + this.testContext = testContext; + this.settings = settings; + } + + public Task Handle(MyMessage message, IMessageHandlerContext context) + { + testContext.MessageId = context.MessageId.Replace(@"\", "-"); + testContext.EndpointNameOfReceivingEndpoint = settings.EndpointName(); + Console.Out.WriteLine("Handling message"); + + if (testContext.RetryCount < 3) + { + Console.Out.WriteLine("Throwing exception for MyMessage"); + throw new Exception("Simulated exception"); + } + + return Task.CompletedTask; + } + } + } + + + public class MyMessage : ICommand + { + public string Name { get; set; } + } + + public class MyContext : ScenarioContext + { + public string MessageId { get; set; } + + public string EndpointNameOfReceivingEndpoint { get; set; } + + public string UniqueMessageId => DeterministicGuid.MakeId(MessageId, EndpointNameOfReceivingEndpoint).ToString(); + public int RetryCount { get; set; } + } + } +} \ No newline at end of file From c5d41ef0e0f2c001c1b4641969f002ef31fec740 Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 27 Feb 2025 10:42:25 +1000 Subject: [PATCH 3/5] More tests --- .../Recoverability/WhenRetrying.cs | 22 ++++ .../WhenRetryingSameMessageMultipleTimes.cs | 116 +++++++----------- .../Recoverability/WhenRetryingWithEdit.cs | 92 ++++++++++++++ 3 files changed, 158 insertions(+), 72 deletions(-) create mode 100644 src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetrying.cs create mode 100644 src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingWithEdit.cs diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetrying.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetrying.cs new file mode 100644 index 0000000000..f9d75e271f --- /dev/null +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetrying.cs @@ -0,0 +1,22 @@ +namespace ServiceControl.MultiInstance.AcceptanceTests.Recoverability; + +using System.Threading.Tasks; +using AcceptanceTesting; +using MessageFailures; +using MessageFailures.Api; +using TestSupport; + +abstract class WhenRetrying : AcceptanceTest +{ + protected Task> GetFailedMessage(string uniqueMessageId, string instance, FailedMessageStatus expectedStatus) + { + if (uniqueMessageId == null) + { + return Task.FromResult(SingleResult.Empty); + } + + return this.TryGet("/api/errors/" + uniqueMessageId, f => f.Status == expectedStatus, instance); + } + + protected Task> GetAllFailedMessage(string instance, FailedMessageStatus expectedStatus) => this.TryGetMany("/api/errors", f => f.Status == expectedStatus, instance); +} \ No newline at end of file diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs index c7e7ab4b79..6ea9668bf0 100644 --- a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs @@ -6,7 +6,6 @@ using AcceptanceTesting; using AcceptanceTesting.EndpointTemplates; using MessageFailures; - using MessageFailures.Api; using NServiceBus; using NServiceBus.AcceptanceTesting; using NServiceBus.Settings; @@ -14,38 +13,19 @@ using ServiceControl.Infrastructure; using TestSupport; - class WhenRetryingSameMessageMultipleTimes : AcceptanceTest + class WhenRetryingSameMessageMultipleTimes : WhenRetrying { - [Test] - public async Task WithNoEdit() + public enum RetryType { - await Define() - .WithEndpoint(b => b.When(bus => bus.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) - .Done(async c => - { - if (c.RetryCount < 3) - { - var result = await GetFailedMessage(c, ServiceControlInstanceName, FailedMessageStatus.Unresolved); - if (result.HasResult) - { - if (result.Item.ProcessingAttempts.Count == c.RetryCount + 1) - { - await this.Post($"/api/errors/{result.Item.UniqueMessageId}/retry", null, null, - ServiceControlInstanceName); - c.RetryCount++; - } - } - - return false; - } - - return await GetFailedMessage(c, ServiceControlInstanceName, FailedMessageStatus.Resolved); - }) - .Run(TimeSpan.FromMinutes(2)); + NoEdit, + Edit } - [Test] - public async Task WithEdit() + [TestCase(new[] { RetryType.NoEdit, RetryType.NoEdit, RetryType.Edit })] + [TestCase(new[] { RetryType.Edit, RetryType.NoEdit, RetryType.Edit })] + [TestCase(new[] { RetryType.NoEdit, RetryType.Edit, RetryType.NoEdit })] + [TestCase(new[] { RetryType.Edit, RetryType.Edit, RetryType.NoEdit })] + public async Task WithMixOfRetryTypes(RetryType[] retryTypes) { CustomServiceControlPrimarySettings = s => { s.AllowMessageEditing = true; }; @@ -54,9 +34,16 @@ await Define() b.When(bus => bus.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) .Done(async c => { - if (c.RetryCount < 3) + if (c.RetryCount >= retryTypes.Length) + { + return !(await GetAllFailedMessage(ServiceControlInstanceName, FailedMessageStatus.Unresolved)) + .HasResult; + } + + if (retryTypes[c.RetryCount] == RetryType.Edit) { - var results = await GetAllFailedMessage(ServiceControlInstanceName, FailedMessageStatus.Unresolved); + var results = await GetAllFailedMessage(ServiceControlInstanceName, + FailedMessageStatus.Unresolved); if (!results.HasResult) { return false; @@ -65,84 +52,69 @@ await Define() var result = results.Items.Single(); c.MessageId = result.MessageId; + } - var failedMessage = await GetFailedMessage(c, ServiceControlInstanceName, FailedMessageStatus.Unresolved); - if (!failedMessage.HasResult) - { - return false; - } + var failedMessage = await GetFailedMessage(c.UniqueMessageId, ServiceControlInstanceName, FailedMessageStatus.Unresolved); + if (!failedMessage.HasResult) + { + return false; + } + if (retryTypes[c.RetryCount] == RetryType.Edit) + { await this.Post($"/api/edit/{failedMessage.Item.UniqueMessageId}", new { - MessageBody = $"{{ \"Name\": \"John{c.RetryCount}\" }}", + MessageBody = $"{{ \"Name\": \"Hello {c.RetryCount}\" }}", MessageHeaders = failedMessage.Item.ProcessingAttempts[^1].Headers }, null, ServiceControlInstanceName); - c.RetryCount++; - - return false; + } + else + { + await this.Post($"/api/errors/{failedMessage.Item.UniqueMessageId}/retry", null, null, + ServiceControlInstanceName); } - return !(await GetAllFailedMessage(ServiceControlInstanceName, FailedMessageStatus.Unresolved)).HasResult; - }) - .Run(TimeSpan.FromMinutes(2)); - } - - Task> GetFailedMessage(MyContext c, string instance, FailedMessageStatus expectedStatus) - { - if (c.MessageId == null) - { - return Task.FromResult(SingleResult.Empty); - } + c.RetryCount++; - return this.TryGet("/api/errors/" + c.UniqueMessageId, f => f.Status == expectedStatus, instance); - } + return false; - Task> GetAllFailedMessage(string instance, FailedMessageStatus expectedStatus) - { - return this.TryGetMany("/api/errors", f => f.Status == expectedStatus, instance); + }) + .Run(TimeSpan.FromMinutes(2)); } - public class FailureEndpoint : EndpointConfigurationBuilder + class FailureEndpoint : EndpointConfigurationBuilder { public FailureEndpoint() => EndpointSetup(c => { c.NoRetries(); }); - public class MyMessageHandler : IHandleMessages + public class MyMessageHandler(MyContext testContext, IReadOnlySettings settings) + : IHandleMessages { - readonly MyContext testContext; - readonly IReadOnlySettings settings; - - public MyMessageHandler(MyContext testContext, IReadOnlySettings settings) - { - this.testContext = testContext; - this.settings = settings; - } - public Task Handle(MyMessage message, IMessageHandlerContext context) { testContext.MessageId = context.MessageId.Replace(@"\", "-"); testContext.EndpointNameOfReceivingEndpoint = settings.EndpointName(); - Console.Out.WriteLine("Handling message"); if (testContext.RetryCount < 3) { - Console.Out.WriteLine("Throwing exception for MyMessage"); + Console.Out.WriteLine("Throwing exception"); throw new Exception("Simulated exception"); } + Console.Out.WriteLine("Handling message"); + return Task.CompletedTask; } } } - - public class MyMessage : ICommand + class MyMessage : ICommand { public string Name { get; set; } } - public class MyContext : ScenarioContext + class MyContext : ScenarioContext { public string MessageId { get; set; } diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingWithEdit.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingWithEdit.cs new file mode 100644 index 0000000000..21aedbc47f --- /dev/null +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingWithEdit.cs @@ -0,0 +1,92 @@ +namespace ServiceControl.MultiInstance.AcceptanceTests.Recoverability; + +using System; +using System.Threading.Tasks; +using AcceptanceTesting; +using AcceptanceTesting.EndpointTemplates; +using MessageFailures; +using NServiceBus; +using NServiceBus.AcceptanceTesting; +using NServiceBus.Settings; +using NUnit.Framework; +using TestSupport; +using ServiceControl.Infrastructure; + +class WhenRetryingWithEdit : WhenRetrying +{ + [Test] + public async Task ShouldCreateNewMessageAndResolveEditedMessage() + { + CustomServiceControlPrimarySettings = s => { s.AllowMessageEditing = true; }; + + await Define() + .WithEndpoint(b => + b.When(bus => bus.SendLocal(new MyMessage { Password = "Bad password!" })).DoNotFailOnErrorMessages()) + .Done(async c => + { + if (!c.ErrorRetried) + { + var failedMessage = await GetFailedMessage(c.UniqueMessageId, ServiceControlInstanceName, + FailedMessageStatus.Unresolved); + if (!failedMessage.HasResult) + { + return false; + } + + await this.Post($"/api/edit/{failedMessage.Item.UniqueMessageId}", + new + { + MessageBody = "{ \"Password\": \"VerySecretPassword\" }", + MessageHeaders = failedMessage.Item.ProcessingAttempts[^1].Headers + }, null, + ServiceControlInstanceName); + c.ErrorRetried = true; + + return false; + } + + var failedResolvedMessage = await GetFailedMessage(c.UniqueMessageId, ServiceControlInstanceName, FailedMessageStatus.Resolved); + + return failedResolvedMessage.HasResult; + }) + .Run(TimeSpan.FromMinutes(2)); + } + + class FailureEndpoint : EndpointConfigurationBuilder + { + public FailureEndpoint() => EndpointSetup(c => { c.NoRetries(); }); + + public class MyMessageHandler(MyContext testContext, IReadOnlySettings settings) : IHandleMessages + { + public Task Handle(MyMessage message, IMessageHandlerContext context) + { + if (message.Password == "VerySecretPassword") + { + Console.Out.WriteLine("Handling message"); + return Task.CompletedTask; + } + + testContext.MessageId = context.MessageId.Replace(@"\", "-"); + testContext.EndpointNameOfReceivingEndpoint = settings.EndpointName(); + + Console.Out.WriteLine("Throwing exception"); + throw new Exception("Simulated exception"); + } + } + } + + class MyMessage : ICommand + { + public string Password { get; set; } + } + + class MyContext : ScenarioContext + { + public string MessageId { get; set; } + + public string EndpointNameOfReceivingEndpoint { get; set; } + + public string UniqueMessageId => DeterministicGuid.MakeId(MessageId, EndpointNameOfReceivingEndpoint).ToString(); + public bool ErrorRetried { get; set; } + } +} \ No newline at end of file From d7e35c21a00eb1aa2520459bbf73fb2dc1c0ee0e Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 27 Feb 2025 13:09:17 +1000 Subject: [PATCH 4/5] Update src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetrying.cs Co-authored-by: Phil Bastian <155411597+PhilBastian@users.noreply.github.com> --- .../Recoverability/WhenRetrying.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetrying.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetrying.cs index f9d75e271f..cdb7f575db 100644 --- a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetrying.cs +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetrying.cs @@ -15,7 +15,7 @@ protected Task> GetFailedMessage(string uniqueMessag return Task.FromResult(SingleResult.Empty); } - return this.TryGet("/api/errors/" + uniqueMessageId, f => f.Status == expectedStatus, instance); + return this.TryGet($"/api/errors/{uniqueMessageId}", f => f.Status == expectedStatus, instance); } protected Task> GetAllFailedMessage(string instance, FailedMessageStatus expectedStatus) => this.TryGetMany("/api/errors", f => f.Status == expectedStatus, instance); From c9419f9cf4d36d6d92287631909403f0b8aee7c6 Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 27 Feb 2025 13:15:13 +1000 Subject: [PATCH 5/5] Added a few comments --- .../WhenRetryingSameMessageMultipleTimes.cs | 8 ++++---- .../Recoverability/WhenRetryingWithEdit.cs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs index 6ea9668bf0..ed2bd76490 100644 --- a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingSameMessageMultipleTimes.cs @@ -34,10 +34,10 @@ await Define() b.When(bus => bus.SendLocal(new MyMessage())).DoNotFailOnErrorMessages()) .Done(async c => { - if (c.RetryCount >= retryTypes.Length) + if (c.RetryCount >= retryTypes.Length) // Are all retries done? { return !(await GetAllFailedMessage(ServiceControlInstanceName, FailedMessageStatus.Unresolved)) - .HasResult; + .HasResult; // Should return true if all failed messages are no longer unresolved } if (retryTypes[c.RetryCount] == RetryType.Edit) @@ -46,7 +46,7 @@ await Define() FailedMessageStatus.Unresolved); if (!results.HasResult) { - return false; + return false; // No failed messages yet } var result = results.Items.Single(); @@ -57,7 +57,7 @@ await Define() var failedMessage = await GetFailedMessage(c.UniqueMessageId, ServiceControlInstanceName, FailedMessageStatus.Unresolved); if (!failedMessage.HasResult) { - return false; + return false; // No failed message yet } if (retryTypes[c.RetryCount] == RetryType.Edit) diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingWithEdit.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingWithEdit.cs index 21aedbc47f..f824abff29 100644 --- a/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingWithEdit.cs +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/Recoverability/WhenRetryingWithEdit.cs @@ -30,7 +30,7 @@ await Define() FailedMessageStatus.Unresolved); if (!failedMessage.HasResult) { - return false; + return false; // No failed message yet } await this.Post($"/api/edit/{failedMessage.Item.UniqueMessageId}", @@ -47,7 +47,7 @@ await this.Post($"/api/edit/{failedMessage.Item.UniqueMessageId}", var failedResolvedMessage = await GetFailedMessage(c.UniqueMessageId, ServiceControlInstanceName, FailedMessageStatus.Resolved); - return failedResolvedMessage.HasResult; + return failedResolvedMessage.HasResult; // If there is a result it means the message was resolved }) .Run(TimeSpan.FromMinutes(2)); }