From 121df8fab07745f3ae0d12729120e6770c22203c Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Mon, 3 Aug 2026 00:15:45 -0500 Subject: [PATCH] Point the AWS tests at LocalStack, and fix the SQS name limit they were hiding (GH-3763) Eleven AWS test files were tagged [Trait("Category", "Flaky")] in a bulk sweep on 2026-03-20/21. Ten of them were never flaky: they call UseAmazonSqsTransport(), which resolves the ambient AWS credential chain. CI has no credentials, so they fail deterministically: Amazon.Runtime.AmazonClientException: No RegionEndpoint or ServiceURL configured Error trying to start message broker sqs on Attempt 1..20 of 20 and on a developer machine that IS logged in, they quietly provision queues in a real AWS account. The tag was not hiding instability, it was hiding a wiring mistake. All of them now use UseAmazonSqsTransportLocally(), like the rest of the suite. Two real defects fell out once they ran: - AmazonSqsTransport.SanitizeSqsName only replaced '.' with '-'. SQS accepts "alphanumeric characters, hyphens, or underscores. 1 to 80 in length", and "shazaam-Wolverine-AmazonSqs-Tests-ConventionalRouting-SqsHandlerTypeNamingMessage" is 81. Broker initialization provisions every queue together, so one overlong or illegally-named queue -- Handle(Item[]), generics, nested types -- takes down startup for every conventionally-routed host in the assembly. Same defect GH-3786 just fixed on Azure Service Bus, and the exception chaining added there is what made this readable in minutes instead of hours. Illegal characters are now substituted rather than stripped, so Item[] stays separable from Item, and an overlong name is truncated with a SHA-256 digest appended -- deterministic across processes, which rules out GetHashCode(). No name that works today changes: a name SQS rejects could never have been provisioned. - The fixtures leaked their hosts. IHost.Dispose() tears down the container without running IHostedService.StopAsync, so the SQS listeners kept polling: 12 listeners started on the shared sqs://routed queue and 4 stopped. Eight zombies then stole messages from whichever class ran next. ConventionalRoutingContext now owns disposal, and derived classes override InitializeAsync instead of re-declaring IAsyncLifetime -- re-declaring it is what let a no-op DisposeAsync shadow the real one, here and in the ASB fixtures (GH-3758). end_to_end_with_conventional_routing also gets its own message type. The shard runs this project across worker PROCESSES partitioned by class, so CollectionPerAssembly only serializes within one process; a sibling class holding a sqs://routed listener in another process received the message and the tracked session timed out waiting for a delivery that had already happened elsewhere. Two that stay excluded, honestly labelled rather than called flaky: - Samples/Bootstrapping.customize_mappers was the file's only [Fact], inside a doc region that must keep showing the real UseAmazonSqsTransport() a reader would write. Now private, compile-checked like every other sample there. - SNS send_to_topic_and_receive_in_queue_in_aws is a line-for-line duplicate of send_to_topic_and_receive_in_queue except that it points at a real AWS account. Now [Fact(Skip = ...)] with the reason, so it reports as skipped instead of being silently filtered. Verified with no AWS credentials at all (HOME pointed at an empty directory) on a freshly recreated LocalStack: CIAWSSqs 137 -> 172 passed, CIAWSSqsCompliance 93, CIAWSSns 119, all with 0 retries. Repo flaky tags 32 -> 21. wolverine.slnx builds clean in Release. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA --- ...nd_to_topic_and_receive_in_queue_in_aws.cs | 7 +- .../AmazonSqsTransportTests.cs | 81 ++++++++++++++++++- ...ventional_routing_respects_named_broker.cs | 14 ++-- .../ConventionalRoutingContext.cs | 46 +++++++++-- .../EndToEndRoutedMessage.cs | 23 ++++++ .../conventional_listener_discovery.cs | 1 - .../discover_with_naming_prefix.cs | 19 +++-- .../end_to_end_with_conventional_routing.cs | 25 +++--- ...d_with_conventional_routing_with_prefix.cs | 21 ++--- ..._a_listening_endpoint_with_all_defaults.cs | 7 +- ...g_endpoint_with_overridden_queue_naming.cs | 7 +- ..._discovering_a_sender_with_all_defaults.cs | 7 +- .../when_using_handler_type_naming.cs | 11 ++- .../Samples/Bootstrapping.cs | 7 +- ...oncurrency_resilient_sharded_processing.cs | 1 - .../Internal/AmazonSqsTransport.cs | 70 ++++++++++++++-- 16 files changed, 271 insertions(+), 76 deletions(-) create mode 100644 src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/EndToEndRoutedMessage.cs diff --git a/src/Transports/AWS/Wolverine.AmazonSns.Tests/send_to_topic_and_receive_in_queue_in_aws.cs b/src/Transports/AWS/Wolverine.AmazonSns.Tests/send_to_topic_and_receive_in_queue_in_aws.cs index d67246635..f942e63e7 100644 --- a/src/Transports/AWS/Wolverine.AmazonSns.Tests/send_to_topic_and_receive_in_queue_in_aws.cs +++ b/src/Transports/AWS/Wolverine.AmazonSns.Tests/send_to_topic_and_receive_in_queue_in_aws.cs @@ -8,7 +8,6 @@ namespace Wolverine.AmazonSns.Tests; -[Trait("Category", "Flaky")] public class send_to_topic_and_receive_in_queue_in_aws : IAsyncLifetime { private IHost _host = null!; @@ -53,7 +52,11 @@ public async ValueTask DisposeAsync() _host.Dispose(); } - [Fact] + // Line-for-line the same test as send_to_topic_and_receive_in_queue, except that it points at a + // real AWS account instead of LocalStack. It is here to be run by hand when SNS fidelity is in + // question; CI has no credentials, so it can only ever fail there. Skipped rather than tagged + // Flaky (#3763) because there is nothing unstable about it. + [Fact(Skip = "Requires real AWS credentials; the LocalStack twin is send_to_topic_and_receive_in_queue")] public async Task send_to_topic_and_receive_in_queue_a_single_message() { var message = new SnsMessage("Josh Allen"); diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/AmazonSqsTransportTests.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/AmazonSqsTransportTests.cs index e70ee65c0..205f701ce 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/AmazonSqsTransportTests.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/AmazonSqsTransportTests.cs @@ -100,4 +100,83 @@ public void findEndpointByUri_should_correctly_create_endpoint_if_it_doesnt_exis transport.Queues.Count.ShouldBe(2); result2.EndpointName.ShouldBe(queueName2); } -} \ No newline at end of file +} +// GH-3763. Conventional routing derives queue names from message type names, so it can hand SQS a +// name it rejects outright -- and because broker initialization provisions every queue together, +// one bad name fails startup for every conventionally-routed host in the assembly. See GH-3786 for +// the same defect on Azure Service Bus. +public class sanitizing_sqs_queue_names +{ + // "Can only include alphanumeric characters, hyphens, or underscores. 1 to 80 in length" + [Theory] + [InlineData("Wolverine.Bugs.BatchedItem[]", "Wolverine-Bugs-BatchedItem__")] + [InlineData("Wolverine.Envelope`1[System.String]", "Wolverine-Envelope_1_System-String_")] + [InlineData("Outer+Inner", "Outer_Inner")] + [InlineData("has spaces", "has_spaces")] + public void illegal_characters_are_substituted(string identifier, string expected) + { + AmazonSqsTransport.SanitizeSqsName(identifier).ShouldBe(expected); + } + + // Substituting rather than stripping is what keeps distinct type names distinct. + [Fact] + public void sanitizing_does_not_collide_an_array_type_with_its_element_type() + { + AmazonSqsTransport.SanitizeSqsName("BatchedItem[]") + .ShouldNotBe(AmazonSqsTransport.SanitizeSqsName("BatchedItem")); + } + + // No name that works today may change: a name SQS rejects could never have been provisioned in + // the first place, so this must stay a pure no-op for legal names of legal length. + [Theory] + [InlineData("Wolverine.Bugs.BatchedItem", "Wolverine-Bugs-BatchedItem")] + [InlineData("two-dead-letter-queue", "two-dead-letter-queue")] + [InlineData("wolverine_retries_MyService", "wolverine_retries_MyService")] + [InlineData("wolverine.retries.MyService.fifo", "wolverine-retries-MyService.fifo")] + public void legal_identifiers_are_left_alone(string identifier, string expected) + { + AmazonSqsTransport.SanitizeSqsName(identifier).ShouldBe(expected); + } + + // The case that actually broke CI: "shazaam-" + a namespace-qualified type name is 81 characters. + [Fact] + public void an_overlong_name_is_brought_under_the_limit() + { + var name = AmazonSqsTransport.SanitizeSqsName( + "shazaam-Wolverine.AmazonSqs.Tests.ConventionalRouting.SqsHandlerTypeNamingMessage"); + + name.Length.ShouldBe(AmazonSqsTransport.MaximumQueueNameLength); + name.ShouldStartWith("shazaam-Wolverine-AmazonSqs-Tests-ConventionalRouting-SqsHandlerType"); + } + + // Truncation alone would collide two long names that share a prefix -- and namespace-qualified + // type names very often do. + [Fact] + public void two_overlong_names_sharing_a_prefix_stay_distinct() + { + var prefix = new string('a', AmazonSqsTransport.MaximumQueueNameLength); + + AmazonSqsTransport.SanitizeSqsName(prefix + "One") + .ShouldNotBe(AmazonSqsTransport.SanitizeSqsName(prefix + "Two")); + } + + // The digest has to be stable across processes and machines, which rules out GetHashCode(). + [Fact] + public void truncation_is_deterministic() + { + var identifier = new string('b', 200); + + AmazonSqsTransport.SanitizeSqsName(identifier) + .ShouldBe("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-aaebc35c"); + } + + // AWS requires the .fifo suffix, and it counts against the 80 character budget. + [Fact] + public void the_fifo_suffix_survives_truncation() + { + var name = AmazonSqsTransport.SanitizeSqsName(new string('c', 200) + ".fifo"); + + name.Length.ShouldBe(AmazonSqsTransport.MaximumQueueNameLength); + name.ShouldEndWith(".fifo"); + } +} diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/Bug_3633_conventional_routing_respects_named_broker.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/Bug_3633_conventional_routing_respects_named_broker.cs index f557529b4..3eef192a3 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/Bug_3633_conventional_routing_respects_named_broker.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/Bug_3633_conventional_routing_respects_named_broker.cs @@ -7,14 +7,14 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; // https://github.com/JasperFx/wolverine/issues/3633 -public class Bug_3633_conventional_routing_respects_named_broker : IDisposable +public class Bug_3633_conventional_routing_respects_named_broker : IAsyncLifetime { private static readonly BrokerName theBrokerName = new("other"); - private readonly IHost _host; + private IHost _host = null!; - public Bug_3633_conventional_routing_respects_named_broker() + public async ValueTask InitializeAsync() { - _host = Host.CreateDefaultBuilder() + _host = await Host.CreateDefaultBuilder() .UseWolverine(opts => { // A default, unnamed broker is also registered so that conventional @@ -25,11 +25,13 @@ public Bug_3633_conventional_routing_respects_named_broker() .UseConventionalRouting(x => x.IncludeTypes(t => t == typeof(RoutedMessage))) .AutoProvision() .AutoPurgeOnStartup(); - }).Start(); + }).StartAsync(); } - public void Dispose() + // StopAsync, not just Dispose -- see the note in ConventionalRoutingContext (GH-3763). + public async ValueTask DisposeAsync() { + await _host.StopAsync(); _host.Dispose(); } diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/ConventionalRoutingContext.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/ConventionalRoutingContext.cs index 2cf6f711b..e39939ac7 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/ConventionalRoutingContext.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/ConventionalRoutingContext.cs @@ -7,21 +7,53 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -public abstract class ConventionalRoutingContext : IDisposable +/// +/// Every class in this namespace listens to the same small set of LocalStack queues -- most of them +/// to sqs://routed. A host that is disposed but never stopped keeps long-polling that queue, +/// so it steals messages from whichever class runs next and the theft looks like flakiness. See +/// GH-3763. +/// +/// This type owns disposal for that reason: derived classes override +/// rather than re-declaring themselves. Re-declaring it is what let a +/// no-op DisposeAsync shadow the real one here and in the Azure Service Bus fixtures +/// (GH-3758) -- an explicit interface implementation on a derived class silently wins. +/// +public abstract class ConventionalRoutingContext : IAsyncLifetime { private IHost _host = null!; + public virtual ValueTask InitializeAsync() => ValueTask.CompletedTask; + internal async Task theRuntime() { _host ??= await WolverineHost.ForAsync(opts => - opts.UseAmazonSqsTransport().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup()); + opts.UseAmazonSqsTransportLocally().UseConventionalRouting(leaveTheEndToEndQueueAlone).AutoProvision() + .AutoPurgeOnStartup()); return _host.Services.GetRequiredService(); } - public void Dispose() + /// + /// These classes only assert on configuration, but they still stand up real listeners for every + /// handler in the assembly -- which would include the queue end_to_end_with_conventional_routing + /// is trying to receive on, in a different worker process. An ExcludeTypes rather than an + /// IncludeTypes: excludes are ANDed and includes are ORed, so an include would silently widen a + /// test's own filter in ConfigureConventions. See GH-3763. + /// + private static void leaveTheEndToEndQueueAlone(AmazonSqsMessageRoutingConvention convention) { - _host?.Dispose(); + convention.ExcludeTypes(t => t == typeof(EndToEndRoutedMessage)); + } + + public async ValueTask DisposeAsync() + { + if (_host == null) return; + + // StopAsync, not just Dispose: IHost.Dispose() tears down the container without ever + // running IHostedService.StopAsync, which leaves the SQS listeners polling. + await _host.StopAsync(); + _host.Dispose(); + _host = null!; } internal async Task ConfigureConventions(Action configure) @@ -29,7 +61,11 @@ internal async Task ConfigureConventions(Action { - opts.UseAmazonSqsTransport().UseConventionalRouting(configure).AutoProvision() + opts.UseAmazonSqsTransportLocally().UseConventionalRouting(c => + { + leaveTheEndToEndQueueAlone(c); + configure(c); + }).AutoProvision() .AutoPurgeOnStartup(); }).StartAsync(); } diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/EndToEndRoutedMessage.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/EndToEndRoutedMessage.cs new file mode 100644 index 000000000..d3bca8ab3 --- /dev/null +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/EndToEndRoutedMessage.cs @@ -0,0 +1,23 @@ +using Wolverine.Attributes; + +namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; + +/// +/// end_to_end_with_conventional_routing needs a queue nothing else in this namespace listens to. +/// +/// The CI shard runs this project across three worker PROCESSES partitioned by test class, so +/// CollectionPerAssembly only buys serialization inside one process. Every other conventional +/// routing class here stands up a host listening at sqs://routed for +/// ; a concurrent worker holding that listener receives the end-to-end +/// message and the tracked session times out waiting for a delivery that already happened +/// somewhere else. Its own message type gives it its own queue. See GH-3763. +/// +[MessageIdentity("end-to-end-routed")] +public class EndToEndRoutedMessage; + +public class EndToEndRoutedMessageHandler +{ + public void Handle(EndToEndRoutedMessage message) + { + } +} diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/conventional_listener_discovery.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/conventional_listener_discovery.cs index cab9230c5..be3014c8a 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/conventional_listener_discovery.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/conventional_listener_discovery.cs @@ -9,7 +9,6 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] public class conventional_listener_discovery : ConventionalRoutingContext { [Fact] diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/discover_with_naming_prefix.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/discover_with_naming_prefix.cs index 5c409fa28..d74352e2f 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/discover_with_naming_prefix.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/discover_with_naming_prefix.cs @@ -5,25 +5,24 @@ using Xunit; namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class discover_with_naming_prefix : IDisposable +public class discover_with_naming_prefix : IAsyncLifetime { - private readonly IHost _host; - private readonly ITestOutputHelper _output; + private IHost _host = null!; - public discover_with_naming_prefix(ITestOutputHelper output) + public async ValueTask InitializeAsync() { - _output = output; - _host = Host.CreateDefaultBuilder() + _host = await Host.CreateDefaultBuilder() .UseWolverine(opts => { - opts.UseAmazonSqsTransport().PrefixIdentifiers("zztop").UseConventionalRouting().AutoProvision() + opts.UseAmazonSqsTransportLocally().PrefixIdentifiers("zztop").UseConventionalRouting().AutoProvision() .AutoPurgeOnStartup(); - }).Start(); + }).StartAsync(); } - public void Dispose() + // StopAsync, not just Dispose -- see the note in ConventionalRoutingContext (GH-3763). + public async ValueTask DisposeAsync() { + await _host.StopAsync(); _host.Dispose(); } diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing.cs index 4ff6ceb6a..efc60f7c4 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing.cs @@ -6,8 +6,7 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class end_to_end_with_conventional_routing : IAsyncLifetime, IDisposable +public class end_to_end_with_conventional_routing : IAsyncLifetime { private IHost _receiver = null!; private IHost _sender = null!; @@ -16,24 +15,28 @@ public async ValueTask InitializeAsync() { _sender = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); + opts.UseAmazonSqsTransportLocally().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); opts.DisableConventionalDiscovery(); opts.ServiceName = "Sender"; }); _receiver = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); + opts.UseAmazonSqsTransportLocally().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); opts.ServiceName = "Receiver"; }); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - - public void Dispose() + // StopAsync, not just Dispose: IHost.Dispose() tears down the container without ever running + // IHostedService.StopAsync, so the SQS listeners keep polling and steal messages from the next + // class in this namespace. See GH-3763. + public async ValueTask DisposeAsync() { - _sender?.Dispose(); - _receiver?.Dispose(); + await _sender.StopAsync(); + _sender.Dispose(); + + await _receiver.StopAsync(); + _receiver.Dispose(); } [Fact] @@ -43,11 +46,11 @@ public async Task send_from_one_node_to_another_all_with_conventional_routing() .AlsoTrack(_receiver) .IncludeExternalTransports() .Timeout(30.Seconds()) - .SendMessageAndWaitAsync(new RoutedMessage()); + .SendMessageAndWaitAsync(new EndToEndRoutedMessage()); var received = session .AllRecordsInOrder() - .Where(x => x.Envelope!.Message!.GetType() == typeof(RoutedMessage)) + .Where(x => x.Envelope!.Message!.GetType() == typeof(EndToEndRoutedMessage)) .Single(x => x.MessageEventType == MessageEventType.Received); received diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing_with_prefix.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing_with_prefix.cs index 7c81fe8a7..0bf481908 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing_with_prefix.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing_with_prefix.cs @@ -6,8 +6,7 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class end_to_end_with_conventional_routing_with_prefix : IAsyncLifetime, IDisposable +public class end_to_end_with_conventional_routing_with_prefix : IAsyncLifetime { private IHost _receiver = null!; private IHost _sender = null!; @@ -16,7 +15,7 @@ public async ValueTask InitializeAsync() { _sender = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport() + opts.UseAmazonSqsTransportLocally() .PrefixIdentifiers("shazaam") .UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); opts.DisableConventionalDiscovery(); @@ -25,19 +24,23 @@ public async ValueTask InitializeAsync() _receiver = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport() + opts.UseAmazonSqsTransportLocally() .PrefixIdentifiers("shazaam") .UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); opts.ServiceName = "Receiver"; }); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - - public void Dispose() + // StopAsync, not just Dispose: IHost.Dispose() tears down the container without ever running + // IHostedService.StopAsync, so the SQS listeners keep polling and steal messages from the next + // class in this namespace. See GH-3763. + public async ValueTask DisposeAsync() { - _sender?.Dispose(); - _receiver?.Dispose(); + await _sender.StopAsync(); + _sender.Dispose(); + + await _receiver.StopAsync(); + _receiver.Dispose(); } [Fact] diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_all_defaults.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_all_defaults.cs index dcfb834a2..6775e5c1f 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_all_defaults.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_all_defaults.cs @@ -5,19 +5,16 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class when_discovering_a_listening_endpoint_with_all_defaults : ConventionalRoutingContext, IAsyncLifetime +public class when_discovering_a_listening_endpoint_with_all_defaults : ConventionalRoutingContext { private readonly Uri theExpectedUri = "sqs://routed".ToUri(); private AmazonSqsQueue theQueue = null!; - public async ValueTask InitializeAsync() + public override async ValueTask InitializeAsync() { theQueue = (await theRuntime()).Endpoints.EndpointFor(theExpectedUri).ShouldBeOfType(); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - [Fact] public void endpoint_should_be_a_listener() { diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_overridden_queue_naming.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_overridden_queue_naming.cs index 4e1ca56ba..73c58a35e 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_overridden_queue_naming.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_overridden_queue_naming.cs @@ -4,13 +4,12 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class when_discovering_a_listening_endpoint_with_overridden_queue_naming : ConventionalRoutingContext, IAsyncLifetime +public class when_discovering_a_listening_endpoint_with_overridden_queue_naming : ConventionalRoutingContext { private readonly Uri theExpectedUri = "sqs://routedmessage2".ToUri(); private AmazonSqsQueue theQueue = null!; - public async ValueTask InitializeAsync() + public override async ValueTask InitializeAsync() { await ConfigureConventions(c => c.QueueNameForListener(t => t.Name.ToLower() + "2")); @@ -19,8 +18,6 @@ public async ValueTask InitializeAsync() theQueue = runtime.Endpoints.EndpointFor(theExpectedUri).ShouldBeOfType(); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - [Fact] public void endpoint_should_be_a_listener() { diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_sender_with_all_defaults.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_sender_with_all_defaults.cs index 9aa386681..404052e9a 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_sender_with_all_defaults.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_sender_with_all_defaults.cs @@ -6,18 +6,15 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class when_discovering_a_sender_with_all_defaults : ConventionalRoutingContext, IAsyncLifetime +public class when_discovering_a_sender_with_all_defaults : ConventionalRoutingContext { private MessageRoute theRoute = null!; - public async ValueTask InitializeAsync() + public override async ValueTask InitializeAsync() { theRoute = (await PublishingRoutesFor()).Single().As(); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - [Fact] public void should_have_exactly_one_route() { diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_using_handler_type_naming.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_using_handler_type_naming.cs index 660c81bc8..df0744c81 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_using_handler_type_naming.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_using_handler_type_naming.cs @@ -10,8 +10,7 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class when_using_handler_type_naming : IAsyncLifetime, IDisposable +public class when_using_handler_type_naming : IAsyncLifetime { private IHost _host = null!; private IWolverineRuntime _runtime = null!; @@ -20,7 +19,7 @@ public async ValueTask InitializeAsync() { _host = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport() + opts.UseAmazonSqsTransportLocally() .UseConventionalRouting(NamingSource.FromHandlerType) .AutoProvision() .AutoPurgeOnStartup(); @@ -52,10 +51,10 @@ public void listener_should_be_active() .ShouldBeTrue($"Expected active listener containing '{expectedName}'"); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - - public void Dispose() + // StopAsync, not just Dispose -- see the note in ConventionalRoutingContext (GH-3763). + public async ValueTask DisposeAsync() { + await _host.StopAsync(); _host.Dispose(); } } diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/Samples/Bootstrapping.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/Samples/Bootstrapping.cs index efa7513df..ffc8f1eab 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/Samples/Bootstrapping.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/Samples/Bootstrapping.cs @@ -12,7 +12,6 @@ namespace Wolverine.AmazonSqs.Tests.Samples; -[Trait("Category", "Flaky")] public class Bootstrapping { private async Task use_named_brokers() @@ -355,8 +354,10 @@ private async Task publish_raw_json() #endregion } - [Fact] - public async Task customize_mappers() + // Compile-checked only, like every other sample in this file. The snippet is documentation, so + // it shows the real `UseAmazonSqsTransport()` a reader would write — which means running it + // would talk to a real AWS account, and CI has no credentials. + private async Task customize_mappers() { #region sample_apply_custom_sqs_mapping using var host = await Host.CreateDefaultBuilder() diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/concurrency_resilient_sharded_processing.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/concurrency_resilient_sharded_processing.cs index f75298905..189b6c9d8 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/concurrency_resilient_sharded_processing.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/concurrency_resilient_sharded_processing.cs @@ -12,7 +12,6 @@ using Xunit; namespace Wolverine.AmazonSqs.Tests; -[Trait("Category", "Flaky")] public class concurrency_resilient_sharded_processing { private readonly ITestOutputHelper _output; diff --git a/src/Transports/AWS/Wolverine.AmazonSqs/Internal/AmazonSqsTransport.cs b/src/Transports/AWS/Wolverine.AmazonSqs/Internal/AmazonSqsTransport.cs index aaa1d58de..56e283cc1 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs/Internal/AmazonSqsTransport.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs/Internal/AmazonSqsTransport.cs @@ -1,3 +1,5 @@ +using System.Security.Cryptography; +using System.Text; using Amazon.Runtime; using Amazon.SQS; using Amazon.SQS.Model; @@ -135,23 +137,79 @@ internal AmazonSqsTransport(IAmazonSQS client) : this() /// public bool SystemQueuesEnabled { get; set; } + /// + /// The hard limit Amazon SQS puts on a queue name. The .fifo suffix counts against it. + /// + public const int MaximumQueueNameLength = 80; + + /// + /// Coerce an identifier into something Amazon SQS will actually accept: "Can only include + /// alphanumeric characters, hyphens, or underscores. 1 to 80 in length". + /// + /// Conventional routing derives queue names from message type names, so the raw input can carry + /// characters SQS rejects (Handle(Item[]), generics, nested types) or simply run past 80 + /// characters once a prefix is applied. Either one fails CreateQueue with a 400, and + /// because broker initialization provisions every queue together, one bad name takes down + /// startup for every conventionally-routed host in the assembly. See GH-3763, and GH-3786 for + /// the same defect on Azure Service Bus. + /// + /// This is a no-op for every name that works today: a name SQS rejects could never have been + /// provisioned in the first place. + /// public static string SanitizeSqsName(string identifier) { //AWS requires FIFO queues to have a `.fifo` suffix var suffixIndex = identifier.LastIndexOf(".fifo", StringComparison.OrdinalIgnoreCase); + var suffix = string.Empty; + var name = identifier; + if (suffixIndex != -1) // ".fifo" suffix found { - var prefix = identifier[..suffixIndex]; - var suffix = identifier[suffixIndex..]; + suffix = identifier[suffixIndex..]; + name = identifier[..suffixIndex]; + } + + return truncateToLimit(substituteIllegalCharacters(name), suffix); + } + + private static string substituteIllegalCharacters(string name) + { + var characters = new char[name.Length]; + + for (var i = 0; i < name.Length; i++) + { + var c = name[i]; - prefix = prefix.Replace('.', Separator); + // '.' has always mapped to the identifier separator, and plenty of existing queue names + // depend on that exact spelling. Everything else illegal becomes '_' -- substituting + // rather than stripping is what keeps Item[] separable from Item. + characters[i] = c switch + { + '.' => Separator, + '-' or '_' => c, + _ => char.IsAsciiLetterOrDigit(c) ? c : '_' + }; + } - return prefix + suffix; + return new string(characters); + } + + private static string truncateToLimit(string name, string suffix) + { + if (name.Length + suffix.Length <= MaximumQueueNameLength) + { + return name + suffix; } - // ".fifo" suffix not found - return identifier.Replace('.', Separator); + // Truncation alone would collide two long names that share a prefix, and conventionally + // routed names are namespace-qualified type names, which very often do. Append a stable + // digest of the full name so the result stays unique -- and stays the SAME across processes + // and machines, which rules out string.GetHashCode() (randomized per process on .NET Core). + var digest = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(name))).ToLowerInvariant()[..8]; + var budget = MaximumQueueNameLength - suffix.Length - digest.Length - 1; + + return string.Concat(name.AsSpan(0, budget), Separator.ToString(), digest, suffix); } public override string SanitizeIdentifier(string identifier)