Mapping Exception x StatusCode in ExceptionHandlerMiddleware

[brunolins16]

Background and Motivation

In .NET 7 the ExceptionHandlerMiddleware was updated to generate a Problem Details when the IProblemDetailsService is registered.

While this change allows a consistent ProblemDetails for the application the default handler for ExceptionHandlerMiddleware always sets the response status code to 500 and changing to produce a different Status Code requires a custom implementation (as mentioned here #43831).

Proposed API

The community ProblemDetails middleware (https://github.com/khellang/Middleware) has a capability to map an Exception to Status Code when producing the payload. Since the ExceptionHandlerMiddleware is the built-in feature to handle exception, my suggestion is to add a similar feature to allow mapping exception type X status code every time an exception is handled.

namespace Microsoft.AspNetCore.Builder;

public class ExceptionHandlerOptions
{
+    public void Map<TException>(int statusCode) where TException : Exception {}
+    public bool TryGetMap(Exception exception, out int statusCode) {}
}

Usage Examples

Configuring the ExceptionHandlerOptions

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.
builder.Services.AddExceptionHandler(o => 
{
    o.Map<InvalidOperationException>(400);
});

var app = builder.Build();

app.UseExceptionHandler();

app.MapGet("/", () =>
{
    throw new InvalidOperationException();
});

app.Run();

Setting ExceptionHandlerOptions directly to the middleware

var app = WebApplication.Create(args);

var options = new ExceptionHandlerOptions();
options.Map<InvalidOperationException>(400);

app.UseExceptionHandler(options);

app.MapGet("/", () =>
{
    throw new InvalidOperationException();
});

app.Run();

Alternative Designs

Alternative names

public void AddMap<TException>(int statusCode) where TException : Exception {} public void MapStatusCode<TException>(int statusCode) where TException : Exception {} public void MapToStatusCode<TException>(int statusCode) where TException : Exception {}

Using a property instead

namespace Microsoft.AspNetCore.Builder;

public class ExceptionHandlerOptions
{
+    public IDictionary<Type, int> StatusCodeMapping { get; } = new Dictionary<Type, int>();
}

The disadvantage of this design is the Dictionary potentially accepts any type, not necessarily an Exception type.

cc @khellang

[davidfowl]

I like the dictionary because it gives the ability to manipulate the data (CRUD). IMO Map isn't a clear name, I like MapStatusCode

[brunolins16]

I like the dictionary because it gives the ability to manipulate the data (CRUD). IMO Map isn't a clear name, I like MapStatusCode

I like it as well :) and using a dictionary might be common pattern in ASP.NET Core already, right. Do you feel we need to do anything regards to the key type (System.Type)? Maybe introduce some generic type (struct ExceptionMapKey<T> where T: Exception)

[khellang]

MapStatusCode sounds good and is what I'm already using.

However, it feels very limiting to restrict mapping from exceptions to status codes. Maybe this is just a first step? In my middleware, status code mapping is just a few higher level convenience methods that boil down to Func<HttpContext, Exception, ProblemDetails> registrations. These can also be paired with Func<HttpContext, Exception, bool> predicates to allow conditional mapping. It's an infinitely more flexible solution that can support a lot of additional scenarios.

What's the rationale behind this limitation? Just keeping it simple?

[brunolins16]

@khellang Thanks for the feedback. Do you have any usage telemetry of the convenience methods x underlying func lists?

The ExceptionHandlerMiddleware provides a mechanism for customization already, but this mechanism is more complex than it need to be when we want to do simple configurations, like Map to a status Code, Ignore an exception, etc. The rationale behind my proposal (that needs more discussion yet) is that we should provide convenient simple options that play well with the default handler. These scenarios you mentioned are great additions, but I feel like they are already cover by:

• ExceptionHandlerOptions.ExceptionHandler or;
• UseExceptionHandler(IApplicationBuilder, Action<IApplicationBuilder>) or;
• ProblemDetailsOptions.CustomizeProblemDetails (when you want customization for the generated PD) or;
• IProblemDetailsWriter custom implementations

[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.

[zyofeng]

Having the option to create typed ProblemDetails is quite useful in OpenApi where you can clearly define a possible list of "Problems" and additional information as per rfc7807 standard, without having to overwrite default ExceptionHandler behavior.

One existing example would be to generate Microsoft.AspNetCore.Http.HttpValidationProblemDetails and include an Errors dictionary when there is a FluentValidation exception.

So yes, having Func<HttpContext, Exception, ProblemDetails> would be ideal :)

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[BrennanConroy]

API Review notes:

• How would this interact with AllowStatusCode404Response?
  • We could ignore that check since we "know" the user set a 404 on purpose
• StatusCodeMapping makes me think you're mapping a status code to an Exception 😆
  • MapExceptionToStatusCode
  • ExceptionToStatusCodeMapping
  • MapStatusCode
• Why are we doing this if it's already possible in the ExceptionHandler request delegate?
  • i.e. is it too complicated to implement for the average user?
  • It's hard because the ProblemDetails writing is handled only if the ExceptionHandler isn't set
• How are AggregateExceptions handled?
  • Don't handle it specially, users either map AggregateException to a status code or get todays behavior.
• Func<Exception, int> would allow users to look at the exception and provide the status code they want, and not inadvertently write to the response if we passed in HttpContext, this helps with the ProblemDetails handling in the middleware
  • StatusCodeSelector

API Approved!

namespace Microsoft.AspNetCore.Builder;

public class ExceptionHandlerOptions
{
+    public Func<Exception, int>? StatusCodeSelector { get; set; }
}

[brunolins16]

If nobody is planning to this yet. Can I work on it?

[mitchdenny]

I don't anyone is working on it yet @brunolins16 - it's all yours :)

[brunolins16]

I don't anyone is working on it yet @brunolins16 - it's all yours :)

Thanks. I will have a PR later this week.

[arkchor]

What's the status on this? Is there a chance this will be released in the next few weeks?

[martincostello]

If the work to do this change hasn't already been merged into the release/8.0 branch (which I would assume not if this issue is still open with no updates since April), then it's too late for .NET 8.

[latonz]

@brunolins16 are you still working on this? Otherwise, I would be happy to provide a PR 😊