Skip to content

Commit

Permalink
Add helpful exception message when exception handler returns 404 (#31142
Browse files Browse the repository at this point in the history
)
  • Loading branch information
John Luo authored Mar 31, 2021
1 parent faa16cd commit fe57908
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -19,9 +20,6 @@ internal static class DiagnosticsLoggerExtensions
private static readonly Action<ILogger, Exception> _errorHandlerException =
LoggerMessage.Define(LogLevel.Error, new EventId(3, "Exception"), "An exception was thrown attempting to execute the error handler.");

private static readonly Action<ILogger, Exception?> _errorHandlerNotFound =
LoggerMessage.Define(LogLevel.Warning, new EventId(4, "HandlerNotFound"), "No exception handler was found, rethrowing original exception.");

// DeveloperExceptionPageMiddleware
private static readonly Action<ILogger, Exception?> _responseStartedErrorPageMiddleware =
LoggerMessage.Define(LogLevel.Warning, new EventId(2, "ResponseStarted"), "The response has already started, the error page middleware will not be executed.");
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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)
Expand Down
27 changes: 11 additions & 16 deletions src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ExceptionHandlerMiddleware>);
var loggerFactory = new TestLoggerFactory(sink, enabled: true);

using var host = new HostBuilder()
.ConfigureWebHost(webHostBuilder =>
{
webHostBuilder
.UseTestServer()
.ConfigureServices(services =>
{
services.AddSingleton<ILoggerFactory>(loggerFactory);
})
.Configure(app =>
{
app.Use(async (httpContext, next) =>
Expand All @@ -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<ApplicationException>(exception.InnerException);
Assert.Equal("Something bad happened.", exception.InnerException.Message);

});

Expand All @@ -520,7 +520,7 @@ public async Task ExceptionHandlerNotFound_RethrowsOriginalError()
{
innerAppBuilder.Run(httpContext =>
{
throw new InvalidOperationException("Something bad happened.");
throw new ApplicationException("Something bad happened.");
});
});
});
Expand All @@ -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]
Expand Down

0 comments on commit fe57908

Please sign in to comment.