Throw original exception if exception handler is not found

[JunTaoLuo]

Fixes #21510.

In the event where the exception handler could not be resolved (i.e. 404) I've taken the third approach as suggested in #21510 (comment) which is to re-throw the original error.

The problem would also be logged:

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/2 GET https://localhost:5001/throw - -fail: Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware[1]
      An unhandled exception has occurred while executing the request.
      System.Exception: Application Exception
         at ExceptionHandlerSample.Startup.<>c.<Configure>b__0_3(HttpContext context) in C:\gh\aspnetcore\src\Middleware\Diagnostics\test\testassets\ExceptionHandlerSample\Startup.cs:line 27
         at Microsoft.AspNetCore.Builder.Extensions.MapMiddleware.Invoke(HttpContext context) in C:\gh\aspnetcore\src\Http\Http.Abstractions\src\Extensions\MapMiddleware.cs:line 70
         at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.<Invoke>g__Awaited|6_0(ExceptionHandlerMiddleware middleware, HttpContext context, Task task) in C:\gh\aspnetcore\src\Middleware\Diagnostics\src\ExceptionHandler\ExceptionHandlerMiddleware.cs:line 75
warn: Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware[4]
      No exception handler was found, rethrowing original exception.
fail: Microsoft.AspNetCore.Server.Kestrel[13]
      Connection id "0HM24OM464IUL", Request id "0HM24OM464IUL:00000003": An unhandled exception was thrown by the application.
      System.Exception: Application Exception
         at ExceptionHandlerSample.Startup.<>c.<Configure>b__0_3(HttpContext context) in C:\gh\aspnetcore\src\Middleware\Diagnostics\test\testassets\ExceptionHandlerSample\Startup.cs:line 27
         at Microsoft.AspNetCore.Builder.Extensions.MapMiddleware.Invoke(HttpContext context) in C:\gh\aspnetcore\src\Http\Http.Abstractions\src\Extensions\MapMiddleware.cs:line 70
         at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.<Invoke>g__Awaited|6_0(ExceptionHandlerMiddleware middleware, HttpContext context, Task task) in C:\gh\aspnetcore\src\Middleware\Diagnostics\src\ExceptionHandler\ExceptionHandlerMiddleware.cs:line 75
         at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.HandleException(HttpContext context, ExceptionDispatchInfo edi) in C:\gh\aspnetcore\src\Middleware\Diagnostics\src\ExceptionHandler\ExceptionHandlerMiddleware.cs:line 143
         at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.<Invoke>g__Awaited|6_0(ExceptionHandlerMiddleware middleware, HttpContext context, Task task) in C:\gh\aspnetcore\src\Middleware\Diagnostics\src\ExceptionHandler\ExceptionHandlerMiddleware.cs:line 85
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application) in C:\gh\aspnetcore\src\Servers\Kestrel\Core\src\Internal\Http\HttpProtocol.cs:line 651
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished HTTP/2 GET https://localhost:5001/throw - - - 500 0 - 49.1896ms

[JunTaoLuo]

Should we handle other 4xx statuses? Also, what about the case where the configured exception handler was executed correctly and it itself returned a 404? We would be overriding what the exception handler did. It seems like an odd behaviour for an exception handler but theoretically, we would be breaking them.

[Tratcher]

Should we handle other 4xx statuses?

404s are the main concern, that's what most people do by accident. A few trigger auth challenges (301 or 401) if they forget to allow anonymous requests to their error page, but those should produce their own logs to make them somewhat discoverable.

Also, what about the case where the configured exception handler was executed correctly and it itself returned a 404? We would be overriding what the exception handler did. It seems like an odd behaviour for an exception handler but theoretically, we would be breaking them.

In theory that could break, but I'd expect it to be far less common than the current issue. In that scenario they could call CompleteAsync to make sure the response was flushed regardless of the exception being rethrown.

[lugospod]

Hi @JunTaoLuo,

Regarding your comment, it turns out that the assumptions with 404 doesn't hold.

If my app. has a global exception handler, and it is catching custom Exceptions and determines "Oh look, this problem was caused by missing data" and it decides to put context.Response.StatusCode = 404 (in the ExceptionHandlerMiddlware), then first problem is that the response doesn't get sent (the workaround that I used is exactly the one you recommended, to flush the response using CompleteAsync method), but the second problem is that the ExceptionHandlerMiddlware won't handle that exception and it will propagate the original exception as if no exception handler was found - resulting in other middlerwares receiving the original exception.

Currently the only thing I can think of to help myself is not to use 404 for NO DATA FOUND, and use some other status code until this problem gets fixed.

Is there a plan to change this assumption that was made regarding 404 error?

Tnx!

[JunTaoLuo]

We added an option to allow 404 response codes: https://github.com/dotnet/aspnetcore/blob/main/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerOptions.cs#L33. We also updated the error message to make this more obvious.

[sebas-bellxcel]

I think this still not resolved, Im using globalExceptionHandler like this:
app.UseGlobalExceptionHandler(x =>
{
x.ContentType = "application/json";
x.Map().ToStatusCode(StatusCodes.Status404NotFound)
.WithBody((ex, context) =>
{
return JsonConvert.SerializeObject(new
{
ex.Message
});
});
});
when I use that exception, it will throw the invalid operation exception

In my case Im not using the exception handler because I have my own middleware, there is an option to add this allow404 property to the global exception handler?
also I add this:
services.AddExceptionHandler((optiones) =>
{
optiones.AllowStatusCode404Response = true;
optiones.ExceptionHandlingPath = "/error";
});
but that change everything, without the ExceptionHandlingPath the fix does not work, and using it, breakes my middleware, because Im using an API, I must not change or redirect routes when throwing an error, there is a workarrond that actually works for this case?, or I will have to change to user another StatusCode, that would be awful but I guess there are no options.
Running on Net 6 btw.