-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Log exception set in IDiagnosticContext #271
Conversation
Until serilog/serilog-extensions-hosting#56 is merged and the dependency version of |
@nblumhardt Continuing from serilog/serilog-extensions-hosting#56 (comment):
In our scenario, the response pipeline looks like this:
Going on my gut feeling, I would expect exception B to be logged since it was thrown last in the response pipeline. Also, I believe this requires no additional change to this PR. What do you think? I assume a new |
Sounds reasonable to me @angularsen 👍 Serilog.Extensions.Hosting 4.2.1-dev-00092 is now on NuGet and can be referenced from this repo to get the PR building 🚀 |
If there is no unhandled exception, then log the exception set in IDiagnosticContext.
1fdff60
to
b8081d8
Compare
@nblumhardt Rebased and upgraded to prerelease nuget, tests are now green. |
Looks good! Will be great to try this out in the wild :-) |
Awesome! 🎉 I will give it a spin the coming week and report back. |
Fixes: #270
Depends on: serilog/serilog-extensions-hosting#56
Changes
RequestLoggingMiddleware.LogCompletion
enriches with the exception set inIDiagnosticContext
, unless an unhandled exception takes precedence.Testing
IDiagnosticContext