Add option to exception handler middleware to suppress logging

src/Middleware/Diagnostics/src/DiagnosticsTelemetry.cs

@@ -1,8 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Diagnostics;

src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandledType.cs

@@ -0,0 +1,31 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.AspNetCore.Diagnostics;
+
+/// <summary>
+/// The result of handling an exception with the <see cref="ExceptionHandlerMiddleware"/>.
+/// </summary>
+public enum ExceptionHandledType
+{
+    /// <summary>
+    /// Exception was unhandled.
+    /// </summary>
+    Unhandled,
+    /// <summary>
+    /// Exception was handled by an <see cref="Diagnostics.IExceptionHandler"/> service instance registered in the DI container.
+    /// </summary>
+    ExceptionHandlerService,
+    /// <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>
+    ExceptionHandlerCallback,
+    /// <summary>
+    /// Exception was handled by by <see cref="Builder.ExceptionHandlerOptions.ExceptionHandlingPath"/>.
+    /// </summary>
+    ExceptionHandlingPath
+}

src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs

@@ -127,13 +127,12 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
             return;
         }
 
-        DiagnosticsTelemetry.ReportUnhandledException(_logger, context, edi.SourceException);
-
         // We can't do anything if the response has already started, just abort.
         if (context.Response.HasStarted)
         {
             _logger.ResponseStartedErrorHandler();
 
+            DiagnosticsTelemetry.ReportUnhandledException(_logger, context, edi.SourceException);
             _metrics.RequestException(exceptionName, ExceptionResult.Skipped, handler: null);
             edi.Throw();
         }
@@ -168,52 +167,92 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
             context.Response.StatusCode = _options.StatusCodeSelector?.Invoke(edi.SourceException) ?? DefaultStatusCode;
             context.Response.OnStarting(_clearCacheHeadersDelegate, context.Response);
 
-            string? handler = null;
-            var handled = false;
+            string? handlerTag = null;
+            var result = ExceptionHandledType.Unhandled;
             foreach (var exceptionHandler in _exceptionHandlers)
             {
-                handled = await exceptionHandler.TryHandleAsync(context, edi.SourceException, context.RequestAborted);
-                if (handled)
+                if (await exceptionHandler.TryHandleAsync(context, edi.SourceException, context.RequestAborted))
                 {
-                    handler = exceptionHandler.GetType().FullName;
+                    result = ExceptionHandledType.ExceptionHandlerService;
+                    handlerTag = exceptionHandler.GetType().FullName;
                     break;
                 }
             }
 
-            if (!handled)
+            if (result == ExceptionHandledType.Unhandled)
             {
                 if (_options.ExceptionHandler is not null)
                 {
                     await _options.ExceptionHandler!(context);
+
+                    // If the response has started, assume exception handler was successful.
+                    if (context.Response.HasStarted)
+                    {
+                        if (_options.ExceptionHandlingPath.HasValue)
+                        {
+                            result = ExceptionHandledType.ExceptionHandlingPath;
+                            handlerTag = _options.ExceptionHandlingPath.Value;
+                        }
+                        else
+                        {
+                            result = ExceptionHandledType.ExceptionHandlerCallback;
+                        }
+                    }
                 }
                 else
                 {
-                    handled = await _problemDetailsService!.TryWriteAsync(new()
+                    if (await _problemDetailsService!.TryWriteAsync(new()
                     {
                         HttpContext = context,
                         AdditionalMetadata = exceptionHandlerFeature.Endpoint?.Metadata,
                         ProblemDetails = { Status = context.Response.StatusCode },
                         Exception = edi.SourceException,
-                    });
-                    if (handled)
+                    }))
                     {
-                        handler = _problemDetailsService.GetType().FullName;
+                        result = ExceptionHandledType.ProblemDetailsService;
+                        handlerTag = _problemDetailsService.GetType().FullName;
                     }
                 }
             }
