Error page can't handle POST errors

[gbbsoft]

I think there is a bug in ExceptionHandlerMiddleware.cs.
if exception is throw during "POST" then Exception Handler call ExceptionHandlingPath also using "POST". I think it should call ExceptionHandlingPath using "GET", so please add ClearHttpContext following line:

context.Request.Method = "GET";

[analogrelay]

This seems somewhat reasonable. It would be breaking to existing users though so we'll have to introduce it as a new option.

[analogrelay]

We would review a PR contributing this fix if you're interested @gbbsoft .

[analogrelay]

One thing that does come to mind though is that since this would have to be an opt-in feature, it might not solve the problem most users would have with this behavior. If the original problem is that the exception page logic is matching against Method and not expecting POST then a simple fix is to change the logic to allow POSTs. If we add this functionality, it would also require the user to understand this problem and change their app, so I'm not sure exactly what the value here is.

[gbbsoft]

More interesting is when exception is in "DELETE" or "UPDATE"... :-)

[rynowak]

One thing that does come to mind though is that since this would have to be an opt-in feature, it might not solve the problem most users would have with this behavior.

Why does it have to be opt-in? We wouldn't add a new option in a patch, so it would be in 5.0 which is a major.

Playing devils advocate a bit - this might have been the first time we've heard this complaint. Are more people depending on the current behavior?

[analogrelay]

@rynowak Sure, we could also just take the break.

[gbbsoft]

We would review a PR contributing this fix if you're interested @gbbsoft .

What does it mean?

[analogrelay]

@gbbsoft I was suggesting that if you wanted to contribute this fix yourself, we'd be happy to review a Pull Request. If not, that's fine, it's on our queue and we'll look at it for a future release.

[Tratcher]

FYI this has come up at least once before and we declined to change it. #3555. I don't recommend taking this change now.

We want to preserve as much information as possible so the error page can accurately report what the original request was trying to do. Note we preserve the request body, method, query, headers, etc.. Changing only some of them would put the request in an inconsistent state (e.g. a GET request with a Content-Length header, a body, query parameters for some other page, etc..) and the results will be unpredictable.

Changing the path is necessary for re-routing the requests. The original path is preserved in a request feature: https://github.com/dotnet/aspnetcore/blob/master/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerFeature.cs.

The alternative is trying to capture the complete state of the original request and creating a completely new request structure for the exception handling. That's expensive, error prone, breaks extensibility, and fragile to maintain.

[Tratcher]

More interesting is when exception is in "DELETE" or "UPDATE"... :-)

This does mean your exception handling endpoint needs to be capable of handling a wide variety of request. DELETE and UPDATE don't worry me, you control that endpoint, you control the behavior of any methods applied there.

[gbbsoft]

@Tratcher So you can add option in ExceptionHandlerOptions: "bool ForceGET" (or similar) with default to false. This will not break any existing code.

And add in ExceptionHandlerFeature variable to remember original operation.

[Tratcher]

That doesn't address the issue of the request being in an inconsistent state.

[vaughanr]

@Tratcher your explanation makes sense to me. I did the pr just because it was marked as a good first issue. I'm happy to withdraw it myself and find a better one.

[Tratcher]

Thanks @vaughanr. At the very least this needs more design before we can proceed to a PR.

[gbbsoft]

My main issue was: when exception had been during "POST" my error page was not called at all.

[Tratcher]

@gbbsoft right. That can be addressed in routing like in #3555. How is the route set up for your error page?

[gbbsoft]

My application is more complicate, because I have isolated multi-instance of my application, so path is https://gbbdance.gbbsoft.pl/eM_Studio/ where "eM_Studio" is an instance name. Than I created special ExceptionHandlerMiddleware to pass instance name to /Error, because there is no official way to create Path to Error dynamical.

Except it my Config() is "normal":

            app.UseMiddleware<GbbDance.Lib.ExceptionHandlerMiddleware>(new object[1]
                {
                    Microsoft.Extensions.Options.Options.Create(new ExceptionHandlerOptions
                        {
                            ExceptionHandlingPath = new PathString("/Error")
                        })
                });

And /Error was from template with small modifications:

             [ResponseCache(Duration = 0, Location = ResponseCacheLocation.None, NoStore = true)]
             [AllowAnonymous]
             public class ErrorModel : PageModel
                {
                    private readonly Data.ApplicationDbContext DB;
                    public ErrorModel(Data.ApplicationDbContext db)
                    {
                        DB = db;
                    }
                    public string RequestId { get; set; }
                    public string Message { get; set; }
                    public bool ShowRequestId => !string.IsNullOrEmpty(RequestId);
                    public void OnGet()
                    {
                        RequestId = Activity.Current?.Id ?? HttpContext.TraceIdentifier;
                        var exceptionHandlerPathFeature = HttpContext.Features.Get<IExceptionHandlerPathFeature>();
                        if (exceptionHandlerPathFeature != null)
                        {
                             // get message from exception, sometimes convert it, and save it in DB
                            Message = DB.OurGetExceptionMessageUI(exceptionHandlerPathFeature.Error, exceptionHandlerPathFeature.Path, Request.QueryString.ToString());
                        }
                    }
                }

Later I had to add:

    // 2019-09-19: called then error on any "OnPost" procedures.
    public void OnPost()
    {
        OnGet();
    }

Later I rewrote ExceptionHandlerMiddleware to better process exceptions in OnPosts.

[Tratcher]

So the only part you needed to workaround the title issue is this?

    public void OnPost()
    {
        OnGet();
    }

@pranavkm is there a way to make a razor page handle any method? We should update our error page.

aspnetcore/src/ProjectTemplates/Web.ProjectTemplates/content/RazorPagesWeb-CSharp/Pages/Error.cshtml.cs

Line 26 in 44e4493

public void OnGet()

@NTaylorMullen said this would work on a normal controller, so our other template is probably not affected.

aspnetcore/src/ProjectTemplates/Web.ProjectTemplates/content/StarterWeb-CSharp/Controllers/HomeController.cs

Line 41 in 44e4493

public IActionResult Error()

[gbbsoft]

Looking at this today: Yes. The simple (but I don't know if the best) is add OnPost with redirection to OnGet in /Error

The idea to correct template is very good! I spend some hours to find out why /Error is not called during OnPost.

[pranavkm]

@pranavkm is there a way to make a razor page handle any method? We should update our error page.

The Page will still execute if no handler matches. A fairly trivial way to fix this would be to lazily evaluate RequestId:

public string RequestId => Activity.Current?.Id ?? HttpContext.TraceIdentifier;

public bool ShowRequestId => !string.IsNullOrEmpty(RequestId);

private readonly ILogger<ErrorModel> _logger;

public ErrorModel(ILogger<ErrorModel> logger)
{
    _logger = logger;
}

[mkArtakMSFT]

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.

[halter73]

If we change the behavior here, we should add the original verb to the IExceptionHandlerPathFeature.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[mikekistler]

This issue has been quiet for several years now. Many users have posted that the workaround described above works in their apps, so I'm going to close this as resolved. But if you are still encountering this issue and can't use the workaround given please add a new reply and re-open the issue.