Skip to content

Add Enabled flag to Microsoft.AspNetCore.Diagnostics.ExceptionHandlerFeature #3342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
franchyd opened this issue Jul 19, 2018 · 12 comments
Closed
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-diagnostics Diagnostic middleware and pages (except EF diagnostics)

Comments

@franchyd
Copy link

franchyd commented Jul 19, 2018

It would be nice to be able to enable/disable the exception handler pages on the fly in Microsoft.AspNetCore.Diagnostics.ExceptionHandlerFeature similar to how: Microsoft.AspNetCore.Diagnostics.StatusCodePagesFeature allows.

@Tratcher
Copy link
Member

How do you plan to handle the exception when the exception handler is disabled?

@franchyd
Copy link
Author

franchyd commented Jul 19, 2018

@Tratcher Perhaps I'm missing something, but I was under the impression that this is the feature that redirects to another page rather than just returning a 500 correct?

There are certain cases(Specifically on my API routes) in which I would rather have the raw 500 returned than an HTML laden error page, and in a site hosting both web content and an API, the options are limited for that, right now I have been forced to add an additional middleware that looks like this:

public class RemoveExceptionHandlerMiddleware
{
    private readonly RequestDelegate next;
    private readonly List<string> prefixesToRemove;

    public RemoveExceptionHandlerMiddleware(RequestDelegate next, List<string> prefixesToRemove)
    {
        this.next = next;
        this.prefixesToRemove = prefixesToRemove;

        // do the lowercase operation one time here so that it isn't called every iteration in the pipeline
        for (int i = 0; i < prefixesToRemove.Count; i++)
            prefixesToRemove[i] = prefixesToRemove[i].ToLower();
    }

    public async Task Invoke(HttpContext context)
    {
        string curPathLower = context.Request.Path.Value.ToLower();

        if (prefixesToRemove.Any(x => curPathLower.StartsWith(x)))
        {
            try
            {
                await next.Invoke(context);
            }
            catch
            {
                context.Response.Clear();
                context.Response.StatusCode = 500;
            }
        }
        else
        {
            await next.Invoke(context);
            return;
        }
    }
}

My main aversion to my current method is that I need to declare a bunch of magic URL's in my startup rather than being able to implement this using a less magic stringy method such as attributes.

@Tratcher
Copy link
Member

Ah, so you still want it to handle the exception, but to conditionally produce a different result. That makes some sense.

Enabling/disabling the exception handler in ever API controller would also be tedious. Have you considered customizing the error endpoint you're redirected to? E.g. it could check the Accept header and decide if HTML is appropriate.

@franchyd
Copy link
Author

I guess that that would be an option I hadn't considered. But it seems a bit odd to still be doing the redirect just to return the same status code from somewhere else though.

@Tratcher
Copy link
Member

Depending on how you're using the middleware, it's not a full redirect, it's an internal re-route (See WithReExecute).

@franchyd
Copy link
Author

franchyd commented Jul 19, 2018

Right, I meant it was odd to reroute the request and walk its way back down the pipeline to my error endpoint to return a 500 status code, when it could just immediately return within the ErrorHandlerMiddleware where it had already set a 500.

@Tratcher
Copy link
Member

Sure, but conditionally signaling it to do so is the hard part. You don't want to add code to all of your WebAPI endpoints to suppress it.

@Tratcher
Copy link
Member

Also, this overlaps the discussion from #2573

@pranavkm
Copy link
Contributor

There are certain cases(Specifically on my API routes) in which I would rather have the raw 500 returned

Might not be super relevant here - but there are some plans to address this experience as part of 2.2.0. See https://github.com/aspnet/Mvc/issues/7756

@franchyd
Copy link
Author

franchyd commented Jul 19, 2018

I was thinking along the lines of an IActionFilter attribute that just runs

context.HttpContext.Features.Get<IExceptionHandlerFeature>().Enabled = false;

Which I could then add onto entire controller classes to blanket apply across them.

@franchyd
Copy link
Author

@pranavkm Actually that is pretty much exactly my issue.

@franchyd
Copy link
Author

Well I guess since there are plans to address this experience moving forward, and since I have a viable workaround in the meantime, this ticket is no longer needed.

@Eilon Eilon added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-diagnostics Diagnostic middleware and pages (except EF diagnostics) and removed repo:Diagnostics labels Nov 26, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
@amcasey amcasey added the area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares label Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-diagnostics Diagnostic middleware and pages (except EF diagnostics)
Projects
None yet
Development

No branches or pull requests

5 participants