Skip to content

feat: add support for constructor injection in middleware exception handling#3000

Closed
danielwinkler wants to merge 1 commit into
JasperFx:mainfrom
danielwinkler:feature/iexceptionmiddleware-injection
Closed

feat: add support for constructor injection in middleware exception handling#3000
danielwinkler wants to merge 1 commit into
JasperFx:mainfrom
danielwinkler:feature/iexceptionmiddleware-injection

Conversation

@danielwinkler
Copy link
Copy Markdown
Contributor

I posted the following question to discord but haven't gotten an answer yet

As I am not deep enough into the wolverine source generation, I can only offer a Claude generated solution for the problem; maybe it helps, otherwise please reject.


Hi,

I saw that there has been an IExceptionFilter equivalent introduced recently
#2410

From the documentation and also my experiments, I was unable to inject anything else than the excption into the handler

My goal would be to log the exception / stack trace, is there a way to achieve something like

public static class LoggingExceptionMiddleware
{
    public static ProblemDetails OnException(DomainException exception)
        => ApiExceptionMapper.CreateDomainExceptionProblemDetails(exception);

    // public static ProblemDetails OnException(DomainException exception, ILogger logger)
    // {
    //     // logger.LogWarning(exception, "HandleDomainException");
    //     return ApiExceptionMapper.CreateDomainExceptionProblemDetails(exception);
    // }
}

jeremydmiller added a commit that referenced this pull request Jun 1, 2026
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.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jeremydmiller
Copy link
Copy Markdown
Member

Thanks for this, @danielwinkler — closing as superseded by #3005, with you kept on as co-author there.

When I dug into the scenarios this PR targets, they already pass on main without the change: non-static/constructor-injected OnException middleware, ILogger constructor injection, static middleware with extra injected parameters, and a shared Before+OnException instance all work today (I added regression tests for each in #3005, including a stronger one asserting the same instance's state set in Before is visible in OnException). Because of that, the added BuildConstructorFrame() path (the "keep both in sync" duplication) isn't needed.

What surfaced instead is a genuine gap that neither main nor this PR addressed: a value returned from an OnException method was silently dropped rather than published as a cascading message — unlike a handler method's return value. #3005 fixes that (with a catch-block-safe cascade frame to avoid a codegen "Frame chain is being re-arranged" collision) and keeps all of the reproduced scenarios as regression coverage.

Really appreciate the push that led us here.

@danielwinkler
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback, really glad that I could contribute!

jeremydmiller added a commit that referenced this pull request Jun 1, 2026
…) (#3005)

* Reproduction for PR #3000 (middleware OnException ctor injection)

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>

* Cascade messages returned from OnException middleware (GH-3000)

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>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Daniel Winkler <dani.winkler@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants