Duende Software
The LoggingOptions.UnhandledExceptionLoggingFilter is not just filtering logs, but swallows whole exceptions
#491
-
|
In a standard ASP.NET Core setup, one normally already has exception logging etc. in place. Hence, we want to silence the implicit "exception handler middleware" that the IdentityServer has built-in. ...
catch (Exception ex) when (options.Logging.InvokeUnhandledExceptionLoggingFilter(context, ex) is not false)
{
await events.RaiseAsync(new UnhandledExceptionEvent(ex));
Telemetry.Metrics.UnHandledException(ex);
_sanitizedLogger.LogCritical(ex, "Unhandled exception: {exception}", ex.Message);
throw;
}
...Sadly, the customization option provided via Ideally, that option would prevent only the logging and let other exception-related middlewares higher up in the chain do the right thing. I.e., we are interested in a change like the following to prevent unnecessary duplicated logs: ...
catch (Exception ex)
{
if (options.Logging.InvokeUnhandledExceptionLoggingFilter(context, ex) is not false)
{
await events.RaiseAsync(new UnhandledExceptionEvent(ex));
Telemetry.Metrics.UnHandledException(ex);
_sanitizedLogger.LogCritical(ex, "Unhandled exception: {exception}", ex.Message);
}
throw;
}
...This change would than allow us to set In general, I would suggest to remove that whole exception logging block and suggest customers/users to follow the standard practices here, which I guess anybody is anyways already doing, so there should really not be a need for this, which also shows, given that there is an option for filtering. |
Beta Was this translation helpful? Give feedback.
Replies: 2 comments
-
|
I ran a quick test locally, setting the logging filter to return Before I share the results of my quick test, I should also point out that the exception filter within the Setting the
|
Beta Was this translation helpful? Give feedback.
-
|
@wcabus Indeed. It appears I was reading the code incorrectly in my sleep-deprived mind. Thank you for checking and clarifying! |
Beta Was this translation helpful? Give feedback.
I ran a quick test locally, setting the logging filter to return
falsefor every call first and then switching totrueto spot the difference.Then, to trigger the exception logic, the bubbling up and whether or not the middleware chain would continue to run, I triggered an exception from within the discovery endpoint.
Before I share the results of my quick test, I should also point out that the exception filter within the
IdentityServerMiddlewarewill only catch or filter exceptions that occur while processing IdentityServer's endpoints.Setting the
UnhandledExceptionLoggingFilterto always returnfalseSince I have
UseDeveloperExceptionPagelogic running in my sample project, when I tri…