diff --git a/docs/guide/codegen.md b/docs/guide/codegen.md index 27cf86b5c..ab9f9dca9 100644 --- a/docs/guide/codegen.md +++ b/docs/guide/codegen.md @@ -327,6 +327,26 @@ using var host = Host.CreateDefaultBuilder() snippet source | anchor +::: warning +Be careful with this one if your handlers live in a **class library** rather than in the entry project. `codegen write` +writes its output relative to the host's content root -- that is, into the *entry* project -- while `TypeLoadMode.Static` +loads pre-built types out of `ApplicationAssembly`. Setting `ApplicationAssembly` to the class library makes the two +disagree: the generated types compile into the entry assembly, and the static loader looks for them in the library. + +If you were only setting `ApplicationAssembly` so that Wolverine would *discover* handlers in that library, use handler +discovery for that instead and leave `ApplicationAssembly` alone: + +```csharp +opts.Discovery.IncludeAssembly(typeof(SomeHandler).Assembly); +``` + +If you really do want `ApplicationAssembly` to be the class library, point the generated code output at the project that +builds it with `opts.CodeGeneration.GeneratedCodeOutputPath` so `codegen write` and the static loader agree. + +Since Wolverine 6.30 this mismatch fails the host start rather than the first message. Before that the host booted +healthy and every message of the affected types was lost. +::: + If the assembly choice is correct, and the expected code files are really in `Internal/Generated` exactly as you'd expect, make sure there's no accidental `` nodes in your project file. *Don't laugh, that's actually happened to Wolverine users* @@ -464,6 +484,13 @@ For example, this functionality might be helpful for: ## Environment Check for Expected Types +::: tip +As of Wolverine 6.30, **message handlers** no longer need this opt in. In `TypeLoadMode.Static` Wolverine +asserts at startup that every handler chain's pre-generated type is really in the application assembly, and +throws a `MissingPreBuiltTypesException` naming the chains it could not load if any are missing. The check +below still covers the other code file collections -- HTTP endpoints, gRPC services, Marten document providers. +::: + As a new option in Wolverine 1.7.0, you can also add an environment check for the existence of the expected pre-built types to [fail fast](https://en.wikipedia.org/wiki/Fail-fast) on application startup like this: diff --git a/src/Persistence/PostgresqlTests/Bugs/Bug_4151_executor_failure_dead_letters_the_envelope.cs b/src/Persistence/PostgresqlTests/Bugs/Bug_4151_executor_failure_dead_letters_the_envelope.cs new file mode 100644 index 000000000..d5441115f --- /dev/null +++ b/src/Persistence/PostgresqlTests/Bugs/Bug_4151_executor_failure_dead_letters_the_envelope.cs @@ -0,0 +1,99 @@ +using IntegrationTests; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Npgsql; +using Shouldly; +using Wolverine; +using Wolverine.Attributes; +using Wolverine.Persistence.Durability; +using Wolverine.Postgresql; +using Wolverine.Runtime; +using Wolverine.Tracking; +using Xunit; + +namespace PostgresqlTests.Bugs; + +// GH-4151: an envelope whose executor cannot be built died in HandlerPipeline's last line of defense, which +// acks the message out of the way. On a durable transport that meant the inbox row was marked Handled with +// attempts=0 and then removed by ordinary durable-inbox cleanup -- byte for byte the lifecycle of a message +// that was handled *successfully*. Nothing in the message store distinguished "never handled, executor could +// not be built" from "handled", the dead letter table stayed empty, and the host stayed healthy. The +// reporter lost every message of one type in production this way. +// +// So this asserts against the message store itself rather than a tracked session: the row that used to be +// absent has to be there. +public class Bug_4151_executor_failure_dead_letters_the_envelope : IAsyncLifetime +{ + private const string SchemaName = "gh4151"; + + private IHost _host = null!; + + public async ValueTask InitializeAsync() + { + _host = await Host.CreateDefaultBuilder() + .UseWolverine(opts => + { + opts.PersistMessagesWithPostgresql(Servers.PostgresConnectionString, SchemaName); + + // Two sticky handlers for Gh4151Message and no unsticky one, so HandlerFor(type, endpoint) + // has nothing to hand back for the queue the message actually arrives on and throws while + // the executor is being built -- before any HandlerChain exists to carry a failure policy. + opts.Discovery.DisableConventionalDiscovery() + .IncludeType(typeof(Gh4151GreenHandler)) + .IncludeType(typeof(Gh4151BlueHandler)); + + opts.LocalQueue("gh4151-durable").UseDurableInbox(); + + opts.Durability.Mode = DurabilityMode.Solo; + }).StartAsync(); + + await _host.Services.GetRequiredService().Storage.Admin.ClearAllAsync(); + } + + public async ValueTask DisposeAsync() + { + await _host.StopAsync(); + _host.Dispose(); + } + + [Fact] + public async Task the_envelope_lands_in_the_dead_letter_table() + { + await _host.TrackActivity() + .DoNotAssertOnExceptionsDetected() + .ExecuteAndWaitAsync(c => + c.EndpointFor(new Uri("local://gh4151-durable")).SendAsync(new Gh4151Message()).AsTask()); + + // The evidence the reporter could not find: a durable message that was never handled is now + // accounted for, visible, and replayable instead of silently swept. + (await countRowsAsync("wolverine_dead_letters")).ShouldBe(1); + } + + private static async Task countRowsAsync(string tableName) + { + await using var conn = new NpgsqlConnection(Servers.PostgresConnectionString); + await conn.OpenAsync(TestContext.Current.CancellationToken); + + await using var cmd = conn.CreateCommand(); + cmd.CommandText = $"select count(*) from {SchemaName}.{tableName}"; + return (long)(await cmd.ExecuteScalarAsync(TestContext.Current.CancellationToken))!; + } +} + +public record Gh4151Message; + +[StickyHandler("gh4151-green")] +public static class Gh4151GreenHandler +{ + public static void Handle(Gh4151Message message) + { + } +} + +[StickyHandler("gh4151-blue")] +public static class Gh4151BlueHandler +{ + public static void Handle(Gh4151Message message) + { + } +} diff --git a/src/Testing/CoreTests/Bugs/Bug_4151_executor_build_failure_loses_message.cs b/src/Testing/CoreTests/Bugs/Bug_4151_executor_build_failure_loses_message.cs new file mode 100644 index 000000000..1a0dd14d1 --- /dev/null +++ b/src/Testing/CoreTests/Bugs/Bug_4151_executor_build_failure_loses_message.cs @@ -0,0 +1,95 @@ +using System.Threading.Tasks; +using JasperFx.CodeGeneration; +using Microsoft.Extensions.Hosting; +using Wolverine.Attributes; +using Wolverine.Runtime.Handlers; +using Wolverine.Tracking; +using Xunit; + +namespace CoreTests.Bugs; + +// GH-4151: TypeLoadMode.Static loads pre-built handler types out of WolverineOptions.ApplicationAssembly, +// but `codegen write` emits its source into the *entry* project. When handlers live in a class library and +// the app points ApplicationAssembly at that library, the two disagree and nothing detected it -- not codegen +// write, not the build, not host start. The host booted healthy, and then StaticTypeLoader threw +// ExpectedTypeMissingException on the first dispatched message, from inside HandlerGraph.HandlerFor while the +// *executor* was being built. No HandlerChain instance exists at that point, so no failure policy could +// apply, and the pipeline's last-resort recovery simply acked the envelope away: on a durable transport the +// row was marked Handled with attempts=0 and then swept by ordinary inbox cleanup. A durable message was +// silently discarded on a configuration error, with no DLQ row and a host that stayed healthy. +// +// Two independent fixes, one per half: +// * the assembly mismatch now fails the deploy at startup instead of the first message, and +// * an envelope whose executor cannot be built is dead-lettered rather than completed -- whatever the +// reason, since every cause takes the same path. +public class Bug_4151_executor_build_failure_loses_message +{ + [Fact] + public async Task static_mode_without_pre_built_types_fails_the_host_start() + { + // No pre-built types were ever generated into this assembly, which is exactly the state the + // entry-project-vs-library split leaves a Static mode app in. + var ex = await Should.ThrowAsync(() => Host.CreateDefaultBuilder() + .UseWolverine(opts => + { + opts.ApplicationAssembly = typeof(Bug_4151_executor_build_failure_loses_message).Assembly; + opts.CodeGeneration.TypeLoadMode = TypeLoadMode.Static; + + opts.Discovery.DisableConventionalDiscovery().IncludeType(); + }).StartAsync(TestContext.Current.CancellationToken)); + + // The message has to name the chain that would have failed and the assembly that was searched, + // because "it threw on the first Ping" was the whole problem. + ex.Message.ShouldContain(nameof(Bug4151Ping)); + ex.Message.ShouldContain("CoreTests"); + } + + [Fact] + public async Task an_envelope_whose_executor_cannot_be_built_is_dead_lettered_not_completed() + { + // A sticky-handler misconfiguration is the same failure at a different origin: two sticky handlers + // and no unsticky one, so HandlerGraph.HandlerFor(type, endpoint) has nothing to hand back for any + // other endpoint and throws while the executor is being built. + using var host = await Host.CreateDefaultBuilder() + .UseWolverine(opts => + { + opts.Discovery.DisableConventionalDiscovery() + .IncludeType(typeof(Bug4151GreenHandler)) + .IncludeType(typeof(Bug4151BlueHandler)); + }).StartAsync(TestContext.Current.CancellationToken); + + var session = await host.TrackActivity() + .DoNotAssertOnExceptionsDetected() + .ExecuteAndWaitAsync(c => + c.EndpointFor(new Uri("local://maroon")).SendAsync(new Bug4151Ping()).AsTask()); + + // The envelope used to be recorded as MessageFailed only: acked away and gone, with nothing in the + // message store to distinguish it from a message that was handled successfully. + session.MovedToErrorQueue.SingleEnvelope().ShouldNotBeNull(); + } +} + +public record Bug4151Ping; + +public class Bug4151PingHandler +{ + public static void Handle(Bug4151Ping ping) + { + } +} + +[StickyHandler("bug4151-green")] +public static class Bug4151GreenHandler +{ + public static void Handle(Bug4151Ping ping) + { + } +} + +[StickyHandler("bug4151-blue")] +public static class Bug4151BlueHandler +{ + public static void Handle(Bug4151Ping ping) + { + } +} diff --git a/src/Testing/CoreTests/Configuration/environment_sensitive_configuration.cs b/src/Testing/CoreTests/Configuration/environment_sensitive_configuration.cs index 229035094..a9b205db8 100644 --- a/src/Testing/CoreTests/Configuration/environment_sensitive_configuration.cs +++ b/src/Testing/CoreTests/Configuration/environment_sensitive_configuration.cs @@ -77,6 +77,11 @@ public async Task optimized_mode_uses_prod_config_for_non_local_env() var host = await Host.CreateDefaultBuilder() .UseWolverine(opts => { + // GH-4151: TypeLoadMode.Static now asserts at startup that every handler chain's pre-built + // type is really in the application assembly. These tests only care that the profile was + // resolved, and CoreTests has no pre-generated types, so give them no chains to check. + opts.Discovery.DisableConventionalDiscovery(); + opts.Services.CritterStackDefaults(x => { // Somebody did want this, so you can actually change the name @@ -114,6 +119,11 @@ public async Task optimized_mode_uses_given_prod_config_for_non_local_env() var host = await Host.CreateDefaultBuilder() .UseWolverine(opts => { + // GH-4151: TypeLoadMode.Static now asserts at startup that every handler chain's pre-built + // type is really in the application assembly. These tests only care that the profile was + // resolved, and CoreTests has no pre-generated types, so give them no chains to check. + opts.Discovery.DisableConventionalDiscovery(); + opts.Services.CritterStackDefaults(x => { // Somebody did want this, so you can actually change the name diff --git a/src/Testing/CoreTests/Tracking/terminal_event_on_message_failure.cs b/src/Testing/CoreTests/Tracking/terminal_event_on_message_failure.cs index a038e76ca..7b52aa32e 100644 --- a/src/Testing/CoreTests/Tracking/terminal_event_on_message_failure.cs +++ b/src/Testing/CoreTests/Tracking/terminal_event_on_message_failure.cs @@ -19,6 +19,10 @@ namespace CoreTests.Tracking; // The second half compounded with WolverineRuntime.MessageFailed recording MessageEventType.Sent // instead of MessageFailed -- Sent is not terminal, it only completes once a matching Received // arrives. Both are fixed here. +// +// GH-4151 then changed which terminal record this path produces. Acking the envelope away was itself the +// bug: an executor that cannot be built will never build on a retry either, so the envelope now goes to the +// dead letter queue and records MovedToErrorQueue. Still exactly one terminal record, still no timeout. public class terminal_event_on_message_failure { [Fact] @@ -43,8 +47,8 @@ public async Task a_message_that_dies_in_the_pipeline_reaches_a_terminal_state() session.Status.ShouldBe(TrackingStatus.Completed); session.AllRecordsInOrder() - .Any(x => x.MessageEventType == MessageEventType.MessageFailed) - .ShouldBeTrue("the failed envelope should record a terminal MessageFailed event"); + .Any(x => x.MessageEventType == MessageEventType.MovedToErrorQueue) + .ShouldBeTrue("the failed envelope should record a terminal MovedToErrorQueue event"); } } diff --git a/src/Testing/CoreTests/critterstack_defaults_usage.cs b/src/Testing/CoreTests/critterstack_defaults_usage.cs index f211fb103..8e8b0af25 100644 --- a/src/Testing/CoreTests/critterstack_defaults_usage.cs +++ b/src/Testing/CoreTests/critterstack_defaults_usage.cs @@ -97,6 +97,11 @@ public async Task running_in_production_mode_2() using var host = await Host.CreateDefaultBuilder() .UseWolverine(opts => { + // GH-4151: TypeLoadMode.Static now asserts at startup that every handler chain's pre-built + // type is really in the application assembly. These tests only care that the profile was + // resolved, and CoreTests has no pre-generated types, so give them no chains to check. + opts.Discovery.DisableConventionalDiscovery(); + opts.Services.CritterStackDefaults(x => { x.Production.GeneratedCodeMode = TypeLoadMode.Static; diff --git a/src/Testing/CoreTests/respecting_jasper_fx_defaults.cs b/src/Testing/CoreTests/respecting_jasper_fx_defaults.cs index cc8e5d082..a9f87d35e 100644 --- a/src/Testing/CoreTests/respecting_jasper_fx_defaults.cs +++ b/src/Testing/CoreTests/respecting_jasper_fx_defaults.cs @@ -29,6 +29,11 @@ public async Task use_jasper_fx_defaults() using var host = await Host.CreateDefaultBuilder() .UseWolverine(opts => { + // GH-4151: TypeLoadMode.Static now asserts at startup that every handler chain's pre-built + // type is really in the application assembly. This test only cares that the option was + // propagated, and CoreTests has no pre-generated types, so give it no chains to check. + opts.Discovery.DisableConventionalDiscovery(); + opts.Services.CritterStackDefaults(cr => { cr.ServiceName = "Special"; diff --git a/src/Wolverine/Runtime/HandlerPipeline.cs b/src/Wolverine/Runtime/HandlerPipeline.cs index fd3dfce3b..4477075b4 100644 --- a/src/Wolverine/Runtime/HandlerPipeline.cs +++ b/src/Wolverine/Runtime/HandlerPipeline.cs @@ -372,7 +372,25 @@ private async Task executeAsync(MessageContext context, Envelope } } - var executor = _executors[envelope.Message!.GetType()]; + IExecutor executor; + try + { + executor = _executors[envelope.Message!.GetType()]; + } + catch (Exception e) + { + // GH-4151: building the executor happens *before* any HandlerChain instance exists, so there + // is no chain.Failures for a failure policy to hang off of -- not even a MoveToErrorQueue rule + // written specifically for this message type. Without this catch the exception escapes to + // InvokeAsync's last line of defense, which acks the envelope away: on a durable transport the + // row is marked Handled with attempts=0 and then swept by ordinary inbox cleanup, so a + // configuration error silently discards durable messages with no DLQ row and a healthy host. + // Every cause takes this same path -- a missing pre-built type in TypeLoadMode.Static, a chain + // that will not compile, a sticky-handler misconfiguration -- and none of them will ever + // succeed on a retry, so the dead letter queue is where the envelope belongs. + activity?.SetStatus(ActivityStatusCode.Error, e.GetType().Name); + return new MoveToErrorQueue(e); + } return await executor.ExecuteAsync(context, _cancellation).ConfigureAwait(false); } diff --git a/src/Wolverine/Runtime/Handlers/HandlerGraph.PreBuiltTypes.cs b/src/Wolverine/Runtime/Handlers/HandlerGraph.PreBuiltTypes.cs new file mode 100644 index 000000000..3e5bebe4d --- /dev/null +++ b/src/Wolverine/Runtime/Handlers/HandlerGraph.PreBuiltTypes.cs @@ -0,0 +1,141 @@ +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using System.Text; +using JasperFx.CodeGeneration; + +namespace Wolverine.Runtime.Handlers; + +public partial class HandlerGraph +{ + /// + /// GH-4151. In there is no fallback: a handler chain whose + /// pre-generated type is not in the application assembly can never be executed. Until now the miss + /// was only discovered lazily, on the first message of that type, from inside + /// while the executor was being built -- too late for any failure + /// policy and too late for a deploy to be rolled back. Attach every expected type up front instead, + /// so the misconfiguration is a failed start rather than a per-message loss. Attaching is not merely + /// a probe: the types the loader would otherwise resolve one message type at a time are resolved + /// here, which is what Static mode wanted in the first place. + /// + internal void AssertPreBuiltTypesExist(WolverineOptions options) + { + if (options.CodeGeneration.TypeLoadMode != TypeLoadMode.Static) + { + return; + } + + // `codegen write` runs against an application whose types do not exist yet -- that is the point of + // running it. Same guard as shouldConsumeStaticRegistry. + if (DynamicCodeBuilder.WithinCodegenCommand) + { + return; + } + + var applicationAssembly = options.CodeGeneration.ApplicationAssembly; + if (applicationAssembly == null) + { + return; + } + + var collection = (ICodeFileCollection)this; + var containingNamespace = collection.ToNamespace(options.CodeGeneration); + + var missing = new List(); + foreach (var file in collection.BuildFiles()) + { + // The pre-generated HandlerRegistry is deliberately not fatal. Its absence only costs the + // cold-start optimization -- compileWithRuntimeScanning already warns and falls back to an + // assembly scan -- and no message is lost over it. + if (file is HandlerRegistryCodeFile) + { + continue; + } + + if (!file.AttachTypesSynchronously(options.CodeGeneration, applicationAssembly, Container.Services, + containingNamespace)) + { + missing.Add(file); + } + } + + if (missing.Count == 0) + { + return; + } + + throw new MissingPreBuiltTypesException(describeMissingTypes(options, applicationAssembly, missing)); + } + + private static string describeMissingTypes(WolverineOptions options, Assembly applicationAssembly, + List missing) + { + var message = new StringBuilder(); + + message.AppendLine( + $"Wolverine is running in {nameof(TypeLoadMode)}.{nameof(TypeLoadMode.Static)}, but {missing.Count} expected pre-built handler type(s) could not be loaded from the configured {nameof(WolverineOptions.ApplicationAssembly)} '{applicationAssembly.GetName().Name}':"); + + foreach (var file in missing) + { + message.AppendLine(" * " + file); + } + + message.AppendLine(); + + var elsewhere = findAssemblyHoldingGeneratedTypes(options, applicationAssembly); + if (elsewhere != null) + { + message.AppendLine( + $"Pre-generated Wolverine types were found in '{elsewhere.GetName().Name}' instead. 'dotnet run -- codegen write' emits its source into the entry project, while {nameof(TypeLoadMode)}.{nameof(TypeLoadMode.Static)} loads pre-built types from {nameof(WolverineOptions)}.{nameof(WolverineOptions.ApplicationAssembly)}, so the two disagree."); + message.AppendLine( + $"If {nameof(WolverineOptions.ApplicationAssembly)} was only set so that Wolverine would discover handlers living in another assembly, use opts.Discovery.{nameof(Configuration.HandlerDiscovery.IncludeAssembly)}(...) for that instead and leave {nameof(WolverineOptions.ApplicationAssembly)} as the entry assembly. Otherwise, point the generated code output at the project that builds '{applicationAssembly.GetName().Name}' with opts.CodeGeneration.{nameof(GenerationRules.GeneratedCodeOutputPath)}."); + } + else + { + message.AppendLine( + "No pre-generated Wolverine types could be found in any assembly this application is using. Run 'dotnet run -- codegen write' as part of the build and compile its output into the application assembly, or run in TypeLoadMode.Auto."); + } + + message.AppendLine(); + message.Append("See https://wolverinefx.net/guide/codegen.html"); + + return message.ToString(); + } + + // Purely a diagnostic for the exception message above: say where the generated code actually landed, + // because "the types are missing" and "the types are in the other assembly" call for different fixes. + // The generated HandlerRegistry is the marker -- codegen write always emits exactly one, alongside the + // handler types, into whichever project it wrote to. + [UnconditionalSuppressMessage("Trimming", "IL2026", + Justification = + "ExportedTypes walk over candidate assemblies to locate misplaced codegen output; runs only on the startup failure path while building an exception message. See AOT guide.")] + private static Assembly? findAssemblyHoldingGeneratedTypes(WolverineOptions options, Assembly applicationAssembly) + { + var candidates = new List(); + + var entryAssembly = Assembly.GetEntryAssembly(); + if (entryAssembly != null) + { + candidates.Add(entryAssembly); + } + + candidates.AddRange(options.Assemblies); + + foreach (var candidate in candidates.Distinct().Where(x => x != applicationAssembly && !x.IsDynamic)) + { + try + { + if (candidate.ExportedTypes.Any(x => x.Name == HandlerRegistry.GeneratedTypeName)) + { + return candidate; + } + } + catch (Exception) + { + // A candidate assembly that cannot be reflected over simply is not a candidate. This probe + // exists to make an exception message more useful and must never become the thing that fails. + } + } + + return null; + } +} diff --git a/src/Wolverine/Runtime/Handlers/MissingPreBuiltTypesException.cs b/src/Wolverine/Runtime/Handlers/MissingPreBuiltTypesException.cs new file mode 100644 index 000000000..1c16d8a53 --- /dev/null +++ b/src/Wolverine/Runtime/Handlers/MissingPreBuiltTypesException.cs @@ -0,0 +1,15 @@ +namespace Wolverine.Runtime.Handlers; + +/// +/// Thrown at startup when Wolverine is running in +/// but one or more of the expected pre-generated handler types cannot be loaded out of +/// . Before GH-4151 this state let the host start up +/// healthy and then threw on the first message of each affected type, from a place in the pipeline where +/// no failure policy could apply. +/// +public class MissingPreBuiltTypesException : Exception +{ + public MissingPreBuiltTypesException(string message) : base(message) + { + } +} diff --git a/src/Wolverine/Runtime/WolverineRuntime.HostService.cs b/src/Wolverine/Runtime/WolverineRuntime.HostService.cs index 2411187c6..67e826e56 100644 --- a/src/Wolverine/Runtime/WolverineRuntime.HostService.cs +++ b/src/Wolverine/Runtime/WolverineRuntime.HostService.cs @@ -141,6 +141,13 @@ public async Task StartAsync(CancellationToken cancellationToken) // which is what decides whether a topology was found. warnOrAssertUnsequencedBatchExecution(); + // GH-4151. Last of the chain-shaping steps, so every chain that will ever exist -- including the + // batch chains just moved above -- is checked. In TypeLoadMode.Static a missing pre-built type + // used to surface on the first message of that type, from inside executor construction where no + // failure policy can reach it. Fail the deploy here instead, before storage migration or any + // listener starts. + Handlers.AssertPreBuiltTypesExist(Options); + // Pre-populate the message-type-name cache so the per-message ToMessageTypeName() // hot path inside Envelope construction never pays the first-occurrence reflection // cost (attribute reads, interface walks, generic-type pretty-printing).