Skip to content

Commit fe57908

Browse files
author
John Luo
authored
Add helpful exception message when exception handler returns 404 (#31142)
1 parent faa16cd commit fe57908

File tree

3 files changed

+16
-26
lines changed

3 files changed

+16
-26
lines changed

src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs

+1-8
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using Microsoft.AspNetCore.Builder;
56
using Microsoft.Extensions.Logging;
67

78
namespace Microsoft.AspNetCore.Diagnostics
@@ -19,9 +20,6 @@ internal static class DiagnosticsLoggerExtensions
1920
private static readonly Action<ILogger, Exception> _errorHandlerException =
2021
LoggerMessage.Define(LogLevel.Error, new EventId(3, "Exception"), "An exception was thrown attempting to execute the error handler.");
2122

22-
private static readonly Action<ILogger, Exception?> _errorHandlerNotFound =
23-
LoggerMessage.Define(LogLevel.Warning, new EventId(4, "HandlerNotFound"), "No exception handler was found, rethrowing original exception.");
24-
2523
// DeveloperExceptionPageMiddleware
2624
private static readonly Action<ILogger, Exception?> _responseStartedErrorPageMiddleware =
2725
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
4442
_errorHandlerException(logger, exception);
4543
}
4644

47-
public static void ErrorHandlerNotFound(this ILogger logger)
48-
{
49-
_errorHandlerNotFound(logger, null);
50-
}
51-
5245
public static void ResponseStartedErrorPageMiddleware(this ILogger logger)
5346
{
5447
_responseStartedErrorPageMiddleware(logger, null);

src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddleware.cs

+4-2
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,9 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
142142
return;
143143
}
144144

145-
_logger.ErrorHandlerNotFound();
145+
edi = ExceptionDispatchInfo.Capture(new InvalidOperationException($"The exception handler configured on {nameof(ExceptionHandlerOptions)} produced a 404 status response. " +
146+
$"This {nameof(InvalidOperationException)} containing the original exception was thrown since this is often due to a misconfigured {nameof(ExceptionHandlerOptions.ExceptionHandlingPath)}. " +
147+
$"If the exception handler is expected to return 404 status responses then set {nameof(ExceptionHandlerOptions.AllowStatusCode404Response)} to true.", edi.SourceException));
146148
}
147149
catch (Exception ex2)
148150
{
@@ -154,7 +156,7 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
154156
context.Request.Path = originalPath;
155157
}
156158

157-
edi.Throw(); // Re-throw the original if we couldn't handle it
159+
edi.Throw(); // Re-throw wrapped exception or the original if we couldn't handle it
158160
}
159161

160162
private static void ClearHttpContext(HttpContext context)

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

+11-16
Original file line numberDiff line numberDiff line change
@@ -469,20 +469,13 @@ public void UsingExceptionHandler_ThrowsAnException_WhenExceptionHandlingPathNot
469469
}
470470

471471
[Fact]
472-
public async Task ExceptionHandlerNotFound_RethrowsOriginalError()
472+
public async Task ExceptionHandlerNotFound_ThrowsIOEWithOriginalError()
473473
{
474-
var sink = new TestSink(TestSink.EnableWithTypeName<ExceptionHandlerMiddleware>);
475-
var loggerFactory = new TestLoggerFactory(sink, enabled: true);
476-
477474
using var host = new HostBuilder()
478475
.ConfigureWebHost(webHostBuilder =>
479476
{
480477
webHostBuilder
481478
.UseTestServer()
482-
.ConfigureServices(services =>
483-
{
484-
services.AddSingleton<ILoggerFactory>(loggerFactory);
485-
})
486479
.Configure(app =>
487480
{
488481
app.Use(async (httpContext, next) =>
@@ -500,9 +493,16 @@ public async Task ExceptionHandlerNotFound_RethrowsOriginalError()
500493
httpContext.Response.StatusCode = StatusCodes.Status500InternalServerError;
501494
}
502495

503-
// The original exception is thrown
496+
// Invalid operation exception
504497
Assert.NotNull(exception);
505-
Assert.Equal("Something bad happened.", exception.Message);
498+
Assert.Equal("The exception handler configured on ExceptionHandlerOptions produced a 404 status response. " +
499+
"This InvalidOperationException containing the original exception was thrown since this is often due to a misconfigured ExceptionHandlingPath. " +
500+
"If the exception handler is expected to return 404 status responses then set AllowStatusCode404Response to true.", exception.Message);
501+
502+
// The original exception is inner exception
503+
Assert.NotNull(exception.InnerException);
504+
Assert.IsType<ApplicationException>(exception.InnerException);
505+
Assert.Equal("Something bad happened.", exception.InnerException.Message);
506506

507507
});
508508

@@ -520,7 +520,7 @@ public async Task ExceptionHandlerNotFound_RethrowsOriginalError()
520520
{
521521
innerAppBuilder.Run(httpContext =>
522522
{
523-
throw new InvalidOperationException("Something bad happened.");
523+
throw new ApplicationException("Something bad happened.");
524524
});
525525
});
526526
});
@@ -535,11 +535,6 @@ public async Task ExceptionHandlerNotFound_RethrowsOriginalError()
535535
Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
536536
Assert.Equal(string.Empty, await response.Content.ReadAsStringAsync());
537537
}
538-
539-
Assert.Contains(sink.Writes, w =>
540-
w.LogLevel == LogLevel.Warning
541-
&& w.EventId == 4
542-
&& w.Message == "No exception handler was found, rethrowing original exception.");
543538
}
544539

545540
[Fact]

0 commit comments

Comments
 (0)