feat(batching): warn/assert on a direct handler shadowing a batch handler (GH-3289 Phase 2)

docs/guide/handlers/batching.md

@@ -141,6 +141,14 @@ By default Wolverine assumes the batch handler is the *only* consumer of the ele
 is always routed straight to the batch. If you *also* declare a direct `Handle(Item)` handler alongside
 `BatchMessagesOf<Item>()`, the direct handler wins and the batch is silently shadowed -- the batched handler never runs.
 
+::: warning
+Because that shadowing is easy to miss, Wolverine logs a loud **warning at startup** whenever a message type has
+both a direct `Handle(T)` handler and a `BatchMessagesOf<T>()` batch handler under the default
+`ClassicCombineIntoOneLogicalHandler` mode, naming both handlers and pointing you at `MultipleHandlerBehavior.Separated`.
+If you would rather this configuration be a hard error, call `opts.AssertNoBatchHandlerConflicts()` and Wolverine will
+throw at startup instead of warning.
+:::
+
 The one exception is `MultipleHandlerBehavior.Separated`. Under that mode Wolverine treats the per-message handler and
 the batched handler as two independent consumers of `Item`, so **both** run for every `Item`:
 

src/Testing/CoreTests/Acceptance/batch_handler_conflict_diagnostic.cs

@@ -0,0 +1,117 @@
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+using Wolverine;
+using Xunit;
+
+namespace CoreTests.Acceptance;
+
+public class batch_handler_conflict_diagnostic
+{
+    private static IHostBuilder buildHost(Action<WolverineOptions> configure, CapturingLogger? logger = null)
+    {
+        return Host.CreateDefaultBuilder()
+            .UseWolverine(opts =>
+            {
+                opts.Discovery.DisableConventionalDiscovery();
+                if (logger != null)
+                {
+                    opts.Services.AddSingleton<ILoggerFactory>(new SingleLoggerFactory(logger));
+                }
+
+                configure(opts);
+            });
+    }
+
+    [Fact]
+    public async Task warns_by_default_when_a_direct_handler_shadows_a_batch_in_classic_mode()
+    {
+        var logger = new CapturingLogger();
+
+        using var host = await buildHost(opts =>
+        {
+            // default MultipleHandlerBehavior is ClassicCombineIntoOneLogicalHandler
+            opts.Discovery.IncludeType<ConflictDirectHandler>();
+            opts.Discovery.IncludeType<ConflictBatchHandler>();
+            opts.BatchMessagesOf<ConflictMessage>();
+        }, logger).StartAsync();
+
+        logger.Entries.ShouldContain(x =>
+            x.Level == LogLevel.Warning && x.Message.Contains("Batch handler conflict"));
+
+        var warning = logger.Entries
+            .Single(x => x.Level == LogLevel.Warning && x.Message.Contains("Batch handler conflict"));
+
+        // Names both roles and points at the fix
+        warning.Message.ShouldContain(nameof(ConflictDirectHandler));
+        warning.Message.ShouldContain("MultipleHandlerBehavior.Separated");
+    }
+
+    [Fact]
+    public async Task throws_when_opted_in_via_AssertNoBatchHandlerConflicts()
+    {
+        var ex = await Should.ThrowAsync<InvalidOperationException>(async () =>
+        {
+            using var host = await buildHost(opts =>
+            {
+                opts.Discovery.IncludeType<ConflictDirectHandler>();
+                opts.Discovery.IncludeType<ConflictBatchHandler>();
+                opts.BatchMessagesOf<ConflictMessage>();
+                opts.AssertNoBatchHandlerConflicts();
+            }).StartAsync();
+        });
+
+        ex.Message.ShouldContain("Batch handler conflict");
+    }
+
+    [Fact]
+    public async Task no_conflict_when_only_a_batch_handler_exists()
+    {
+        // A BatchMessagesOf<T> with no direct Handle(T) is the normal case - must not warn or throw.
+        var logger = new CapturingLogger();
+
+        using var host = await buildHost(opts =>
+        {
+            opts.Discovery.IncludeType<ConflictBatchHandler>();
+            opts.BatchMessagesOf<ConflictMessage>();
+            opts.AssertNoBatchHandlerConflicts();
+        }, logger).StartAsync();
+
+        logger.Entries.ShouldNotContain(x => x.Message.Contains("Batch handler conflict"));
+    }
+
+    [Fact]
+    public async Task no_conflict_under_separated_mode_even_with_both_handlers()
+    {
+        // Separated legitimately runs both handlers (the batch is moved to its own queue), so the
+        // conflict diagnostic must stay silent even when opted in to throwing.
+        var logger = new CapturingLogger();
+
+        using var host = await buildHost(opts =>
+        {
+            opts.MultipleHandlerBehavior = MultipleHandlerBehavior.Separated;
+            opts.Discovery.IncludeType<ConflictDirectHandler>();
+            opts.Discovery.IncludeType<ConflictBatchHandler>();
+            opts.BatchMessagesOf<ConflictMessage>();
+            opts.AssertNoBatchHandlerConflicts();
+        }, logger).StartAsync();
+
+        logger.Entries.ShouldNotContain(x => x.Message.Contains("Batch handler conflict"));
+    }
+}
+
+public record ConflictMessage(string Name);
+
+public class ConflictDirectHandler
+{
+    public void Handle(ConflictMessage message)
+    {
+    }
+}
+
+public class ConflictBatchHandler
+{
+    public void Handle(ConflictMessage[] messages)
+    {
+    }
+}

