Allow to favor throwing Exceptions instead of swallowing and formatting them directly to Response

[gimlichael]

Is there an existing issue for this?

• I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

So when I was browsing my API using your excellent framework i noticed this "exception":

{
  "type": "https://docs.api-versioning.org/problems#invalid",
  "title": "Invalid API version",
  "status": 400,
  "detail": "The HTTP resource that matches the request URI 'someurl' does not support the API version 'b3'.",
  "Code": "InvalidApiVersion",
  "traceId": "00-fe622aed3e09484395e8cad918cb4df8-78da4931d596fa48-00"
}

Problem is, that this is never thrown as an exception .. you are overtaking the Response pipeline and providing a "friendly" error message.

I would really like to have full control of exception handling - and I strongly believe a framework such as yours, should support this, and not try to take control of the Response pipeline and hereby interfere with expected outcome.

The "exception" is triggered because both Edge and Chrome has an odd version in their default accept header.

Describe the solution you'd like

I hope you will consider allowing throwing exceptions instead of swallowing them and transform them on my behalf.
This should be done while staying true to your intent of not breaking backward compatibility.

In this case, I would expect (by looking at your code) a FormatException which I could choose to ignore or format as I see fit.

It is vital that APIs has full control over exception handling - or in this case, the lack of it.

Additional context

Areas of interest I have found in your code:

https://github.com/dotnet/aspnet-api-versioning/blob/main/src/AspNetCore/WebApi/src/Asp.Versioning.Http/Routing/MalformedApiVersionEndpoint.cs

https://github.com/dotnet/aspnet-api-versioning/blob/main/src/Abstractions/src/Asp.Versioning.Abstractions/ApiVersionParser.cs

[commonsensesoftware]

Apologies if it sounds completely pedantic, but HTTP cannot throw an exception. Problems Details are the established mechanism to provide additional information for an error response in accordance with RFC 7808. A scenario such as an invalid API version isn't exceptional, but it is a bad request (e.g. HTTP status code 400). This has been the behavior of API Versioning since day one.

Exceptions are not free and come at a cost. They should be reserved for things that are truly exceptional. Furthermore, if an exception is thrown and unhandled, it will result in HTTP status code 500 because - as I say - HTTP has no concept of exceptions.

Fear not! That doesn't mean you can't change or alter the behavior. Problem Details are can be defined a couple of different ways. Although MVC (Core) provides the ProblemDetailsFactory, it is not available for Minimal APIs. API Versioning, therefore, provides its own IProblemDetailsFactory that is a stand in. It is used for Minimal APIs and provides an adapter over the MVC-provided ProblemDetailsFactory implementation. Depending on your application, you can change one or both of these implementations.

Problem Details will not let you change the response, only the error details. There is no way to change the response behavior - by design; however, it is possible to change whether parsing of an API Version is valid (or not). I'm not sure that's what you're looking for, but it's possible. I think you just want more control over the error response provided, which can be achieved via custom ProblemDetails. Logging is also provided through ILogger with a category of ApiVersionMatcherPolicy.

[gimlichael]

I am aware that HTTP perse cannot throw exceptions, but I do believe your library should have the option to throw exceptions instead of interacting directly with the response pipeline as it is currently the case.

I am also aware that there is no such thing as a free lunch; but the performance penalty involved around exception handling is not something that concerns me, and as I proposed, it could be an option you provide - maybe for inspiration, it could be in a similar way that Microsoft did there implementation on HttpResponseMessage where they introduced EnsureSuccessStatusCode(): https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpresponsemessage.ensuresuccessstatuscode?view=net-6.0

Again, I like the flexibility of your framework - and I admit I am probably the rule to the exception (no pun intended), but I do believe firmly in all errors should be treated as exceptions, and not being wrapped in delivery objects.

That said, I will look further into both your code and the feedback you provided here, and see if I can find a way to bypass your direct interaction with the response pipeline.

Lastly, to give some insight on my profile, I have been working with REST APIs for a decade now and has delivered many API with HATEOAS maturity level 2; a few POC with maturity level 3; all exceptions thrown by an application will be translated into their respective response - and if in fact unhandled, of course result in a 500 (which is fine when it is in fact the app/server that has an issue).

Anyway, this was just to establish that I - to some extend - know what I am doing and what I am proposing :-)

[commonsensesoftware]

REST assured, I'm sure you know what you're doing. 😉

An invalid or unspecified API version on the server side isn't an unhandled error to the server; it's a client mistake. Client errors (e.g. 4xx) aren't anything exceptional on the server side. Effectively, the server is telling the client "You don't know how to call me" or "You're calling me wrong". This is no different than 404, 405, 406, or 415.

