aspnet / Diagnostics

[Archived] Diagnostics middleware for reporting info and handling exceptions and errors in ASP.NET Core, and diagnosing Entity Framework Core migrations errors. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
213 stars 108 forks source link

UseStatusCodePagesWithReExecute behaves differently in 2.1.x (migration from 1.1.x) #478

Closed mkArtakMSFT closed 6 years ago

mkArtakMSFT commented 6 years ago

From @drauch on Monday, 10 September 2018 08:56:29

Hi!

We migrated from ASP.NET Core 1.1.x to 2.1.x and run into problems with our UseStatusCodePagesWithReExecute() middleware.

Using UseStatusCodePagesWithReExecute("/Error") behaves different now. Previously, if the user was at https://server/WebApp/12345 and run into an exception we could obtain 12345 from the route values when executing the Error controller action. Now it is not possible anymore.

Are the route values cleared in some way now? Why is this behaving differently? How to obtain 12345 when executing the Error action?

Maybe related: Before upgrading we hosted our application at https://server/WebApp/, which is no longer possible in ASP.NET Core 2.1.x. We must host it at https://server/ now and use UsePathBase("/WebApp") instead.

Copied from original issue: aspnet/Mvc#8437

mkArtakMSFT commented 6 years ago

From @mkArtakMSFT on Monday, 10 September 2018 16:46:43

Thanks for contacting us, @drauch. @rynowak, can you please look into this? Thanks!

Tratcher commented 6 years ago

The original request path is stored in the feature collection: https://github.com/aspnet/Diagnostics/blob/f94ad0f2029243f116f7639e50a5c10ef63a63c5/src/Microsoft.AspNetCore.Diagnostics/ExceptionHandler/ExceptionHandlerMiddleware.cs#L72-L77

drauch commented 6 years ago

@Tratcher : So, this is a breaking change from 1.1.x to 2.x?

Tratcher commented 6 years ago

I don't know about any changes to route values, but IExceptionHandlerPathFeature.Path is the expected way to get access to the original value. This was added in 1.1 via #292.

rynowak commented 6 years ago

Is your /Error running through MVC? If so, I'm not sure how that would be been possible in 1.x. Routing would create a new collection of route values and stash them on the context.

rynowak commented 6 years ago

We may have had a bug at some point in 1.x that was fixed around routing + re-execute accidentally retaining route values. it's definitely not intended that you'd re-evaluate routing and still see route values from the previous match.

drauch commented 6 years ago

IExceptionHandlerPathFeature.Path

Thank you, I'm using that now. Issue can be closed.

rynowak commented 6 years ago

If you want this behavior btw, you could implement something like it using a middleware:

app.Use((next) => async (context) =>
{
    try
    {
         await next(context);
    }
    finally
    {
        context.Properties["oldroutedata"] = context.GetRouteData();
    }
} 

Register this in between your status code pages and MVC, it will capture the route data in a property of the context when the request is unwinding. Then look for it with the "oldroutedata" key in your error handler.