[Discussion] ExceptionHandlerMiddleware will throw original exception if exception handler cannot be found

[JunTaoLuo]

Discussion for aspnet/Announcements#434.

ExceptionHandlerMiddleware will throw original exception if exception handler cannot be found

Currently, the ExceptionHandlerMiddleware will execute the configured exception handler when an exception has occurred. If the exception handler, configured via ExceptionHandlerOptions.ExceptionHandlingPath for example, cannot be found, a 404 response will be produced. The 404 error is especially misleading since it makes it seem like an user error and obscures the fact that an exception occurred on the server.

To resolve this, the default behavior of ExceptionHandlerMiddleware will now throw the original exception if the exception handler could not be found. As a result, a 500 response will be produced by the server and it will be more obvious to examine the server logs when debugging the error that occurred.

Version introduced

ASP.NET Core 5.0 RC 1

Old behavior

The ExceptionHandlerMiddleware will currently produce a 404 error response when the configured exception handler could not be found when the middleware executes.

New behavior

By default, the ExceptionHandlerMiddleware will now throw the original exception when the configured exception handler could not be found (i.e. the exception handler returns a 404) when the middleware executes. To allow 404 responses to be returned by the exception handler, please configure the option AllowStatusCode404Response on ExceptionHandlerOptions to true. When this option is set to true, the current behavior is maintained.

Reason for change

We have often seen ExceptionHandlerMiddleware with misconfigured handlers producing 404 responses leading to confusion since the 404 error doesn't make it obvious an exception occurred on the server. This change is made so that a 500 error will be produced instead to make it more obvious that it's not caused by user error but rather an exception was encountered on the server.

Recommended action

There is no API change due to this breaking change so all existing apps will continue to compile and run. The exception thrown will be handled by the server. For example, the exception will be converted to a 500 error response by Kestrel HTTP server or HTTP.SYS server.

For backwards compatibility using the current behavior, where a 404 response from the exception handler is allowed, please set AllowStatusCode404Response on ExceptionHandlerOptions to true. See "New behavior" section for details.

Category

ASP.NET

Affected APIs

"Not detectable via API analysis"

---

Issue metadata

• Issue type: breaking-change

See discussion at: #25288

[KalleOlaviNiemitalo]

The implementation is apparently at #25062. It uses ExceptionDispatchInfo.Throw to preserve the original call stack of the exception. If the exception handler returns an HTTP status other than 404, then the middleware uses that response as is.

[crozone]

This is great. We've hit the existing pitfall where errors will return a 404 response because the error handler was misnamed or routing was broken in an update. What made this worse is that the developer error page middleware would obscure the issue until the code was deployed on staging or production.

[danikun]

I'm having an issue related to this. After migration to asp.net 5.0RC1 from 3.1, my lambda exception handler stopped working as expected. In it depending the exception thrown I set a status code or another, it works for all exceptions but the ones that in which I set status code 404. For those it says no exception handler has been found and rethrows the exception. Is that a bug or am I doing something wrong?

[JunTaoLuo]

Interesting, we made the change under the assumption that 404 status code wouldn't be intentionally returned by the exception handler and would only be returned when there's an issue while handling the error. It seems that assumption doesn't hold. See the discussion in the original change at: #25062 (comment). As mentioned there, you can still call CompleteAsync to make sure the response is flushed.

What's worth discussing is whether we should modify the condition so that it can differentiate between intentionally returned 404s and unintentional 404s due to the handler being missing.

[danikun]

Thanks for the fast response.

I tried with CompleteAsync and still not working, this is my lambda:

app.UseExceptionHandler(errorApp =>
            {
                errorApp.Run(async context =>
                {
                    var contextFeature = context.Features.Get<IExceptionHandlerFeature>();

                    if (contextFeature?.Error is EntityNotFoundException)
                    {
                        context.Response.StatusCode = (int)HttpStatusCode.NotFound;
                    }
                    if (contextFeature?.Error is ForbiddenException)
                    {
                        context.Response.StatusCode = (int)HttpStatusCode.Forbidden;
                    }

                    await context.Response.WriteAsJsonAsync(new { contextFeature?.Error.Message });
                    await context.Response.CompleteAsync();
                });
            }); 

Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware: Warning: No exception handler was found, rethrowing original exception.

[danikun]

ok, it's working if I run the real application and doing HTTP calls, but it doesn't work with my integration tests cause the exception rethrown prevents the test from finishing correctly.

[Tratcher]

Yeah, TestServer doesn't implement CompleteAsync yet. The warning will still get logged either way, but at least the client gets the response.

Can you open a new issue for your scenario? We have a short window to get feedback into 5.0 before it ships.

[danikun]

sure, thanks

[JunTaoLuo]

FYI, I have updated the description to include an option for backwards compatibility that we added in #26528

[marnilss]

This breaking-change left us trouble-shooting for two days until we finally found this little gem.
If anyone like us are using Microsoft.AspNetCore.Mvc.Testing (integration tests) and using lambda exceptionhandlers that intentionally return 404, with content, look out for an System.IOException and the exception message: "Error while copying content to a stream."

So it seems that the intended behavior by this change doesn't work when running Microsoft.AspNetCore.Mvc.Testing tests and returning 404 and content from ErrorHandler.

What we used (or really a simplification to highlight the problem):

app.UseExceptionHandler(builder =>
{
    builder.Run(context => context.WriteExceptionResponseAsync());
});

public static async Task WriteExceptionResponseAsync(this HttpContext context)
{
    // Simplified code to highlight issue
    var asJson = JsonConvert.SerializeObject(new ProblemDetails { Status = 404 });
    var bodyBuffer = Encoding.UTF8.GetBytes(asJson);
    context.Response.StatusCode = 404;
    await context.Response.Body.WriteAsync(bodyBuffer);
}

[ghost]

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!