WIP

Author: JamesNK

src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs

@@ -137,6 +137,8 @@ public async Task Invoke(HttpContext context)
                 return;
             }
 
+            _logger.UnhandledException(ex);
+
             if (context.Response.HasStarted)
             {
                 _logger.ResponseStartedErrorPageMiddleware();

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

@@ -170,55 +170,67 @@ 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 handler = Handler.None;
             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;
+                    handler = Handler.IExceptionHandler;
+                    handlerTag = exceptionHandler.GetType().FullName;
                     break;
                 }
             }
 
-            if (!handled)
+            if (handler == Handler.None)
             {
                 if (_options.ExceptionHandler is not null)
                 {
                     await _options.ExceptionHandler!(context);
+
+                    // If the response has started, assume exception handler was successful.
+                    if (context.Response.HasStarted)
+                    {
+                        handler = Handler.ExceptionHandler;
+                        if (_options.ExceptionHandlingPath.HasValue)
+                        {
+                            handlerTag = _options.ExceptionHandlingPath.Value;
+                        }
+                    }
                 }
                 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;
+                        handler = Handler.IProblemDetailsService;
+                        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 (handler != Handler.None || _options.StatusCodeSelector != null || context.Response.StatusCode != StatusCodes.Status404NotFound || _options.AllowStatusCode404Response)
             {
-                const string eventName = "Microsoft.AspNetCore.Diagnostics.HandledException";
-                if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled(eventName))
+                // Customers with an IExceptionHandler that reports it handled the exception could do their own logging.
+                // Don't log IExceptionHandler handled exceptions if SuppressLoggingIExceptionHandler is set to true.
+                // Note: Microsoft.AspNetCore.Diagnostics.HandledException is used by AppInsights to log errors so it's also skipped.
+                if (handler != Handler.IExceptionHandler || !_options.SuppressLoggingIExceptionHandler)
                 {
-                    WriteDiagnosticEvent(_diagnosticListener, eventName, new { httpContext = context, exception = edi.SourceException });
-                }
+                    const string eventName = "Microsoft.AspNetCore.Diagnostics.HandledException";
+                    if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled(eventName))
+                    {
+                        WriteDiagnosticEvent(_diagnosticListener, eventName, new { httpContext = context, exception = edi.SourceException });
+                    }
 
-                if (!_options.SuppressLoggingOnHandledException)
-                {
                     _logger.UnhandledException(edi.SourceException);
                 }
 
-                _metrics.RequestException(exceptionName, ExceptionResult.Handled, handler);
+                _metrics.RequestException(exceptionName, ExceptionResult.Handled, handlerTag);
                 return;
             }
 
@@ -273,4 +285,12 @@ private static Task ClearCacheHeaders(object state)
         headers.ETag = default;
         return Task.CompletedTask;
     }
+
+    private enum Handler
+    {
+        None,
+        IExceptionHandler,
+        IProblemDetailsService,
+        ExceptionHandler
+    }
 }

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

@@ -49,8 +49,7 @@ public class ExceptionHandlerOptions
 
     /// <summary>
     /// Gets or sets a value that determines if the exception handler middleware should suppress logging
-    /// if the exception was handled by either <see cref="ExceptionHandler"/>, <see cref="IProblemDetailsService"/> or
-    /// <see cref="IExceptionHandler"/> registered in the DI container.
+    /// if the exception was handled by a <see cref="IExceptionHandler"/> registered in the DI container.
     /// </summary>
-    public bool SuppressLoggingOnHandledException { get; set; }
+    public bool SuppressLoggingIExceptionHandler { get; set; }
 }

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

@@ -1,3 +1,3 @@
 #nullable enable
-Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressLoggingOnHandledException.get -> bool
-Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressLoggingOnHandledException.set -> void
+Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressLoggingIExceptionHandler.get -> bool
+Microsoft.AspNetCore.Builder.ExceptionHandlerOptions.SuppressLoggingIExceptionHandler.set -> void

src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs

@@ -247,6 +247,32 @@ public async Task Metrics_ExceptionThrown_Handled_Reported()
             m => AssertRequestException(m, "System.InvalidOperationException", "handled", typeof(TestExceptionHandler).FullName));
     }
 
+    [Fact]
+    public async Task Metrics_ExceptionThrown_ErrorPathHandled_Reported()
+    {
+        // Arrange
+        var httpContext = CreateHttpContext();
+        var optionsAccessor = CreateOptionsAccessor(
+            exceptionHandler: context =>
+            {
+                context.Features.Set<IHttpResponseFeature>(new TestHttpResponseFeature());
+                return Task.CompletedTask;
+            },
+            exceptionHandlingPath: "/error");
+        var meterFactory = new TestMeterFactory();
+        var middleware = CreateMiddleware(_ => throw new InvalidOperationException(), optionsAccessor, meterFactory: meterFactory);
+        var meter = meterFactory.Meters.Single();
+
+        using var diagnosticsRequestExceptionCollector = new MetricCollector<long>(meterFactory, DiagnosticsMetrics.MeterName, "aspnetcore.diagnostics.exceptions");
+
+        // Act
+        await middleware.Invoke(httpContext);
+
+        // Assert
+        Assert.Collection(diagnosticsRequestExceptionCollector.GetMeasurementSnapshot(),
+            m => AssertRequestException(m, "System.InvalidOperationException", "handled", "/error"));
+    }
+
     [Fact]
     public async Task Metrics_ExceptionThrown_ResponseStarted_Skipped()
     {
@@ -529,4 +555,9 @@ public object GetService(Type serviceType)
             throw new NotImplementedException();
         }
     }
+
+    private class TestHttpResponseFeature : HttpResponseFeature
+    {
+        public override bool HasStarted => true;
+    }
 }