This particular case is in the gray area. 400 is being returned, which is consistent for just about every case, but this case could also be 415. This is very much like versioning by URL segment (which is so wrong and not RESTful 🤓), where 400 is also returned, but it could be 404. The idea of using 400 was to try and help clients when they make a mistake.

In terms of code, what you're looking for is ApiVersionPolicyJumpTable.GetDestination. The routing system uses a direct graph under the hood and this is the place to determine how to jump to the next node.

Speaking of exceptions, there used to be a single gap exposed via IApiVersioningFeature, which is where the raw and parsed API version values are held during a request. In previous implementations, there was only RawRequestedApiVersion, which meant in the case of ambiguous API versions (e.g. multiple), the only escape hatch was an exception - specifically AmbiguousApiVersionException. This still exists, but now that the plural RawRequestedApiVersions is in place, ambiguous versions can be handled a different way without exception. The only remaining scenario is via the parsed RequestedApiVersion in the same scenario. An exception is required because while you might expect the property to be null when parsing fails, you wouldn't expect it to also be null if there are multiple values. It's very rare for this to happen and the framework itself will never run into that situation. As a library consumer it's also rare that would happen to you since an ambiguous request would have been handled upstream, but there's no guarantee as to when you might access the value. It's possible you access the value at a time when there really are two or more, different, requested API versions.

If you have some more details about your scenario and setup, I might have some additional suggestions. It's important to note that it is intended that once you opt into versioning, there is no such thing as unversioned anymore. Even if you don't explicitly provide a version, the DefaultApiVersion will be the value used. There are very few exceptions to this. The IApiControllerFilter is a multi-instance service that allows filtering controllers. A filter is provided out-of-the-box for the [ApiController] attribute so that UI controllers and Razor Pages are excluded. Minimal APIs are also excluded by default because there is no way to get in front and differentiate them from any other Endpoint.

It may be possible to use some middleware or so other facility to achieve your desired result without specifically introducing exceptions.

[gimlichael]

Thank you for your thorough walkthrough of the code - as well as your viewpoint.

I have great respect for both you and the code you are writing, and in many ways, your code stands out for an inspiration to everybody.

That said, we both have some dogmatic viewpoint and practices; and although I agree strongly to your 400 BadRequest, I also believe that this is an exception that should be raised to the client. A common and consistent way to raise a problem, that in return can be translated into lets say RFC 7807 or "my" way of having a consistent way of reporting errors to the consuming client.

I would argue that client or server specific errors are still that; errors that needs to be communicated clearly to the consumer. Be that in the 400 or 500 range is irrelevant .. 400 range is just particular good in explaining why the server has chosen to respond the way it did, e.g., the more common 400 (invalid syntax), 401, 403 to the more sophisticated 409, 412, 428, 429 or even custom non-standard statuses.

Problem with your approach is, that a lot extra code now needs to be written to support a consistent way of serving error messages to the consuming part of an API; so they don't end up having different error experiences - one way from your framework - another way from my error handlers.

I am pro following standards - its just the implementation I question.

This is actually a good example the one we are having.

I could choose to implement a middleware component that translates all exceptions into lets say RFC 7807 - because I encourage and uses exceptions consistently. I could also choose to follow the path I have taken - and provide even more details for developers that surpasses the said mentioned standard .. or I could do adapt something third.

With exceptions this is easy; with different framework (yours being one of them), I could end up having to many chefs cooking there own error handling directly to the Response pipeline rendering a consistent way to communicate faults impossible.

Common denominator is proper exception handling within your ASP.NET application.

Anyway, we have different viewpoints and I respect that .. I will continue to embrace your work as I have done since 2019 ..

Keep up the good work - and thank you for taking time to respond.

For reference (and I am sure you know about it):

https://learn.microsoft.com/en-us/dotnet/standard/exceptions/best-practices-for-exceptions
https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/exceptions-and-performance
https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/exception-throwing

I also encourage to read through the Framework Design Guidelines - all editions .. they have shaped me into favoring consistency and usability .. in some areas maybe wrongfully .. I reckon that I might move in the area of control flow :-)

Feel free to close this proposal if you are not convinced that your framework should allow for opt-in to default exception handling.

[commonsensesoftware]

Healthy discussions are welcome here. There have been plenty of heated knives out debates. 😉 There is usually a reckoning, but sometimes we agree to disagree.

I've followed the published design guidelines in all its many forms over the past 20 years, including reviewing the many framework sources (even with decompilation before the OSS days). Per your cited guidelines, "Design classes so that exceptions can be avoided". Exceptions should be exceptional.

