From 250f5dad954a9252638cf2a994eaedeb1c3f2db8 Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Sun, 26 Apr 2026 09:16:49 -0500 Subject: [PATCH] Accept *Async suffix on saga method names (GH-2578) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HandlerDiscovery has always treated saga methods named StartAsync / HandleAsync / OrchestrateAsync / ConsumeAsync / NotFoundAsync as valid — it strips the Async suffix when matching against the canonical method name list. But SagaChain.findByNames matched on strict string equality, so async-suffixed methods were discovered into the handler graph and then silently dropped from StartingCalls / ExistingCalls / NotFoundCalls. The generated chain would construct the saga but never invoke the user's method, leaving Saga.Id == Guid.Empty and throwing on insert with a confusing "must define the saga id" message. Fix: SagaChain.findByNames now accepts both the bare name (e.g. "Start") and its async-suffixed twin (e.g. "StartAsync"), making it symmetric with HandlerDiscovery. Tests: Bug_2578_saga_async_method_names exercises both halves — discovery-level (StartingCalls / ExistingCalls / NotFoundCalls populated for *Async methods across Start, Handle, Orchestrate, Consume, StartOrHandle, NotFound) and end-to-end (StartAsync / HandleAsync are actually invoked, saga state mutates and persists). 8/8 fail on main, 8/8 pass with this change. All other 1352 CoreTests still pass. Docs: docs/guide/durability/sagas.md "Method Conventions" section now documents that every name accepts the *Async variant for Task-returning methods. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/guide/durability/sagas.md | 32 ++- .../Bugs/Bug_2578_saga_async_method_names.cs | 240 ++++++++++++++++++ src/Wolverine/Persistence/Sagas/SagaChain.cs | 12 +- 3 files changed, 281 insertions(+), 3 deletions(-) create mode 100644 src/Testing/CoreTests/Bugs/Bug_2578_saga_async_method_names.cs diff --git a/docs/guide/durability/sagas.md b/docs/guide/durability/sagas.md index 5d7758c35..41cf3e7b1 100644 --- a/docs/guide/durability/sagas.md +++ b/docs/guide/durability/sagas.md @@ -475,8 +475,36 @@ The following method names are meaningful in `Saga` types: | `Orchestrate`, `Orchestrates` | Called only when the identified saga already exists | | `NotFound` | Only called if the identified saga does not already exist, and there is no matching `Start` handler for the incoming message | -Note that only `Start`, `Starts`, or `NotFound` methods can be static methods because these methods logically assume that the -identified `Saga` does not yet exist. Wolverine as of 4.6 will assert that other named `Saga` methods are instance +Each of the names above is also accepted with the `Async` suffix when the +method returns a `Task` or `Task` — e.g. `StartAsync`, `HandleAsync`, +`OrchestrateAsync`, `ConsumeAsync`, `StartOrHandleAsync`, `NotFoundAsync`. +Wolverine treats the suffixed name identically to the bare name; pick whichever +reads better in your codebase. Mixing styles within a single saga is allowed +but generally discouraged for readability. + +```csharp +public class OrderSaga : Saga +{ + public Guid Id { get; set; } + + public Task StartAsync(StartOrder command) + { + Id = command.Id; + return Task.CompletedTask; + } + + public Task HandleAsync(CompleteOrder command) + { + MarkCompleted(); + return Task.CompletedTask; + } +} +``` + +Note that only `Start` / `Starts` / `StartAsync` / `StartsAsync` and +`NotFound` / `NotFoundAsync` methods can be static methods, because these +methods logically assume that the identified `Saga` does not yet exist. +Wolverine as of 4.6 will assert that other named `Saga` methods are instance methods to try to head off confusion. ## When Sagas are Not Found diff --git a/src/Testing/CoreTests/Bugs/Bug_2578_saga_async_method_names.cs b/src/Testing/CoreTests/Bugs/Bug_2578_saga_async_method_names.cs new file mode 100644 index 000000000..08bd88c99 --- /dev/null +++ b/src/Testing/CoreTests/Bugs/Bug_2578_saga_async_method_names.cs @@ -0,0 +1,240 @@ +using JasperFx.CodeGeneration; +using JasperFx.Core.Reflection; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Wolverine.Persistence.Sagas; +using Wolverine.Runtime.Handlers; +using Wolverine.Tracking; +using Xunit; + +namespace CoreTests.Bugs; + +/// +/// Regression coverage for https://github.com/JasperFx/wolverine/issues/2578. +/// +/// HandlerDiscovery already accepts StartAsync / HandleAsync / +/// OrchestrateAsync / ConsumeAsync / NotFoundAsync as +/// valid handler method names (it strips the "Async" suffix when matching). +/// But SagaChain.findByNames previously matched on strict equality, so +/// async-suffixed saga methods were discovered into the handler graph yet +/// silently dropped from StartingCalls / ExistingCalls / +/// NotFoundCalls. The generated handler then constructed the saga but +/// never invoked the user's method, leaving Saga.Id == Guid.Empty and +/// throwing on insert. +/// +/// These tests verify that all forms of saga method are now discovered AND +/// invoked when their names carry the Async suffix. +/// +public class Bug_2578_saga_async_method_names : IAsyncLifetime +{ + private IHost _host = null!; + + public async Task InitializeAsync() + { + _host = await Host.CreateDefaultBuilder() + .UseWolverine(opts => + { + opts.Discovery.DisableConventionalDiscovery() + .IncludeType() + .IncludeType() + .IncludeType() + .IncludeType() + .IncludeType(); + + opts.CodeGeneration.TypeLoadMode = TypeLoadMode.Auto; + }).StartAsync(); + } + + public async Task DisposeAsync() + { + await _host.StopAsync(); + _host.Dispose(); + } + + private SagaChain ChainFor() + { + return (SagaChain)_host.Services.GetRequiredService() + .HandlerFor()!.As().Chain!; + } + + // ---------- Discovery-level assertions ---------- + + [Fact] + public void StartAsync_is_picked_up_as_a_StartingCall() + { + var chain = ChainFor(); + chain.StartingCalls.ShouldHaveSingleItem() + .Method.Name.ShouldBe(nameof(AsyncMethodSaga.StartAsync)); + } + + [Fact] + public void HandleAsync_is_picked_up_as_an_ExistingCall() + { + var chain = ChainFor(); + chain.ExistingCalls.ShouldHaveSingleItem() + .Method.Name.ShouldBe(nameof(AsyncMethodSaga.HandleAsync)); + } + + [Fact] + public void OrchestrateAsync_is_picked_up_as_an_ExistingCall() + { + var chain = ChainFor(); + chain.ExistingCalls.ShouldHaveSingleItem() + .Method.Name.ShouldBe(nameof(AsyncOrchestrateSaga.OrchestrateAsync)); + } + + [Fact] + public void ConsumeAsync_is_picked_up_as_an_ExistingCall() + { + var chain = ChainFor(); + chain.ExistingCalls.ShouldHaveSingleItem() + .Method.Name.ShouldBe(nameof(AsyncConsumeSaga.ConsumeAsync)); + } + + [Fact] + public void StartOrHandleAsync_is_picked_up_in_both_StartingCalls_and_ExistingCalls() + { + var chain = ChainFor(); + chain.StartingCalls.ShouldHaveSingleItem() + .Method.Name.ShouldBe(nameof(AsyncStartOrHandleSaga.StartOrHandleAsync)); + chain.ExistingCalls.ShouldHaveSingleItem() + .Method.Name.ShouldBe(nameof(AsyncStartOrHandleSaga.StartOrHandleAsync)); + } + + [Fact] + public void NotFoundAsync_is_picked_up_as_a_NotFoundCall() + { + var chain = ChainFor(); + chain.NotFoundCalls.ShouldHaveSingleItem() + .Method.Name.ShouldBe(nameof(AsyncNotFoundSaga.NotFoundAsync)); + } + + // ---------- End-to-end assertions ---------- + + [Fact] + public async Task StartAsync_is_actually_invoked_and_persists_the_saga() + { + var id = Guid.NewGuid(); + await _host.InvokeMessageAndWaitAsync(new StartAsyncCommand2578(id, "first")); + + var saga = _host.Services.GetRequiredService() + .Load(id); + saga.ShouldNotBeNull("StartAsync must be invoked by the generated handler"); + saga.Id.ShouldBe(id); + saga.Name.ShouldBe("first"); + } + + [Fact] + public async Task HandleAsync_is_actually_invoked_against_an_existing_saga() + { + var id = Guid.NewGuid(); + await _host.InvokeMessageAndWaitAsync(new StartAsyncCommand2578(id, "first")); + await _host.InvokeMessageAndWaitAsync(new HandleAsyncCommand2578 { SagaId = id, NextName = "second" }); + + var saga = _host.Services.GetRequiredService() + .Load(id); + saga.ShouldNotBeNull(); + saga.Name.ShouldBe("second"); + } +} + +#region Test sagas + +public record StartAsyncCommand2578(Guid Id, string Name); + +public class HandleAsyncCommand2578 +{ + public Guid SagaId { get; set; } + public string NextName { get; set; } = ""; +} + +public class AsyncMethodSaga : Saga +{ + public Guid Id { get; set; } + public string Name { get; set; } = ""; + + public Task StartAsync(StartAsyncCommand2578 command) + { + Id = command.Id; + Name = command.Name; + return Task.CompletedTask; + } + + public Task HandleAsync(HandleAsyncCommand2578 command) + { + Name = command.NextName; + return Task.CompletedTask; + } +} + +public class OrchestrateAsyncCommand2578 +{ + public Guid SagaId { get; set; } +} + +public class AsyncOrchestrateSaga : Saga +{ + public Guid Id { get; set; } + + // Need a Start so the saga can come into existence for the test, even though + // we only assert OrchestrateAsync discovery-level behavior here. + public void Start(InitOrchestrateAsyncSaga command) => Id = command.Id; + + public Task OrchestrateAsync(OrchestrateAsyncCommand2578 command) + { + return Task.CompletedTask; + } +} + +public record InitOrchestrateAsyncSaga(Guid Id); + +public class ConsumeAsyncCommand2578 +{ + public Guid SagaId { get; set; } +} + +public class AsyncConsumeSaga : Saga +{ + public Guid Id { get; set; } + + public void Start(InitConsumeAsyncSaga command) => Id = command.Id; + + public Task ConsumeAsync(ConsumeAsyncCommand2578 command) + { + return Task.CompletedTask; + } +} + +public record InitConsumeAsyncSaga(Guid Id); + +public record StartOrHandleAsyncCommand2578(Guid Id); + +public class AsyncStartOrHandleSaga : Saga +{ + public Guid Id { get; set; } + + public Task StartOrHandleAsync(StartOrHandleAsyncCommand2578 command) + { + Id = command.Id; + return Task.CompletedTask; + } +} + +public record NotFoundAsyncCommand2578(Guid Id); + +public class AsyncNotFoundSaga : Saga +{ + public Guid Id { get; set; } + + public void Handle(NotFoundAsyncCommand2578 command) + { + // Existing-saga path + } + + public static Task NotFoundAsync(NotFoundAsyncCommand2578 command) + { + return Task.CompletedTask; + } +} + +#endregion diff --git a/src/Wolverine/Persistence/Sagas/SagaChain.cs b/src/Wolverine/Persistence/Sagas/SagaChain.cs index f9721de46..d391d40c4 100644 --- a/src/Wolverine/Persistence/Sagas/SagaChain.cs +++ b/src/Wolverine/Persistence/Sagas/SagaChain.cs @@ -215,7 +215,17 @@ protected override void validateAgainstInvalidSagaMethods(IGrouping methodNames.Contains(x.Method.Name) && x.HandlerType.CanBeCastTo()).ToArray(); + // Match either the bare name (e.g. "Start") or its async-suffixed twin + // (e.g. "StartAsync"). HandlerDiscovery already strips the "Async" + // suffix when picking up handler methods, so without this the saga + // method would be discovered into the chain but silently dropped from + // StartingCalls / ExistingCalls / NotFoundCalls and never invoked. + // See https://github.com/JasperFx/wolverine/issues/2578. + return Handlers + .Where(x => x.HandlerType.CanBeCastTo() + && methodNames.Any(n => + x.Method.Name == n || x.Method.Name == n + "Async")) + .ToArray(); } internal override List DetermineFrames(GenerationRules rules, IServiceContainer container,