Add `ExceptionHandlerOptions.SuppressLoggingCallback`

[JamesNK]

Background and Motivation

A config callback used to optionally suppress logging by ExceptionHandlerMiddleware.

For #54554

Proposed API

namespace Microsoft.AspNetCore.Builder;

public class ExceptionHandlerOptions
{
+    /// <summary>
+    /// Gets or sets a callback that can be used to suppress logging by <see cref="ExceptionHandlerMiddleware" />.
+    /// This callback is only run if the exception was handled by the middleware.
+    /// Unhandled exceptions and exceptions thrown after the response has started are always logged.
+    /// </summary>
+    public Func<ExceptionHandlerSuppressLoggingContext, bool>? SuppressLoggingCallback { get; set; }
}

namespace Microsoft.AspNetCore.Diagnostics;

/// <summary>
/// The context used to determine whether exception handler middleware should log an exception.
/// </summary>
public sealed class ExceptionHandlerSuppressLoggingContext
{
    /// <summary>
    /// Gets the <see cref="System.Exception"/> that the exception handler middleware is processing.
    /// </summary>
    public required Exception Exception { get; init; }

    /// <summary>
    /// Gets the result of the exception handler middleware.
    /// </summary>
    public required ExceptionHandlerResult HandlerResult { get; init; }
}

/// <summary>
/// The result of executing <see cref="ExceptionHandlerMiddleware"/>.
/// </summary>
public enum ExceptionHandlerResult
{
    /// <summary>
    /// Exception was unhandled.
    /// </summary>
    Unhandled,
    /// <summary>
    /// Exception was handled by an <see cref="Diagnostics.IExceptionHandler"/> instance registered in the DI container.
    /// </summary>
    IExceptionHandler,
    /// <summary>
    /// Exception was handled by an <see cref="Http.IProblemDetailsService"/> instance registered in the DI container.
    /// </summary>
    ProblemDetailsService,
    /// <summary>
    /// Exception was handled by by <see cref="Builder.ExceptionHandlerOptions.ExceptionHandler"/>.
    /// </summary>
    ExceptionHandler,
    /// <summary>
    /// Exception was handled by by <see cref="Builder.ExceptionHandlerOptions.ExceptionHandlingPath"/>.
    /// </summary>
    ExceptionHandlingPath
}

Defaults to null.

Usage Examples

app.UseExceptionHandler(new ExceptionHandlerOptions
{
    SuppressLoggingCallback = context => context.HandlerResult == ExceptionHandlerResult.IExceptionHandler;
});

Alternative Designs

• Could have bool properties to suppress different scenarios. Not flexible
• Could avoid context type on callback. Added to make it easy to pass new properties in the future

Risks

[dotnet-policy-service]

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]

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]

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

[halter73]

API Review Notes:

• What's the default? false
• What's the log level? Error
• The current proposed name is SuppressLoggingIExceptionHandler. What other names could we use?
  • DisableLoggingIExceptionHandlerWhenHandled
  • DisableLoggingExceptionsWhenHandled
  • DisableLoggingOnHandledException
  • DisableLoggingHandledExceptions
• Could we suppress logging all unhandled exceptions? Maybe

We'd like to know whether or not it makes sense to suppress logging all unhandled exceptions before deciding on a name for the bool option.

[dotnet-policy-service]

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.