The client is the one calling the server. The server is not a client of this library. By opting into this library, or even ASP.NET Core for that matter, there are certain behaviors you sign up for. Many behaviors can be customized or extended, but not all. The goal is to have the 80% fall into The Pit of Success and give the 20% the tools to tweak knobs. A concerted effort is made so that people can't go over the rails. I might make it difficult for you to do some custom things (albeit sometimes intentionally), but it's pretty rare that I would provide no hook whatsoever. Those that really want to see what happens after the red pill, I let them.

A bit of history... Before 6.0, there was the IErrorResponseProvider. This was meant as a hook to control error responses. There was no standard mechanism to provide non-exceptional error responses in ASP.NET Core at the time. That all changed back in ASP.NET Core 2.1 when ProblemDetails were introduced as the official mechanism. The guiding principles for this library come from what is now the Microsoft REST Guidelines (it had several alternate, internal names and iterations previously). The guidelines do outline standardized error responses, but ASP.NET Core never adopted them. Though it is not cited, the error response format comes from OData errors (there are reasons for that). As there was no other standard, this was the default implementation of IErrorResponseProvider. ASP.NET Core 2.1 brought ProblemDetails from RFC 7807 as the standard, default mechanism for all server-generated errors. API Versioning had a compatible IErrorResponseProvider implementation that could align to that. Now, that ProblemDetails has been established as the de facto error response format, it didn't make sense to keep IErrorResponseProvider around. Instead, the behavior coalesced into ProblemDetails.

Implementing ProblemDetailsFactory is the service you implement or change to control the creation of ProblemDetails. If you haven't reviewed it already, you may want to review ASP.NET Core's guidelines for web API error responses:

https://learn.microsoft.com/en-us/aspnet/core/web-api/handle-errors

Oddly, ProblemDetailsFactory is an abstract class and is part of MVC Core, but ProblemDetails aren't. This is a design flaw IMHO. API Versioning provides a bridge with its own IProblemDetailsFactory interface so that ProblemDetails can be created for Minimal APIs as well as when using MVC Core. An adapter is automatically registered for ProblemDetailsFactory making it easy to replace, but you can still replace IProblemDetailsFactory if you want to. The goal was to only make you have to change one.

