UseExceptionHandler cannot handle returning a 404 error when not running in IIS Express without causing an HTTP protocol error #31024
Assignees
Labels
area-networking
Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
bug
This issue describes a behavior which is not expected - a bug.
Milestone
Comments
That looks like it came from TestServer rather than a real HttpClient instance? The protocol error may be related to #24502, where Kestrel didn't used to filter out headers that were prohibited by HTTP/2. That said, the exception handler only sets cache headers which should be allowed: Try turning on kestrel's connection logger to capture the response headers: |
|
You are correct that the httpclient exception was coming from one configured from a test server. I tried hitting this endpoint in an integration test (which is actually how I found the error to begin with). The code below is how the server and httpclient was set up in case you are curious. Its nothing special. _factory = new TestWebApplicationFactory<Startup>();
_client = _factory.CreateClient();As requested, though, here are the logs from doing what you have requested. I also put the log level to trace for added help. I have also added the code used to produce this on the kestrel-connection-logging branch in the same repo. When NOT running in IIS Express: (If I did this incorrectly, let me know) |
|
Ok, it doesn't look like a headers issue, I only see the expected headers here: The client might be reporting this RST as a protocol error: The RST is caused because the exception hander re-threw the original exception after the your custom code. The reason this specifically affects a 404 is because of this check: We found that too often the exception handler was pointed at a missing error page. You can override that by setting AllowStatusCode404Response. There was an earlier HasStarted check to make sure we didn't try to render an error page in the middle of an existing page, but that check isn't repeated after running the exception handler. Proposal: Check HasStarted before doing the 404 check. Do not rethrow if the exception handler started the response (unless it threw). General guidance: Don't use exceptions as a normal means of producing status codes. The ExceptionHandler is only meant as a fallback for unexpected exceptions. MVC controllers should set the status code themselves for normal business logic scenarios. |
|
Ah ok, so this is by design. Also thank you for your guidance. If I still need to handle a 404 in the exception handler, I see that I need to set that property to true. Looking through the code, I found this test which shows how to configure this, although this doesn't seem to work. I'm now getting 500s on the front end. Am I doing something wrong or is there a further issue here? .Configure<ExceptionHandlerOptions>(options =>
{
options.AllowStatusCode404Response = true;
options.ExceptionHandler = async context =>
{
var errorFeature = context.Features.Get<IExceptionHandlerFeature>();
var ex = errorFeature.Error;
context.Response.StatusCode = ex switch
{
DomainException domainEx => StatusCodes.Status400BadRequest,
NotFoundException notFoundEx => StatusCodes.Status404NotFound,
_ => StatusCodes.Status500InternalServerError,
};
await context.Response.WriteAsync("There was an error caught in the exception handler lambda");
};
}); |
Tratcher
added
the
bug
|
You'll still need to call UseExceptionHandler() to include the middleware. |
|
Thanks. That was the part that I was missing. I updated the example repo with the resolution. |
|
Triage: Relevant comment about fixing the bug #31024 (comment) |
BrennanConroy
added
the
help wanted
|
We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process. |
|
Thanks for contacting us. |
BrennanConroy
removed
the
help wanted
|
I have an alternative suggestion. Instead of rethrowing the original Exception, throw a custom suggestion suggesting configuring |
ghost
locked as resolved and limited conversation to collaborators
amcasey
added
area-networking
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Describe the bug
When returning a 404 within UseExceptionHandler when the api is not running in IIS Express, http protocol errors happen. There are no issues when running in IIS Express.
To Reproduce
Here is an example repo that reliably reproduces this error.
Use a handler like the following:
Then use a controller like the following
Lastly, run the app NOT in IIS Express and hit the endpoint that causes the middleware to set the status code to
404. In this example, that is/throw-not-found.It should also be noted that setting the response code to anything else in the middleware circumvents the issue.
It should also be noted that using normal middleware works fine. So replacing the aforementioned middleware with the one below will fix the issue.
Exceptions (if any)
This is what the chrome dev tools logs when I hit this endpoint using Swagger UI
Hitting the endpoint with something like
HttpClientwill cause the following error:System.Net.Http.HttpRequestException: Error while copying content to a stream. ---> System.IO.IOException: ---> NotFoundBugExample.NotFoundBugExampleFurther technical details
net5.0dotnet --infoThe text was updated successfully, but these errors were encountered: