GH-4191: unwrap before testing for a faulted receiver, and stop restacking the interceptor

[jeremydmiller]

Closes #4191. Follows #4190.

Three defects in ListeningAgent, all in how it reasons about a receiver that is actually a pass-through wrapper. The first two are the ones #4191 describes; the third turned up while verifying them and is in the same method.

1–2. The last two raw-_receiver fault tests

public bool ReceiverHasFaulted => _receiver is IFaultTrackingReceiver { HasFaulted: true };   // ~:166
if (_receiver is IFaultTrackingReceiver { HasFaulted: true } faulted)                          // ~:425

IFaultTrackingReceiver is implemented by BufferedReceiver, DurableReceiver and NativeAckReceiver — and by neither wrapper. ReceiverWithRules is installed for any incoming envelope rule, which includes a bare endpoint-level MessageType or TenantId, so "behind a wrapper" is the ordinary case. A terminally faulted receiver behind one reported healthy forever and was never rebuilt: the silently-dead-listener case CritterWatch#942 exists to catch, defeated on exactly the endpoints most likely to be non-trivially configured.

Both now detect through #4190's Unwrap(). The two halves do not have the same blast radius, which is worth stating since the issue is the only place it is written down: ReceiverHasFaulted is read only by BackPressureAgent, which does not run for Inline or NativeAck at all (Endpoint.ShouldEnforceBackPressure, GH-3708) — so that half is Buffered and Durable only, and there was never a periodic detector to lose for the other two modes. The StartAsync guard is the mode-independent one.

The disposal subtlety

The rebuild disposes the unwrapped receiver, not the outer one. The wrappers hold nothing of their own and forward Dispose() to Inner, but they are IDisposable only — so faulted is IAsyncDisposable ? DisposeAsync() : Dispose() applied to the outer silently takes the sync branch and skips DurableReceiver.DisposeAsync entirely. Nulling the field drops the wrappers; the rebuild re-applies both.

3. StartAsync restacked the interceptor on every back-pressure resume

Not an Unwrap() problem. The receiver rebuild is guarded by ??=; the wrap next to it was not:

_receiver ??= Endpoint.MaybeWrapReceiver(await buildReceiverAsync());   // guarded
...
_receiver = new GlobalPartitionedInterceptor(_receiver, _runtime);      // NOT guarded

StartAsync is also the back-pressure resume path. MarkAsTooBusyAndStopReceivingAsync deliberately keeps _receiver alive — the queue it holds is precisely what has to drain before the listener resumes — and only drops the Listener. So every latch/resume cycle on a Buffered or Durable endpoint in a global-partitioned topology added another interceptor layer, with nothing ever removing one.

Never incorrect, because every layer delegates and Dispose/DrainAsync forward to inner. Just an unbounded chain whose per-message cost grows with how long the endpoint has been flapping. It wants its own guard rather than riding on Unwrap(), which is depth-agnostic by construction: it keeps working at any depth and so tells you nothing about the depth being wrong. Worth noting that #4190 is what makes this silent rather than fatal — before it, anything behind the interceptor threw from EnqueueDirectlyAsync regardless of depth, so nesting could never have been observed there.

Evidence

wrapped_receiver_fault_detection_4191, three tests, each verified red first and for its own reason:

test	pre-fix failure
faulted receiver behind a rule is reported	agent.ReceiverHasFaulted should be True but was False
faulted receiver is rebuilt on resume	should not be same as BufferedReceiver (39097574) but was BufferedReceiver (39097574)
resume does not stack an interceptor	after.Inner should be of type BufferedReceiver but was GlobalPartitionedInterceptor

Two things about how they are written:

They fault a real receiver, not a fake. The receivers assign their own onBlockError onto the public settable IBlock.OnError, and that handler sets HasFaulted on exactly one condition — a null envelope, the #506 terminal signature the block itself raises. The test reflects the block field and invokes the handler the block would have invoked, rather than writing <HasFaulted>k__BackingField or standing in a stub.

The rebuild test drives MarkAsTooBusy, not a stop/start. StopAndDrainCoreAsync nulls _receiver before it drains, so RestartAsync never reaches the guard — a stop-then-start test passes on unfixed code and proves nothing. MarkAsTooBusyAndStopReceivingAsync → StartAsync is both the path that reaches the guard and the literal CritterWatch#942 scenario, since a faulted receiver's frozen QueueCount is what latches the listener in the first place. Both of these came from a third session that had been started on this same issue from a task chip and stood down when we reconciled — it produced no code, and these two findings were the whole of what it contributed. Without the second one I would have written a stop-then-start test that went vacuously green.

CoreTests 2685/2685 with 2 pre-existing skips, on the rebased post-#4190 base. dotnet build wolverine.slnx -c Release -f net9.0 clean.

🤖 Generated with Claude Code