diff --git a/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs b/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs index 1a812f8d1579..210e28e768c7 100644 --- a/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs +++ b/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Diagnostics @@ -19,9 +20,6 @@ internal static class DiagnosticsLoggerExtensions private static readonly Action _errorHandlerException = LoggerMessage.Define(LogLevel.Error, new EventId(3, "Exception"), "An exception was thrown attempting to execute the error handler."); - private static readonly Action _errorHandlerNotFound = - LoggerMessage.Define(LogLevel.Warning, new EventId(4, "HandlerNotFound"), "No exception handler was found, rethrowing original exception."); - // DeveloperExceptionPageMiddleware private static readonly Action _responseStartedErrorPageMiddleware = LoggerMessage.Define(LogLevel.Warning, new EventId(2, "ResponseStarted"), "The response has already started, the error page middleware will not be executed."); @@ -44,11 +42,6 @@ public static void ErrorHandlerException(this ILogger logger, Exception exceptio _errorHandlerException(logger, exception); } - public static void ErrorHandlerNotFound(this ILogger logger) - { - _errorHandlerNotFound(logger, null); - } - public static void ResponseStartedErrorPageMiddleware(this ILogger logger) { _responseStartedErrorPageMiddleware(logger, null); diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddleware.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddleware.cs index 11cfac7f19ce..a21a634daf11 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddleware.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddleware.cs @@ -142,7 +142,9 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed return; } - _logger.ErrorHandlerNotFound(); + 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)); } catch (Exception ex2) { @@ -154,7 +156,7 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed context.Request.Path = originalPath; } - edi.Throw(); // Re-throw the original if we couldn't handle it + edi.Throw(); // Re-throw wrapped exception or the original if we couldn't handle it } private static void ClearHttpContext(HttpContext context) diff --git a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs index 5e2f38f7d13f..a7790bb145f6 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs @@ -469,20 +469,13 @@ public void UsingExceptionHandler_ThrowsAnException_WhenExceptionHandlingPathNot } [Fact] - public async Task ExceptionHandlerNotFound_RethrowsOriginalError() + public async Task ExceptionHandlerNotFound_ThrowsIOEWithOriginalError() { - var sink = new TestSink(TestSink.EnableWithTypeName); - var loggerFactory = new TestLoggerFactory(sink, enabled: true); - using var host = new HostBuilder() .ConfigureWebHost(webHostBuilder => { webHostBuilder .UseTestServer() - .ConfigureServices(services => - { - services.AddSingleton(loggerFactory); - }) .Configure(app => { app.Use(async (httpContext, next) => @@ -500,9 +493,16 @@ public async Task ExceptionHandlerNotFound_RethrowsOriginalError() httpContext.Response.StatusCode = StatusCodes.Status500InternalServerError; } - // The original exception is thrown + // Invalid operation exception Assert.NotNull(exception); - Assert.Equal("Something bad happened.", exception.Message); + Assert.Equal("The exception handler configured on ExceptionHandlerOptions produced a 404 status response. " + + "This InvalidOperationException containing the original exception was thrown since this is often due to a misconfigured ExceptionHandlingPath. " + + "If the exception handler is expected to return 404 status responses then set AllowStatusCode404Response to true.", exception.Message); + + // The original exception is inner exception + Assert.NotNull(exception.InnerException); + Assert.IsType(exception.InnerException); + Assert.Equal("Something bad happened.", exception.InnerException.Message); }); @@ -520,7 +520,7 @@ public async Task ExceptionHandlerNotFound_RethrowsOriginalError() { innerAppBuilder.Run(httpContext => { - throw new InvalidOperationException("Something bad happened."); + throw new ApplicationException("Something bad happened."); }); }); }); @@ -535,11 +535,6 @@ public async Task ExceptionHandlerNotFound_RethrowsOriginalError() Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); Assert.Equal(string.Empty, await response.Content.ReadAsStringAsync()); } - - Assert.Contains(sink.Writes, w => - w.LogLevel == LogLevel.Warning - && w.EventId == 4 - && w.Message == "No exception handler was found, rethrowing original exception."); } [Fact]