diff --git a/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs b/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs index 995bbc6786..fdd67aee9e 100644 --- a/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs +++ b/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -19,6 +19,9 @@ namespace Microsoft.AspNetCore.Diagnostics 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."); @@ -41,6 +44,11 @@ namespace Microsoft.AspNetCore.Diagnostics _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 65564cbd59..fbf0c8913d 100644 --- a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddleware.cs +++ b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddleware.cs @@ -113,18 +113,22 @@ namespace Microsoft.AspNetCore.Diagnostics }; context.Features.Set(exceptionHandlerFeature); context.Features.Set(exceptionHandlerFeature); - context.Response.StatusCode = 500; + context.Response.StatusCode = StatusCodes.Status500InternalServerError; context.Response.OnStarting(_clearCacheHeadersDelegate, context.Response); await _options.ExceptionHandler(context); - if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled("Microsoft.AspNetCore.Diagnostics.HandledException")) + if (context.Response.StatusCode != StatusCodes.Status404NotFound) { - _diagnosticListener.Write("Microsoft.AspNetCore.Diagnostics.HandledException", new { httpContext = context, exception = edi.SourceException }); + if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled("Microsoft.AspNetCore.Diagnostics.HandledException")) + { + _diagnosticListener.Write("Microsoft.AspNetCore.Diagnostics.HandledException", new { httpContext = context, exception = edi.SourceException }); + } + + return; } - // TODO: Optional re-throw? We'll re-throw the original exception by default if the error handler throws. - return; + _logger.ErrorHandlerNotFound(); } catch (Exception ex2) { diff --git a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs index c09bc66768..52dec57e34 100644 --- a/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs +++ b/src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerTest.cs @@ -7,17 +7,17 @@ using System.Diagnostics; using System.IO; using System.Linq; using System.Net; -using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.TestHost; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; using Xunit; namespace Microsoft.AspNetCore.Diagnostics @@ -469,5 +469,79 @@ namespace Microsoft.AspNetCore.Diagnostics "Alternatively, set one of the aforementioned properties in 'Startup.ConfigureServices' as follows: 'services.AddExceptionHandler(options => { ... });'.", exception.Message); } + + [Fact] + public async Task ExceptionHandlerNotFound_RethrowsOriginalError() + { + 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) => + { + Exception exception = null; + try + { + await next(); + } + catch (InvalidOperationException ex) + { + exception = ex; + + // This mimics what the server would do when an exception occurs + httpContext.Response.StatusCode = StatusCodes.Status500InternalServerError; + } + + // The original exception is thrown + Assert.NotNull(exception); + Assert.Equal("Something bad happened.", exception.Message); + + }); + + app.UseExceptionHandler("/non-existent-hander"); + + app.Map("/handle-errors", (innerAppBuilder) => + { + innerAppBuilder.Run(async (httpContext) => + { + await httpContext.Response.WriteAsync("Handled error in a custom way."); + }); + }); + + app.Map("/throw", (innerAppBuilder) => + { + innerAppBuilder.Run(httpContext => + { + throw new InvalidOperationException("Something bad happened."); + }); + }); + }); + }).Build(); + + await host.StartAsync(); + + using (var server = host.GetTestServer()) + { + var client = server.CreateClient(); + var response = await client.GetAsync("throw"); + 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."); + } } }