Add extensible handlers to DeveloperExceptionPage

[DamianEdwards]

Add extensibility point to sub-systems to add handlers via DI that get inspected when an exception occurs. The handler can choose to render a response (e.g. the current EF diagnostic middleware page would change to this) or pass-through (do nothing). The middleware would fallback to rendering an HTML error page if no handler matched and the client sent an Accept header that included text/html, otherwise it would just return the error as plain text. For API responses, MVC would register a handler that formats a ProblemDetails with the relevant information according to the configured formatters.

There's 2 work items here:

• - Allowing components to take over the rendering of the error page depending on the exception type (like how EF has the database error page)
• - Rendering a plaintext response if the client isn't a browser (if the Accept header does not contain text/html)

[davidfowl]

As a strawman design for the first part of this issue, we could introduce a simple interface like this:

public interface IDeveloperPageExceptionFilter
{
    Task HandleExceptionAsync(HttpContext context, Exception exception, Func<HttpContext, Exception, Task> next);
}

This follows the filter pattern we have elsewhere in the product (middleware, startup filters etc)

• Short circuiting the default logic means you don't call next
• With access to the Exception and next you can change the exception type in a filter and call next with a more specific exception (it would allow for doing things like exception unwrapping)

We could provide an ExceptionContext in case we wanted to expand it further in the future with more data (it clashes with the ExceptionContext type in MVC though so maybe a different name).

cc @rynowak @Tratcher

[rynowak]

This pattern seems OK, how do we decide on the order? Are these in DI or options? What's the lifetime? Those are some of the rough edges in MVC's filters. If it's based on options then the answers are pretty straightforward IMO.

+1 for the idea of chain of responsibility here. Is the idea that we register the current behaviour as the last filter?

I'm not sure the idea of transforming the exception is valuable. I think it's fine if the contract allows it, but I don't see why its necessary.

I don't think we need a context here. If we get that far, we might want to consider using features or enhancing existing contracts on http context.

note: since nothing clears the endpoint on throw - you could use endpoint metadata in one of these filters.

[davidfowl]

This pattern seems OK, how do we decide on the order?

DI order, like IStartupFilters. I don't think the order will matter that much in practice as I'm hoping the majority use case is to filter on exception type if you're not wholesale changing exception formatting (then you just need to be the first).

Are these in DI or options?

DI.

What's the lifetime?

Singleton

I'm not sure the idea of transforming the exception is valuable. I think it's fine if the contract allows it, but I don't see why its necessary.

Turning an Aggregate into something else or a TargetInvocationException into something. It might be contrived but we'll see what people do.

note: since nothing clears the endpoint on throw - you could use endpoint metadata in one of these filters.

👍 That's pretty useful.

[rynowak]

DI order, like IStartupFilters. I don't think the order will matter that much in practice as I'm hoping the majority use case is to filter on exception type if you're not wholesale changing exception formatting (then you just need to be the first).

I think there's another important dimension here - what is the client? We've wanted for a while to have a JSON version for APIs.

If the framework registers handlers for you then it won't be straightforward to resolve ordering problems.

Turning an Aggregate into something else or a TargetInvocationException into something. It might be contrived but we'll see what people do.

If that's the case though, then I wouldn't do it that way as a user. It's a little too clever.

If there are places the framework always throws a TIE or an AggyBoy we should treat those as bugs should being unwrapping for you.

[rynowak]

Imagine registering a handler that does problemdetails for an API based on IApiBehaviourMetadata ([ApiController])

[davidfowl]

I think there's another important dimension here - what is the client? We've wanted for a while to have a JSON version for APIs.
If the framework registers handlers for you then it won't be straightforward to resolve ordering problems.

That's a good question. If you look at the original issue, we also want to change the output to the plain text exception if text/html isn't in the accept header (or if there's no accept header). That's the lowest common denominator and wouldn't be expressed as a filter (it'll just be built in).

So that was the intent. If a filter ends up being greedy it'll never run but I don't know that we want to do any more complicated matching (content type/metadata/exception type). I think in a typical application with MVC and EF, we'll have 2 of these filters:

• The one you mention that uses IApiBehaviourMetadata on the Endpoint to determine if to show JSON (or use conneg etc)
• The EF database error page filter (that would replace the database error page)

The EF one will show up only if the exception is of a specific type so it won't be greedy. Can you think of other things that might cause issues in practice?

If there are places the framework always throws a TIE or an AggyBoy we should treat those as bugs should being unwrapping for you.

Maybe, it means you lose the ability to do your own formatting of those types of exceptions. I wouldn't want to hide that type of information. I just threw that out as a possible example of a thing that would change the exception type. I don't know if anyone would ever need to do it though.

[Eilon]

FYI here are previous bugs tracking related (similar/same) stuff:

• Very related:
  • DeveloperExceptionPageMiddleware always returns HTML, sometimes JSON would be better #2590 - DeveloperExceptionPageMiddleware always returns HTML, sometimes JSON would be better
  • Provide in-the-box handling for page vs. API errors #2573 - Provide in-the-box handling for page vs. API errors
• Not quite related, but related contepts:
  • Passing more than Status Code to the Error action #2591 - Passing more than Status Code to the Error action
  • Define handlers for status codes with UseStatusCodePages #2589 - Define handlers for status codes with UseStatusCodePages

If the 'very related' bugs are entirely covered by this, should close those.

[rynowak]

Maybe, it means you lose the ability to do your own formatting of those types of exceptions. I wouldn't want to hide that type of information.

I'm referring to cases where the TIE or AggyBoy doesn't have any information. If you use reflection to invoke a method, there's no real downside to unwrapping the TIE you get back. Likewise for a Task.Result.

I realise this is just banter at this point 😆

[davidfowl]

Moving this to preview5, here's what I think we should do:

• Support the plaintext option. I think it'll break a bunch of tests since lots of them use TestServer and assume HTML will come back (this should be easy to verify)
• Try to implement
  • MVC JSON formatting
  • The EF page - This will return into code in ConfigureServices vs Configure since something will need to add the EF middleware (assuming it'll be application code for now).

[Eilon]

So in the con-neg you want plaintext to be the fallback? Or should the real fallback still be text/html if no Accept-Type is sent?

[Tratcher]

If you were doing real content negotiation you'd be honoring the client's Accept ordering preference. That doesn't match up with this pipeline model as well.

text/plain is the safest fallback, and the easiest to understand in the common dev/debug scenario. I always hate having to dig through HTML in the debugger, test logs, etc..

[Eilon]

Con-neg doesn't require honoring anything! 😄 And it says nothing about the ultimate fallback in case no desired content types are supported. I agree that plaintext is simplest to see, but I wanted to understand if there's any consideration to "compatibility" or "doing what most people would expect."

[analogrelay]

With #9342 merged, is this done? We now have a plain-text exception page.

[davidfowl]

Yes I'll file another IIS for the other work items (MVC and EF)

[IanKemp]

As far as I can see, this item should not be marked as done, as it only addresses the second point in the OP. The first is still very much a missing feature.

[davidfowl]

Not sure what you mean.