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.