-            // If the response has already started, assume exception handler was successful.
-            if (context.Response.HasStarted || handled || _options.StatusCodeSelector != null || context.Response.StatusCode != StatusCodes.Status404NotFound || _options.AllowStatusCode404Response)
+
+            if (result != ExceptionHandledType.Unhandled || _options.StatusCodeSelector != null || context.Response.StatusCode != StatusCodes.Status404NotFound || _options.AllowStatusCode404Response)
             {
-                const string eventName = "Microsoft.AspNetCore.Diagnostics.HandledException";
-                if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled(eventName))
+                var suppressDiagnostics = false;
+
+                // Customers may prefer to handle the exception and to do their own diagnostics.
+                // In that case, it can be undesirable for the middleware to log the exception at an error level.
+                // Run the configured callback to determine if exception diagnostics in the middleware should be suppressed.
+                if (_options.SuppressDiagnosticsCallback is { } suppressCallback)
                 {
-                    WriteDiagnosticEvent(_diagnosticListener, eventName, new { httpContext = context, exception = edi.SourceException });
+                    var suppressDiagnosticsContext = new ExceptionHandlerSuppressDiagnosticsContext
+                    {
+                        HttpContext = context,
+                        Exception = edi.SourceException,
+                        ExceptionHandledBy = result
+                    };
+                    suppressDiagnostics = suppressCallback(suppressDiagnosticsContext);
                 }
 
-                _metrics.RequestException(exceptionName, ExceptionResult.Handled, handler);
+                if (!suppressDiagnostics)
+                {
+                    // Note: Microsoft.AspNetCore.Diagnostics.HandledException is used by AppInsights to log errors.
+                    // The diagnostics event is run together with standard exception logging.
+                    const string eventName = "Microsoft.AspNetCore.Diagnostics.HandledException";
+                    if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled(eventName))
+                    {
+                        WriteDiagnosticEvent(_diagnosticListener, eventName, new { httpContext = context, exception = edi.SourceException });
+                    }
+
+                    DiagnosticsTelemetry.ReportUnhandledException(_logger, context, edi.SourceException);
+                }
+
+                _metrics.RequestException(exceptionName, ExceptionResult.Handled, handlerTag);
                 return;
             }
 
+            // Exception is unhandled. Record diagnostics for the unhandled exception before it is wrapped.
+            DiagnosticsTelemetry.ReportUnhandledException(_logger, context, edi.SourceException);
+
             edi = ExceptionDispatchInfo.Capture(new InvalidOperationException($"The exception handler configured on {nameof(ExceptionHandlerOptions)} produced a 404 status response. " +
                 $"This {nameof(InvalidOperationException)} containing the original exception was thrown since this is often due to a misconfigured {nameof(ExceptionHandlerOptions.ExceptionHandlingPath)}. " +
                 $"If the exception handler is expected to return 404 status responses then set {nameof(ExceptionHandlerOptions.AllowStatusCode404Response)} to true.", edi.SourceException));
@@ -222,6 +261,9 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
         {
             // Suppress secondary exceptions, re-throw the original.
             _logger.ErrorHandlerException(ex2);
+
+            // There was an error handling the exception. Log original unhandled exception.
+            DiagnosticsTelemetry.ReportUnhandledException(_logger, context, edi.SourceException);
         }
         finally
         {

src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerOptions.cs

@@ -1,8 +1,10 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics.Tracing;
 using Microsoft.AspNetCore.Diagnostics;
 using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Builder;
 
@@ -11,6 +13,14 @@ namespace Microsoft.AspNetCore.Builder;
 /// </summary>
 public class ExceptionHandlerOptions
 {
+    /// <summary>
+    /// Initializes a new instance of the <see cref="ExceptionHandlerOptions"/> class.
+    /// </summary>
+    public ExceptionHandlerOptions()
+    {
+        SuppressDiagnosticsCallback = static c => c.ExceptionHandledBy == ExceptionHandledType.ExceptionHandlerService;
+    }
+
     /// <summary>
     /// The path to the exception handling endpoint. This path will be used when executing
     /// the <see cref="ExceptionHandler"/>.
@@ -40,10 +50,34 @@ public class ExceptionHandlerOptions
     public bool AllowStatusCode404Response { get; set; }
 
     /// <summary>
-    /// Gets or sets a delegate used to map an exception to a http status code.
+    /// Gets or sets a delegate used to map an exception to an HTTP status code.
     /// </summary>
     /// <remarks>
     /// If <see cref="StatusCodeSelector"/> is <c>null</c>, the default exception status code 500 is used.
     /// </remarks>
     public Func<Exception, int>? StatusCodeSelector { get; set; }
+
+    /// <summary>
+    /// Gets or sets a callback that can return <see langword="true" /> be used to suppress diagnostics by <see cref="ExceptionHandlerMiddleware" />.
+    /// The default value is to suppress diagnostics if the exception was handled by an <see cref="IExceptionHandler"/> service instance registered in the DI container.
+    /// <para>
+    /// 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.
+    /// </para>
+    /// <para>
+    /// Suppress diagnostics include:
+    /// </para>
+    /// <list type="bullet">
+    ///   <item>
+    ///     <description>Logging <c>UnhandledException</c> to <see cref="ILogger"/>.</description>
+    ///   </item>
+    ///   <item>
+    ///     <description>Writing <c>Microsoft.AspNetCore.Diagnostics.HandledException</c> event to <see cref="EventSource" />.</description>
+    ///   </item>
+    ///   <item>
+    ///     <description>Adding <c>error.type</c> tag to the <c>http.server.request.duration</c> metric.</description>
+    ///   </item>
+    /// </list>
+    /// </summary>
+    public Func<ExceptionHandlerSuppressDiagnosticsContext, bool>? SuppressDiagnosticsCallback { get; set; }
 }

src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerSuppressDiagnosticsContext.cs

@@ -0,0 +1,27 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.AspNetCore.Http;
+
+namespace Microsoft.AspNetCore.Diagnostics;
+
+/// <summary>
+/// The context used to determine whether <see cref="ExceptionHandlerMiddleware"/> should record diagnostics for an exception.
+/// </summary>
+public sealed class ExceptionHandlerSuppressDiagnosticsContext
+{
+    /// <summary>
+    /// Gets the <see cref="Http.HttpContext"/> of the current request.
+    /// </summary>
+    public required HttpContext HttpContext { get; init; }
+
+    /// <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 exception handling by <see cref="ExceptionHandlerMiddleware"/>.
+    /// </summary>
+    public required ExceptionHandledType ExceptionHandledBy { get; init; }
+}

src/Middleware/Diagnostics/src/PublicAPI.Unshipped.txt

@@ -1,4 +1,20 @@
 #nullable enable
+Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressDiagnosticsCallback.get -> System.Func<Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressDiagnosticsContext!, bool>?
+Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressDiagnosticsCallback.set -> void
 Microsoft.AspNetCore.Builder.StatusCodePagesOptions.CreateScopeForErrors.get -> bool
 Microsoft.AspNetCore.Builder.StatusCodePagesOptions.CreateScopeForErrors.set -> void
-static Microsoft.AspNetCore.Builder.StatusCodePagesExtensions.UseStatusCodePagesWithReExecute(this Microsoft.AspNetCore.Builder.IApplicationBuilder! app, string! pathFormat, bool createScopeForErrors, string? queryFormat = null) -> Microsoft.AspNetCore.Builder.IApplicationBuilder!
+Microsoft.AspNetCore.Diagnostics.ExceptionHandledType
+Microsoft.AspNetCore.Diagnostics.ExceptionHandledType.ExceptionHandlerCallback = 3 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandledType
+Microsoft.AspNetCore.Diagnostics.ExceptionHandledType.ExceptionHandlerService = 1 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandledType
+Microsoft.AspNetCore.Diagnostics.ExceptionHandledType.ExceptionHandlingPath = 4 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandledType
+Microsoft.AspNetCore.Diagnostics.ExceptionHandledType.ProblemDetailsService = 2 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandledType
+Microsoft.AspNetCore.Diagnostics.ExceptionHandledType.Unhandled = 0 -> Microsoft.AspNetCore.Diagnostics.ExceptionHandledType
+Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressDiagnosticsContext
+Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressDiagnosticsContext.Exception.get -> System.Exception!
+Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressDiagnosticsContext.Exception.init -> void
+Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressDiagnosticsContext.ExceptionHandledBy.get -> Microsoft.AspNetCore.Diagnostics.ExceptionHandledType
+Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressDiagnosticsContext.ExceptionHandledBy.init -> void
+Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressDiagnosticsContext.ExceptionHandlerSuppressDiagnosticsContext() -> void
+Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressDiagnosticsContext.HttpContext.get -> Microsoft.AspNetCore.Http.HttpContext!
+Microsoft.AspNetCore.Diagnostics.ExceptionHandlerSuppressDiagnosticsContext.HttpContext.init -> void
+static Microsoft.AspNetCore.Builder.StatusCodePagesExtensions.UseStatusCodePagesWithReExecute(this Microsoft.AspNetCore.Builder.IApplicationBuilder! app, string! pathFormat, bool createScopeForErrors, string? queryFormat = null) -> Microsoft.AspNetCore.Builder.IApplicationBuilder!