gRPC #2525 follow-up: Correctness Gaps

docs/guide/grpc/handlers.md

@@ -80,12 +80,17 @@ Both paths feed into the same generated-code pipeline used by Wolverine's messag
 adapters, so your gRPC services show up in the standard diagnostics:
 
 ```bash
+# List every handler / HTTP endpoint / gRPC service Wolverine knows about
 dotnet run -- describe
-dotnet run -- describe-routing
+
+# Preview the generated wrapper for one proto-first gRPC stub
+dotnet run -- wolverine-diagnostics codegen-preview --grpc Greeter
 ```
 
-If you're debugging discovery, those commands will show the generated handler type name and the
-handler method it forwards to.
+If you're debugging discovery, `describe` proves Wolverine found the stub; `codegen-preview --grpc`
+shows the exact generated override and the handler method each RPC forwards to. See
+[`codegen-preview`](/guide/command-line#codegen-preview) for the full set of accepted identifiers
+(bare proto service name, stub class name, or short `-g` alias).
 
 ## Observability
 

docs/guide/grpc/index.md

@@ -1,9 +1,8 @@
 # gRPC Services with Wolverine
 
 ::: info
-The `WolverineFx.Grpc` package (experimental, shipping alongside gRPC support on the
-`feature/grpc-and-streaming-support` branch) lets you expose Wolverine handlers as
-ASP.NET Core gRPC services with minimal wiring. It supports both the **code-first**
+The `WolverineFx.Grpc` package lets you expose Wolverine handlers as ASP.NET Core gRPC services
+with minimal wiring. It supports both the **code-first**
 ([protobuf-net.Grpc](https://protobuf-net.github.io/protobuf-net.Grpc/)) and **proto-first**
 ([Grpc.Tools](https://learn.microsoft.com/en-us/aspnet/core/grpc/)) styles.
 :::
@@ -116,41 +115,31 @@ and comparisons to the official `grpc-dotnet` examples.
   through `Bus.StreamAsync<TResp>(item, ct)` — see [Streaming](./streaming) for the pattern and the
   [RacerWithGrpc](https://github.com/JasperFx/wolverine/tree/main/src/Samples/RacerWithGrpc) sample.
 - **Exception mapping** of the canonical `Exception → StatusCode` table is not yet user-configurable
-  (follow-up item). Rich, structured responses are already available — see
-  [Error Handling](./errors).
+  on the server side (follow-up item). Rich, structured responses are already available — see
+  [Error Handling](./errors). On the client side, `WolverineGrpcClientOptions.MapRpcException`
+  already allows per-client overrides — see [Typed gRPC Clients](./client#per-client-override).
+- **`MiddlewareScoping.Grpc` middleware** — the enum value ships and is honored by Wolverine's
+  discovery primitives, but no code path yet *weaves* `[WolverineBefore(MiddlewareScoping.Grpc)]`
+  / `[WolverineAfter(MiddlewareScoping.Grpc)]` methods into the generated gRPC service wrappers.
+  The attribute is safe to apply — it compiles, it is correctly filtered away from message-handler
+  and HTTP chains, and it will start firing once the codegen path (tracked as M15) lands — but
+  today nothing runs at RPC time. Until then, middleware that needs to execute on gRPC calls
+  should live in a custom gRPC interceptor rather than rely on the attribute or on
+  `services.AddWolverineGrpc(g => g.AddMiddleware<T>())` (both take effect together in M15).
 
 ## Roadmap
 
-The gRPC integration is intentionally shipping as a focused, reviewable slice. The items below are
-on the roadmap but *not* in the initial drop — they're listed here so contributors can plan around
-them and consumers know what's coming.
-
-### Shipping in this PR
-
-- **`MiddlewareScoping.Grpc`** — the existing [scoped middleware](/guide/handlers/middleware#applying-middleware-explicitly-by-attribute)
-  enum grows a `Grpc` value so gRPC-specific middleware can be registered the same way HTTP and
-  messaging middleware already are. Previously, gRPC service chains reported themselves as
-  `MessageHandlers`-scoped, which silently over-attached message middleware to gRPC calls. This is
-  a behavior correction, not an additive feature.
-- **`codegen-preview --grpc`** — the [`codegen-preview` CLI](/guide/command-line#codegen-preview)
-  grows a `--grpc` / `-g` flag (mirroring `--handler` / `-h` and `--route` / `-r`) so you can inspect
-  the code Wolverine generates for gRPC service chains without dropping into the full `codegen write`
-  output.
-- **Typed gRPC client extension (`AddWolverineGrpcClient<T>()`)** — a thin Wolverine wrapper over
-  `Grpc.Net.ClientFactory.AddGrpcClient<T>()` that layers envelope-header propagation and
-  `RpcException` → typed .NET exception translation onto both code-first (`protobuf-net.Grpc`
-  `[ServiceContract]`) and proto-first (`Grpc.Tools`-generated) typed clients. See
-  [Typed gRPC Clients](./client) for the full surface. Raw `GrpcChannel` + generated stubs (as used
-  in the samples today) remain a fully supported path.
-
-### Deferred to follow-up PRs
+The gRPC integration has a handful of deferred items that are known-good fits but haven't shipped
+yet. They're listed here so contributors can plan around them and consumers know what's coming.
 
+- **`MiddlewareScoping.Grpc` codegen weaving (M15)** — attribute-based middleware on gRPC stubs
+  (see Current Limitations above). Phase 0 landed the discovery + options surface; Phase 1 will
+  wire execution into the generated `GrpcServiceChain` wrappers.
 - **`Validate` convention → `Status?`** — HTTP handlers already support an opt-in `Validate` method
   whose non-null return short-circuits the call. The gRPC equivalent would return
   `Grpc.Core.Status?` (or a richer `google.rpc.Status`) so a handler could express "this call is
-  invalid, return `InvalidArgument` with these field violations" without throwing. This is
-  deferred because it lands cleanest on top of the code-first codegen work below — shipping it
-  against the current runtime path would bake in assumptions we'll want to revisit.
+  invalid, return `InvalidArgument` with these field violations" without throwing. Deferred because
+  it lands cleanest on top of the code-first codegen work below.
 - **Code-first codegen parity** — proto-first services flow through a generated `GrpcServiceChain`
   with the usual JasperFx codegen pipeline; code-first services (the `WolverineGrpcServiceBase` path)
   currently resolve dependencies via service location inside each method. Generating per-method code

docs/guide/grpc/streaming.md

@@ -122,7 +122,10 @@ but your detached tasks keep running. Always thread the token through.
 - **Exception timing:** an exception thrown **before** the first `yield return` surfaces on the
   client via the trailers as expected. An exception thrown **mid-stream** surfaces as a trailer
   after messages the client has already received — well-behaved clients must still check the final
-  status even after consuming messages successfully.
+  status even after consuming messages successfully. Server-side, the OpenTelemetry activity for
+  the handler is marked `Error` in both cases (including cancellation) — the activity stays open
+  until the stream fully drains or faults, so dashboards reflect the real terminal state rather
+  than the moment the handler returned the `IAsyncEnumerable<T>`.
 
 ## Related
 

src/Testing/CoreTests/Acceptance/streaming_handler_support.cs

@@ -1,3 +1,4 @@
+using System.Diagnostics;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
 using Shouldly;
@@ -201,6 +202,37 @@ public async Task handler_exception_after_partial_yield_surfaces_to_caller_with_
         items.Select(i => i.Value).ShouldBe([0, 1]);
     }
 
+    [Fact]
+    public async Task mid_stream_throw_marks_activity_status_error()
+    {
+        var capturedActivities = new List<Activity>();
+        using var listener = new ActivityListener
+        {
+            ShouldListenTo = source => source.Name == "Wolverine",
+            Sample = (ref ActivityCreationOptions<ActivityContext> _) => ActivitySamplingResult.AllDataAndRecorded,
+            ActivityStopped = activity => capturedActivities.Add(activity)
+        };
+        ActivitySource.AddActivityListener(listener);
+
+        using var host = await Host.CreateDefaultBuilder()
+            .UseWolverine()
+            .StartAsync();
+
+        var bus = host.Services.GetRequiredService<IMessageBus>();
+
+        await Should.ThrowAsync<InvalidOperationException>(async () =>
+        {
+            await foreach (var _ in bus.StreamAsync<StreamItem>(new FaultingStreamRequest(2)))
+            {
+            }
+        });
+
+        var streamingActivity = capturedActivities
+            .FirstOrDefault(a => a.OperationName.Contains("stream", StringComparison.OrdinalIgnoreCase));
+        streamingActivity.ShouldNotBeNull();
+        streamingActivity.Status.ShouldBe(ActivityStatusCode.Error);
+    }
+
     [Fact]
     public async Task stream_with_delivery_options()
     {

src/Wolverine.Grpc.Tests/GrpcMiddlewareScoping/GreetMessageHandler.cs

@@ -0,0 +1,29 @@
+using Wolverine.Grpc.Tests.GrpcMiddlewareScoping.Generated;
+
+namespace Wolverine.Grpc.Tests.GrpcMiddlewareScoping;
+
+/// <summary>
+///     Wolverine handler for the unary RPC. Records its invocation against the shared
+///     <see cref="MiddlewareInvocationSink"/> so middleware-ordering tests can assert
+///     before/after relative to the handler call.
+/// </summary>
+public static class GreetMessageHandler
+{
+    public const string Marker = "Handler";
+
+    public static GreetReply Handle(GreetRequest request, MiddlewareInvocationSink sink)
+    {
+        sink.Record(Marker);
+        return new GreetReply { Message = $"Hello, {request.Name}" };
+    }
+
+    public static async IAsyncEnumerable<GreetReply> Handle(GreetManyRequest request, MiddlewareInvocationSink sink)
+    {
+        sink.Record(Marker);
+        for (var i = 0; i < 3; i++)
+        {
+            yield return new GreetReply { Message = $"Hello #{i}, {request.Name}" };
+            await Task.Yield();
+        }
+    }
+}

src/Wolverine.Grpc.Tests/GrpcMiddlewareScoping/GreeterMiddlewareTestStub.ScopeProbes.cs

@@ -0,0 +1,36 @@
+using Wolverine.Attributes;
+
+namespace Wolverine.Grpc.Tests.GrpcMiddlewareScoping;
+
+/// <summary>
+///     Probe methods used by <c>scope_discovery_tests</c> to verify that
+///     <see cref="GrpcServiceChain.DiscoveredBefores"/> / <see cref="GrpcServiceChain.DiscoveredAfters"/>
+///     respect <see cref="MiddlewareScoping"/>. Lives on the smoke stub via <c>partial</c> so tests
+///     don't need a second proto to exercise the discovery path. These are inert under Phase 0
+///     (no weaving yet); when Phase 1 lands, they'll be the first concrete demonstration that the
+///     M15 promise (middleware fires alongside the gRPC handler) actually holds.
+/// </summary>
+public abstract partial class GreeterMiddlewareTestStub
+{
+    public const string AnywhereMarker = "ScopeProbe.Anywhere";
+    public const string GrpcMarker = "ScopeProbe.Grpc";
+    public const string MessageHandlersMarker = "ScopeProbe.MessageHandlers";
+
+    [WolverineBefore]
+    public static void BeforeAnywhere(MiddlewareInvocationSink sink) => sink.Record(AnywhereMarker);
+
+    [WolverineBefore(MiddlewareScoping.Grpc)]
+    public static void BeforeGrpc(MiddlewareInvocationSink sink) => sink.Record(GrpcMarker);
+
+    [WolverineBefore(MiddlewareScoping.MessageHandlers)]
+    public static void BeforeMessageHandlers(MiddlewareInvocationSink sink) => sink.Record(MessageHandlersMarker);
+
+    [WolverineAfter]
+    public static void AfterAnywhere(MiddlewareInvocationSink sink) => sink.Record(AnywhereMarker + ".After");
+
+    [WolverineAfter(MiddlewareScoping.Grpc)]
+    public static void AfterGrpc(MiddlewareInvocationSink sink) => sink.Record(GrpcMarker + ".After");
+
+    [WolverineAfter(MiddlewareScoping.MessageHandlers)]
+    public static void AfterMessageHandlers(MiddlewareInvocationSink sink) => sink.Record(MessageHandlersMarker + ".After");
+}

src/Wolverine.Grpc.Tests/GrpcMiddlewareScoping/GreeterMiddlewareTestStub.cs

@@ -0,0 +1,13 @@
+using Wolverine.Grpc.Tests.GrpcMiddlewareScoping.Generated;
+
+namespace Wolverine.Grpc.Tests.GrpcMiddlewareScoping;
+
+/// <summary>
+///     Proto-first Wolverine stub for the M15 (<see cref="Wolverine.Attributes.MiddlewareScoping.Grpc"/>)
+///     test suite. Intentionally bare — middleware methods carrying
+///     <c>[WolverineBefore]</c>/<c>[WolverineAfter]</c> are added per-test via partial-class
+///     extensions or test-specific subclasses so each scenario can scope its assertions
+///     without polluting the shared stub.
+/// </summary>
+[WolverineGrpcService]
+public abstract partial class GreeterMiddlewareTestStub : GreeterMiddlewareTest.GreeterMiddlewareTestBase;

src/Wolverine.Grpc.Tests/GrpcMiddlewareScoping/MiddlewareInvocationSink.cs

@@ -0,0 +1,30 @@
+using System.Collections.Concurrent;
+
+namespace Wolverine.Grpc.Tests.GrpcMiddlewareScoping;
+
+/// <summary>
+///     Thread-safe append-only ledger of named events that the M15 integration tests use to
+///     assert middleware ordering and scope filtering. Each captured entry records the marker
+///     name supplied by a test fixture (typically a stub-class method name like
+///     <c>"BeforeGrpc"</c> or a handler name like <c>"Handler"</c>) so the test can later
+///     assert what fired and in what order without coupling to clock timing.
+/// </summary>
+public sealed class MiddlewareInvocationSink
+{
+    private readonly ConcurrentQueue<string> _events = new();
+
+    public void Record(string marker) => _events.Enqueue(marker);
+
+    public IReadOnlyList<string> Events => _events.ToArray();
+
+    public void Clear()
+    {
+        while (_events.TryDequeue(out _))
+        {
+        }
+    }
+
+    public bool Contains(string marker) => _events.Contains(marker);
+
+    public int CountOf(string marker) => _events.Count(e => e == marker);
+}

src/Wolverine.Grpc.Tests/GrpcMiddlewareScoping/MiddlewareScopingFixture.cs

@@ -0,0 +1,73 @@
+using Grpc.Net.Client;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.TestHost;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Wolverine.Grpc.Tests.GrpcMiddlewareScoping.Generated;
+using Xunit;
+
+namespace Wolverine.Grpc.Tests.GrpcMiddlewareScoping;
+
+/// <summary>
+///     Dedicated host fixture for the M15 <c>MiddlewareScoping.Grpc</c> integration tests.
+///     Booted as a per-class fixture (not collection-shared) so each test class can verify
+///     middleware-invocation ordering against a fresh <see cref="MiddlewareInvocationSink"/>
+///     without inter-test interference.
+/// </summary>
+/// <remarks>
+///     Modeled on <see cref="GrpcTestFixture"/> but isolated from the PingPong/Streaming/Faulting
+///     samples so M15 assertions don't drift when those samples evolve. Uses ASP.NET Core's
+///     <c>TestHost</c> for an in-process channel — no real network port.
+/// </remarks>
+public class MiddlewareScopingFixture : IAsyncLifetime
+{
+    private WebApplication? _app;
+    public GrpcChannel? Channel { get; private set; }
+    public MiddlewareInvocationSink Sink { get; } = new();
+
+    public async Task InitializeAsync()
+    {
+        var builder = WebApplication.CreateBuilder([]);
+
+        builder.WebHost.UseTestServer();
+
+        builder.Host.UseWolverine(opts =>
+        {
+            opts.ApplicationAssembly = typeof(MiddlewareScopingFixture).Assembly;
+        });
+
+        builder.Services.AddSingleton(Sink);
+        builder.Services.AddGrpc();
+        builder.Services.AddWolverineGrpc();
+
+        _app = builder.Build();
+
+        _app.UseRouting();
+
+        // Trigger Wolverine's proto-first discovery + code-gen and register the generated
+        // wrapper. Pre-M15 weaving, this just emits forward-frames; once §7.3 lands the same
+        // generated code will additionally carry middleware/postprocessor frames per RPC.
+        _app.MapWolverineGrpcServices();
+
+        await _app.StartAsync();
+
+        var handler = _app.GetTestServer().CreateHandler();
+        Channel = GrpcChannel.ForAddress("http://localhost", new GrpcChannelOptions
+        {
+            HttpHandler = handler
+        });
+    }
+
+    public async Task DisposeAsync()
+    {
+        Channel?.Dispose();
+        if (_app != null)
+        {
+            await _app.StopAsync();
+            await _app.DisposeAsync();
+        }
+    }
+
+    public GreeterMiddlewareTest.GreeterMiddlewareTestClient CreateClient()
+        => new(Channel!);
+}

src/Wolverine.Grpc.Tests/GrpcMiddlewareScoping/Protos/middleware_scoping_test.proto

@@ -0,0 +1,29 @@
+syntax = "proto3";
+
+option csharp_namespace = "Wolverine.Grpc.Tests.GrpcMiddlewareScoping.Generated";
+
+package wolverine.grpc.tests.middleware_scoping;
+
+// Test-only service used by middleware-scoping tests. Two RPC shapes (unary + server-streaming)
+// so the same fixture exercises the per-RPC weaving paths from §7.3 of the M15 plan.
+service GreeterMiddlewareTest {
+  // Unary: maps to Wolverine via IMessageBus.InvokeAsync<GreetReply>.
+  rpc Greet (GreetRequest) returns (GreetReply);
+
+  // Server-streaming: maps to Wolverine via IMessageBus.StreamAsync<GreetReply>. Uses a
+  // distinct request type because Wolverine dispatches handlers by message type, so the
+  // unary and streaming RPCs cannot share GreetRequest.
+  rpc GreetMany (GreetManyRequest) returns (stream GreetReply);
+}
+
+message GreetRequest {
+  string name = 1;
+}
+
+message GreetManyRequest {
+  string name = 1;
+}
+
+message GreetReply {
+  string message = 1;
+}