GH-4151: a Static-mode assembly mismatch fails the deploy, and a message that cannot be executed is dead-lettered

docs/guide/codegen.md

@@ -327,6 +327,26 @@ using var host = Host.CreateDefaultBuilder()
 <sup><a href='https://github.com/JasperFx/wolverine/blob/main/src/Samples/DocumentationSamples/BootstrappingSamples.cs#L10-L20' title='Snippet source file'>snippet source</a> | <a href='#snippet-sample_overriding_application_assembly' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
+::: 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 `<Exclude />` 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:
 

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<IWolverineRuntime>().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<long> 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)
+    {
+    }
+}

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<MissingPreBuiltTypesException>(() => Host.CreateDefaultBuilder()
+            .UseWolverine(opts =>
+            {
+                opts.ApplicationAssembly = typeof(Bug_4151_executor_build_failure_loses_message).Assembly;
+                opts.CodeGeneration.TypeLoadMode = TypeLoadMode.Static;
+
+                opts.Discovery.DisableConventionalDiscovery().IncludeType<Bug4151PingHandler>();
+            }).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<Bug4151Ping>().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)
+    {
+    }
+}

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

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");
     }
 }
 

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;

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";

src/Wolverine/Runtime/HandlerPipeline.cs

@@ -372,7 +372,25 @@ private async Task<IContinuation> 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);
     }