Skip to content
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

Add option to exception handler middleware to not log error #59074

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Nov 20, 2024

Fixes #54554

Background

Exception handler middleware handles exceptions thrown in the app. There are various mechanisms for handling an exception:

  • Registered IExceptionHandler instances. Return true to indicate exception is handled.
  • Registered IProblemDetailsService instance. Return true to indicate exception is handled.
  • Configured ExceptionHandlerOptions.ExceptionHandler delegate.
  • Configured ExceptionHandlerOptions.StatusCodeSelector delegate.

If the exception is handled then the middleware invoke returns without rethrowing the exception. Unhandled exceptions are rethrown back into the middleware pipeline.

Problem

Today the exception handle middleware always writes the UnhandledException log message (skipped if the exception is related to the request being aborted). I think the idea here is the exception handler received an unhandled error. The problem is it is logged at an Error level, and customers who automatically log all Error level logs could get a potentially unwanted error the message.

The request from users is to only write the UnhandledException log message if the exception handler didn't handle the exception. That stops the error level UnhandledException log message from being logged.

Fix

The PR adds a new option to the exception handler, ExceptionHandlerOptions.SuppressLoggingOnHandledException (final name TBD). It keeps the current behavior but gives an option to opt-in to only logging the unhandled exception log message if the middleware didn't handle the exception.

@JamesNK JamesNK added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Nov 20, 2024
@JamesNK
Copy link
Member Author

JamesNK commented Nov 20, 2024

Product changes ready to review. Tests coming.

@JamesNK
Copy link
Member Author

JamesNK commented Nov 20, 2024

API review: #59075

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhandled exceptions are being logged before custom IExceptionHandler is being called
1 participant