src/Wolverine/Runtime/WolverineRuntime.HostService.cs

@@ -107,6 +107,11 @@ public async Task StartAsync(CancellationToken cancellationToken)
             // the new queue still receives the durable/local-queue endpoint policies.
             reassignBatchQueuesThatCollideWithHandlers();
 
+            // Under the DEFAULT Classic behavior the same collision is NOT resolved: the direct
+            // Handle(T) handler wins and the BatchMessagesOf<T>() batch handler is silently shadowed.
+            // Warn loudly (or throw, if opted in) so the shadowing is not a silent surprise. GH-3289.
+            warnOrAssertBatchHandlerConflicts();
+
             // 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).
@@ -600,6 +605,50 @@ private void reassignBatchQueuesThatCollideWithHandlers()
         }
     }
 
+    private void warnOrAssertBatchHandlerConflicts()
+    {
+        // Only relevant under the default Classic behavior. Under Separated the same collision is
+        // legitimately resolved by reassignBatchQueuesThatCollideWithHandlers() (both handlers run).
+        if (Options.MultipleHandlerBehavior != MultipleHandlerBehavior.ClassicCombineIntoOneLogicalHandler)
+        {
+            return;
+        }
+
+        if (Options.BatchDefinitions.Count == 0)
+        {
+            return;
+        }
+
+        foreach (var batch in Options.BatchDefinitions)
+        {
+            // A non-null chain for the element type itself means there is a direct Handle(T) handler
+            // colliding with the batch (the batch handler is for T[], a different chain).
+            var directChain = Handlers.ChainFor(batch.ElementType);
+            if (directChain == null)
+            {
+                continue;
+            }
+
+            var elementType = batch.ElementType.NameInCode();
+            var directHandlers = directChain.Handlers
+                .Select(x => $"{x.HandlerType.NameInCode()}.{x.Method.Name}()").Join(", ");
+
+            var message =
+                $"Batch handler conflict for message type '{batch.ElementType.FullNameInCode()}': it has BOTH a direct handler ({directHandlers}) " +
+                $"and a BatchMessagesOf<{elementType}>() batch handler ({elementType}[]). Under the default " +
+                $"MultipleHandlerBehavior.ClassicCombineIntoOneLogicalHandler the direct handler wins and the batch handler is silently " +
+                $"shadowed (it never runs). To run both independently set opts.MultipleHandlerBehavior = MultipleHandlerBehavior.Separated; " +
+                $"otherwise remove one of the two handlers. (Call opts.AssertNoBatchHandlerConflicts() to make Wolverine throw on this instead of warning.)";
+
+            if (Options.AssertsNoBatchHandlerConflicts)
+            {
+                throw new InvalidOperationException(message);
+            }
+
+            Logger.LogWarning(message);
+        }
+    }
+
     private void discoverListenersFromConventions()
     {
         // Let any registered routing conventions discover listener endpoints

src/Wolverine/WolverineOptions.Batching.cs

@@ -8,6 +8,21 @@ public sealed partial class WolverineOptions
 {
     internal List<BatchingOptions> BatchDefinitions { get; } = new();
 
+    internal bool AssertsNoBatchHandlerConflicts { get; private set; }
+
+    /// <summary>
+    /// Make Wolverine throw at startup (instead of only logging a warning) if a message type has both a
+    /// direct <c>Handle(T)</c> handler and a <c>BatchMessagesOf&lt;T&gt;()</c> batch handler under the
+    /// default <see cref="MultipleHandlerBehavior.ClassicCombineIntoOneLogicalHandler"/> mode — a
+    /// configuration in which the direct handler wins and the batch handler is silently shadowed. Has
+    /// no effect under <see cref="MultipleHandlerBehavior.Separated"/>, where both handlers legitimately
+    /// run and Wolverine moves the batch onto its own queue.
+    /// </summary>
+    public void AssertNoBatchHandlerConflicts()
+    {
+        AssertsNoBatchHandlerConflicts = true;
+    }
+
     /// <summary>
     /// Configure batch processing of an incoming (or local) message type. Note that Wolverine
     /// will require you to use the Array of that element type for the actual batch handler