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 ExceptionHandlerOptions.SuppressLoggingOnHandledException #59075

Open
JamesNK opened this issue Nov 20, 2024 · 3 comments
Open

Add ExceptionHandlerOptions.SuppressLoggingOnHandledException #59075

JamesNK opened this issue Nov 20, 2024 · 3 comments
Labels
api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware
Milestone

Comments

@JamesNK
Copy link
Member

JamesNK commented Nov 20, 2024

Background and Motivation

A config option to only log in exception handler middleware if the exception is unhandled.

For #54554

Proposed API

namespace Microsoft.AspNetCore.Builder;

public class ExceptionHandlerOptions
{
+    public bool SuppressLoggingOnHandledException { get; set; }
}

Defaults to false.

Usage Examples

app.UseExceptionHandler(new ExceptionHandlerOptions()
{
    SuppressLoggingOnHandledException = true
});

Alternative Designs

Could make the new behavior the default. I don't think that is a good idea for backwards compatibility reasons, and some people might prefer the current behavior.

Risks

@JamesNK JamesNK added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Nov 20, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware label Nov 20, 2024
Copy link
Contributor

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@mitchdenny
Copy link
Member

This seems reasonable to me in terms of the feature. Naming wise ... Suppress vs. Disable vs. Other options. I'm assuming you are using a negative property name because you want the default value to be false?

@JamesNK
Copy link
Member Author

JamesNK commented Nov 22, 2024

Yes. Suppress is common in our APIs: https://learn.microsoft.com/en-us/dotnet/api/?view=aspnetcore-9.0&term=Suppress

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware
Projects
None yet
Development

No branches or pull requests

2 participants