Skip to content

Commit 4c381e8

Browse files
author
John Luo
authored
Throw original exception if exception handler is not found (#25062)
* Throw original exception if exception handler is not found
1 parent 9a8a1d4 commit 4c381e8

File tree

3 files changed

+94
-8
lines changed

3 files changed

+94
-8
lines changed

src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -19,6 +19,9 @@ internal static class DiagnosticsLoggerExtensions
1919
private static readonly Action<ILogger, Exception> _errorHandlerException =
2020
LoggerMessage.Define(LogLevel.Error, new EventId(3, "Exception"), "An exception was thrown attempting to execute the error handler.");
2121

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+
2225
// DeveloperExceptionPageMiddleware
2326
private static readonly Action<ILogger, Exception> _responseStartedErrorPageMiddleware =
2427
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 @@ public static void ErrorHandlerException(this ILogger logger, Exception exceptio
4144
_errorHandlerException(logger, exception);
4245
}
4346

47+
public static void ErrorHandlerNotFound(this ILogger logger)
48+
{
49+
_errorHandlerNotFound(logger, null);
50+
}
51+
4452
public static void ResponseStartedErrorPageMiddleware(this ILogger logger)
4553
{
4654
_responseStartedErrorPageMiddleware(logger, null);

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,18 +113,22 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
113113
};
114114
context.Features.Set<IExceptionHandlerFeature>(exceptionHandlerFeature);
115115
context.Features.Set<IExceptionHandlerPathFeature>(exceptionHandlerFeature);
116-
context.Response.StatusCode = 500;
116+
context.Response.StatusCode = StatusCodes.Status500InternalServerError;
117117
context.Response.OnStarting(_clearCacheHeadersDelegate, context.Response);
118118

119119
await _options.ExceptionHandler(context);
120120

121-
if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled("Microsoft.AspNetCore.Diagnostics.HandledException"))
121+
if (context.Response.StatusCode != StatusCodes.Status404NotFound)
122122
{
123-
_diagnosticListener.Write("Microsoft.AspNetCore.Diagnostics.HandledException", new { httpContext = context, exception = edi.SourceException });
123+
if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled("Microsoft.AspNetCore.Diagnostics.HandledException"))
124+
{
125+
_diagnosticListener.Write("Microsoft.AspNetCore.Diagnostics.HandledException", new { httpContext = context, exception = edi.SourceException });
126+
}
127+
128+
return;
124129
}
125130

126-
// TODO: Optional re-throw? We'll re-throw the original exception by default if the error handler throws.
127-
return;
131+
_logger.ErrorHandlerNotFound();
128132
}
129133
catch (Exception ex2)
130134
{

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

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,17 @@
77
using System.IO;
88
using System.Linq;
99
using System.Net;
10-
using System.Text;
1110
using System.Threading;
1211
using System.Threading.Tasks;
1312
using Microsoft.AspNetCore.Builder;
1413
using Microsoft.AspNetCore.Hosting;
1514
using Microsoft.AspNetCore.Http;
16-
using Microsoft.AspNetCore.Http.Features;
1715
using Microsoft.AspNetCore.TestHost;
1816
using Microsoft.AspNetCore.Testing;
1917
using Microsoft.Extensions.DependencyInjection;
2018
using Microsoft.Extensions.Hosting;
19+
using Microsoft.Extensions.Logging;
20+
using Microsoft.Extensions.Logging.Testing;
2121
using Xunit;
2222

2323
namespace Microsoft.AspNetCore.Diagnostics
@@ -469,5 +469,79 @@ public void UsingExceptionHandler_ThrowsAnException_WhenExceptionHandlingPathNot
469469
"Alternatively, set one of the aforementioned properties in 'Startup.ConfigureServices' as follows: 'services.AddExceptionHandler(options => { ... });'.",
470470
exception.Message);
471471
}
472+
473+
[Fact]
474+
public async Task ExceptionHandlerNotFound_RethrowsOriginalError()
475+
{
476+
var sink = new TestSink(TestSink.EnableWithTypeName<ExceptionHandlerMiddleware>);
477+
var loggerFactory = new TestLoggerFactory(sink, enabled: true);
478+
479+
using var host = new HostBuilder()
480+
.ConfigureWebHost(webHostBuilder =>
481+
{
482+
webHostBuilder
483+
.UseTestServer()
484+
.ConfigureServices(services =>
485+
{
486+
services.AddSingleton<ILoggerFactory>(loggerFactory);
487+
})
488+
.Configure(app =>
489+
{
490+
app.Use(async (httpContext, next) =>
491+
{
492+
Exception exception = null;
493+
try
494+
{
495+
await next();
496+
}
497+
catch (InvalidOperationException ex)
498+
{
499+
exception = ex;
500+
501+
// This mimics what the server would do when an exception occurs
502+
httpContext.Response.StatusCode = StatusCodes.Status500InternalServerError;
503+
}
504+
505+
// The original exception is thrown
506+
Assert.NotNull(exception);
507+
Assert.Equal("Something bad happened.", exception.Message);
508+
509+
});
510+
511+
app.UseExceptionHandler("/non-existent-hander");
512+
513+
app.Map("/handle-errors", (innerAppBuilder) =>
514+
{
515+
innerAppBuilder.Run(async (httpContext) =>
516+
{
517+
await httpContext.Response.WriteAsync("Handled error in a custom way.");
518+
});
519+
});
520+
521+
app.Map("/throw", (innerAppBuilder) =>
522+
{
523+
innerAppBuilder.Run(httpContext =>
524+
{
525+
throw new InvalidOperationException("Something bad happened.");
526+
});
527+
});
528+
});
529+
}).Build();
530+
531+
await host.StartAsync();
532+
533+
using (var server = host.GetTestServer())
534+
{
535+
var client = server.CreateClient();
536+
var response = await client.GetAsync("throw");
537+
Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
538+
Assert.Equal(string.Empty, await response.Content.ReadAsStringAsync());
539+
}
540+
541+
Assert.Contains(sink.Writes, w =>
542+
w.LogLevel == LogLevel.Warning
543+
&& w.EventId == 4
544+
&& w.Message == "No exception handler was found, rethrowing original exception.");
545+
}
472546
}
473547
}

0 commit comments

Comments
 (0)