Cascade messages returned from OnException middleware (supersedes #3000)#3005
Merged
Conversation
Ports PR #3000's four OnException tests onto main (WITHOUT its MiddlewarePolicy implementation) plus a stronger test that sets instance state in Before and reads it in OnException. All of them PASS on main as-is, so the PR's tests do not reproduce a gap — the existing OnException convention (commit 85cd239) already supports: - non-static middleware OnException with constructor injection (incl. ILogger), - additional injected parameters on OnException, - shared middleware instance across Before + OnException (instance state preserved). This branch is an investigation baseline: before adopting #3000 (which also duplicates BuildBeforeCalls' constructor/disposal logic) we need a scenario that actually fails on main; none of #3000's scenarios do. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Investigating PR #3000 showed its stated scenarios (non-static/ctor-injected OnException, ILogger injection, extra injected params, shared Before+OnException instance state) already work on main — the ported tests pass without the PR's change. The real, unaddressed gap is that a *value returned* from an OnException method was silently dropped instead of being published as a cascading message, unlike a handler method's return value. ApplyExceptionHandling now cascades the OnException return value (when no continuation strategy — IResult/HandlerContinuation/HTTP ProblemDetails — claims it). EnqueueCascadingAsync already unwraps OutgoingMessages/IEnumerable, so one path covers every return shape. The cascade must use a catch-safe frame: CaptureCascadingMessages is a MethodCall whose FindVariables exposes its dependency on the OnException call's return variable, which makes the codegen arranger pre-link the catch frames' Next pointers and collide with TryCatchFinallyFrame's manual chaining ("Frame chain is being re-arranged"). CaptureCascadingMessagesInCatch mirrors the HTTP ProblemDetails catch frame: it takes the variable in its ctor but does not yield it from FindVariables. Regression tests (kept regardless of the PR outcome): - CoreTests: ported PR #3000 scenarios + a stronger shared-instance-state test + static/instance OnException-return-value-cascaded tests. - HTTP: probe confirming an endpoint OnException with an extra injected parameter (ILogger) returning ProblemDetails works. Supersedes #3000. Co-Authored-By: Daniel Winkler <dani.winkler@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 2, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #3000 (credit retained: co-authored with @danielwinkler).
What #3000 set out to fix already works on
mainStarting from #3000, I reproduced its scenarios as tests on a fresh branch. All of them pass without the PR's change:
OnExceptionmiddleware with constructor injectionILoggerconstructor injectionBefore+OnExceptionsharing instance state (added a stronger test asserting the same instance's field set inBeforeis visible inOnException)So a test in #3000 cannot fail without #3000's own code, and its
BuildConstructorFrame()duplication (author-flagged "keep in sync") addresses a problem that isn't present. HTTP has no gap either — a probe withOnException(CustomHttpException, ILogger<…>) => ProblemDetails(the original ILogger example) passes.The real, unaddressed gap (fixed here)
A value returned from an
OnExceptionmethod was silently dropped instead of being published as a cascading message — unlike a handler method's return value. Neithermainnor #3000 handled this.MiddlewarePolicy.ApplyExceptionHandlingnow cascades theOnExceptionreturn value when no continuation strategy (IResult/HandlerContinuation/ HTTPProblemDetails) claims it.EnqueueCascadingAsyncalready unwrapsOutgoingMessages/IEnumerable, so one path covers every return shape.Codegen note
The cascade uses a catch-safe frame.
CaptureCascadingMessagesis aMethodCallwhoseFindVariablesexposes its dependency on theOnExceptioncall's return variable, which makes the codegen arranger pre-link the catch frames'Nextpointers and collide withTryCatchFinallyFrame's manual chaining ("Frame chain is being re-arranged"). The newCaptureCascadingMessagesInCatchmirrors the HTTP ProblemDetails catch frame: it takes the variable in its ctor but does not yield it fromFindVariables. (The pre-existingOutgoingMessages-from-OnExceptionpath had the same latent bug, now also covered.)Tests (kept as regression regardless)
on_exception_convention— ported feat: add support for constructor injection in middleware exception handling #3000 scenarios + shared-instance-state test + static/instanceOnException-return-value-cascaded tests (13 total).OnExceptionTests— extra-injected-parameter (ILogger) probe.Full
wolverine.slnxRelease build clean (0 warnings / 0 errors); broader middleware/exception suite green.🤖 Generated with Claude Code