API Versioning's behavior with respect to client errors during route processing is congruent with what ASP.NET Core does. You will not get a EndpointNotFoundException or HttpResponseException (like legacy ASP.NET Web API) when an endpoint doesn't exist. The client simply gets 404. The API Versioning behavior is no different. The condition is neither exceptional nor can be thrown to a client (as HTTP doesn't support that). Changing the response can be done a number of ways, but the ProblemDetailsFactory is likely what you want. API Versioning has been careful to make sure you continue to have this hook (much like the ol' IErrorResponseProvider) when ASP.NET Core itself does not (see dotnet/aspnetcore#32957). That should be fixed in the forthcoming .NET 7.0 release.

It makes little difference to me how you want you want to present your errors. If you don't like ProblemDetails or have some other format you prefer, so be it. It's your API after all. My goal was simply to align to how ASP.NET Core presents all other client errors so you have a chance to control their output in a consistent way with the least amount of effort. One of the problems with using exceptions to control this behavior is that API authors may not be aware that such exceptions exist, can happen, when they happen, how/if they should be handled, nor what the response should be. As far as I can tell, the current approach is about as consistent with the provided mechanisms by ASP.NET Core as it can be, allows most developers to fall into The Pit of Success, and still affords customization (like you are asking for) without exceptions. I can appreciate it might require more customization than you'd like, but you do have the necessary extension points even if there is no perfect solution.

[gimlichael]

I truly enjoy your thorough comments; thank you for taking the time for that - even though I can imagine you have a busy schedule.

I agree to disagree - and I will continue to have my Matrix-love for your fine library; I must say that it was not without a challenge to tweak your code to my "vision".

As you mentioned, most of it was fairly simple - I managed to do that by making a custom implementation you can view here: https://github.com/gimlichael/Cuemon/blob/development/src/Cuemon.Extensions.Asp.Versioning/RestfulProblemDetailsFactory.cs

The challenging part was your endpoint serving 415; this does not use the IProblemDetailsFactory as you decided to opt-out of content.

Here I had to go to the creative corner and force my hand to make this extension (with the sole purpose of catching client side faults: https://github.com/gimlichael/Cuemon/blob/development/src/Cuemon.Extensions.Asp.Versioning/ApplicationBuilderExtensions.cs

So, as I mentioned earlier, consistency is key for me; so if an API adapted RFC 7807 all is good and dandy and they can go on with their lives. BUT, if for whatever reason, someone adapted a different path, they can now fairly easy adapt their own customization with those two proposals.

I learned a trick or two by examining your library in depth - so thank you for that - and thank you again for your perspective - it is much appreciated.

I will follow you closely and see if I can keep up with your continued maintenance of your library; keep up the good work.

[commonsensesoftware]

Nice! Looks like you were largely able to achieve your goals with minimal effort.

😲 You are correct on 415. As an army of one, there are very few reviewers and I do make mistakes from time-to-time. Discussions like these help find some of them. I was thinking it just passed through and let the default routing handle it, but it doesn't nor does it provide a behavior better than ASP.NET Core itself. 😞 This is an oversight and I'm willing to call that a 🐞. There's no reason to not provide ProblemDetails in the same way everything else does. It seems you have a workaround, but I will get this fixed. As I recall, I mostly just copied what ASP.NET Core previously did. Unsure how this happened because it seems pointless as written. I think I just went down a path and forgot to actually hook in the ProblemDetails.

I did notice from this PR, that ASP.NET Core is fixing their issue across the board. There are a bunch of changes. I suspect I will have to refactor to come into alignment, but the net effect will be identical or very, very close. You might want to peek at them since it would affect your strategy as well.

RFCs and spec aren't for everyone, but I've found them grounding and useful as a guide against my own dogma and bias. Deprecation has long been supported, but conveying policy has been missing. Stumbling across RFC 8594 provided a clear and sensible way to do so. This inspired the feature to convey a Sunset Policy (>= 6.0), which can be a supported (predestined to deprecate) or deprecated API. It also inspired useful client extensions to query policy information or capture telemetry in API clients. 😃 I still haven't finished the docs on all of that, but there plenty of examples in the test cases.

[commonsensesoftware]

A short musing... as I recall, I didn't add ProblemDetails for 415 because I couldn't (can't in .NET 6.0) add it for 404, 405, or 406. It's not really a good reason, but that was my rationale. I'm going to go ahead and add it because it should only enhance things. Basically, 415 will behave just like 400, but with a different status code. Since they are fixing things in .NET 7, I should be able to bring all responses into parity 🤞🏽.

Since you are knowledgeable in this area, should it only be 415? For example, if the version is coming from Content-Type, then it should be 415. However, if the version comes from Accept, it should arguably be 406. Everything else would be same. This would also be inline with the behavior of 405.

Today, everything is 415 because that's how it is in ASP.NET Core. The routing system doesn't know to yield 406 until a response body is provided, but there is no corresponding, negotiated IOutputFormatter. In the case of API Versioning, it is known early in the request pipeline for Accept whether the associated version can be served. That should yield 406 IMHO. Thoughts?

[commonsensesoftware]

To further clarify... after updating some unit tests I realized how I got to plain old 415.

Scenarios:

• Unsupported version: 415, but now with ProblemDetails
• Supported version with unsupported media type: 415 with no body (ASP.NET Core has all the control)

I was originally thinking it was good to have parity, but if more information is available, why not add it?

[gimlichael]

Reading about it, I think I would opt-in for 406 (GET) and keep 415 (PUT, POST).
That said, I still have sleep in my eyes, but I was happy for the reach out - and for the update.
Thanks.

As a follow up; maybe provide an option for users to decide themself?

I like the strict version (eg. 406 and 415), but other might like a more relaxed "policy", hence always return 415.
Or, as Microsoft has done in some of there areas, add a "KeepBackwardCompatiblityToXXX".

Just some seeds for you to grow; as always - keep up the good work!

[commonsensesoftware]

The status shouldn't be based on method because you could have something like:

POST /session HTTP/2
Host: api.server.com
Accept: application/json; v=2.0

This should return 406 as opposed to 415. 415 is closely tied to Content-Type whereas 406 is closely tied to Accept.

There are some ways people can change this behavior. It's way down in the routing system so it's not particularly easy to change. Furthermore, this has little consequence on the server side. There is some variability on the client side, but it's not really a recoverable error; the client is calling the server wrong. A client will almost certainly handle any 4xx. Both responses return the same ProblemDetails, which is the thing the client should be checking for and using to make additional decisions or provide an error log/message.

Over-configuration is always a concern. I'm hesitant to add configuration for something that no one will probably ever change. Once it's added, it's hard or impossible to remove. There are escape hatches for those that are really motivated to change it. These are status codes inline with the HTTP spec, so I think that is good enough - for now.

I have something working. Expect it in the next release.

As a parallel, we don't have configuration options for returning 410 vs 404.