-
Notifications
You must be signed in to change notification settings - Fork 712
UnsupportedApiVersion error when giving some different content type #744
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
Comments
Are you able to show at least the skeleton of the rest of your controller and perhaps the configuration setup? Which route/action is giving you trouble? You seem to indicate that the one you've shown works. If no media types match, I would have expected HTTP 406 (Not Acceptable). A response will contain the same error, but may be HTTP 400 or 405 depending on whether the HTTP method is supported by any version. Repros always the best, but with a little more information I can probably figure out what's happening here. |
This is the structure of the controller . One of the headers "Content-Type" which is set in this attribute on the controller [Consumes(CommonConstants.ApplicationMergePatchContentType)] and is made to accept value application/merge-patch+json ...but when we give some other value in content type header .. it gives error api version not supported (400 bad request).. let me know if you need any other information? We want the api versioning library to not throw unsupported error so that we can error out properly. |
This provides a little more about the action, but it doesn't provide any information about the controller or application setup. I still have a number of questions.
I'm not 100% sure what that means. API Versioning has very few errors it throws. It will, however, return client errors - without exception - for invalid requests. There are ways to change the error response; for example, a different status code, message, or payload. It's unclear if that's what you mean by |
Okay .. @commonsensesoftware To simplify.. Let me give you the exact scenario ..I tried the same on one of your examples .. for the sample used for api-versioning (https://github.com/microsoft/aspnet-api-versioning/tree/master/samples/aspnetcore).. I appended Consumes attribute over one of the post APis in OrdersController
Now some of the answers to your questions: 1.There is versioning only on string and not on any mediaType/ContentType. Outputs:
2. Issue : But on giving content type "application/json" and same version "1.0" It should not have errored out by api-versioning library as the version is correct and it should others contents types. I hope this examples gives you more context about controller and application-setup. |
This makes a lot more sense. Out of curiosity, does this work without versioning? I would not expect it to. PATCH api/Orders?api-version=1.0 HTTP/2
Host: localhost:5000
Content-Type: application/json
Content-Length: 142
{
"id": 42,
"createdDate": "2021-03-05T00:00:00+00:00",
"customer": "kkk"
} I agree, the server should respond with HTTP 415 (Media Type Unsupported). Without versioning does it? This is an interesting edge case. I don't think I've seen it before, but I suspect I know why it's happening. Propagating 406 and 415 is hard for ASP.NET Core (and presumably all web frameworks). This decision happens not at the controller level, but at during serialization/deserialization. I as recall 406 is harder because first an action has to be selected, executed, and the decision is made trying to resolve a corresponding OutputFormatter. If the resolution fails, the client receives 406. You should think 415 would be selected early during the request, but it seems it isn't matched until the InputFormatter is selected during model binding. So what does that have to do with versioning? The versioning endpoint matcher policy invalidates any valid candidate by API version. This should be the last step and constraint. Unfortunately, it seems that media type does not invalidate a candidate; at least, not before API versioning sees it. That's why you don't receive 406 or 415. API versioning returns 400 when no candidate for a given route doesn't match. It returns 405 if there is some candidate, in at least one version, that could match. This is often the case if a HTTP method is added or removed between versions. I'm not entirely sure why/where the wires are crossed. There is a similar when versioning by media type, which can be addressed by changing the IErrorResponseProvider.
|
@commonsensesoftware I verified its working without versioning. It gives proper error 415 UnSupportedMediaType. You can verify it once on some sample controllers. Yes, We want "application/merge-patch+json" according to some guidelines. I'll reconfirm on Patch + application/json thing. But We want to support "application/merge-patch+json" and we might need any other content type also for that matter for any scenario. We shall try to fix this to get unblocked early. |
Thanks for confirming. I'll see what I can find. |
@commonsensesoftware any updates? |
My opinionated dogma aside 😉 , I'm not able to repro your scenario. I've tried against the latest I tried following all the points from the discussion, but clearly our streams are crossed. You first mentioned putting I saw things work as expected. If you want to support multiple media types, then you have to add them all: /// <summary>
/// Places a new order.
/// </summary>
/// <param name="order">The order to place.</param>
/// <returns>The created order.</returns>
/// <response code="201">The order was successfully placed.</response>
/// <response code="400">The order is invalid.</response>
[HttpPost]
[MapToApiVersion( "1.0" )]
[Produces( "application/json" )]
[Consumes( "application/json", "application/merge-patch+json" )] // ← list all supported media types
[ProducesResponseType( typeof( Order ), 201 )]
[ProducesResponseType( 400 )]
public IActionResult Post( [FromBody] Order order )
{
order.Id = 42;
return CreatedAtAction( nameof( Get ), new { id = order.Id }, order );
} With this in place, the Swagger UI will show both media types and they both succeed with
At this point, it's not clear to me how our two setups are different. If you're able to take a sample project, repro the issue, and drop a copy of it here, that will go a long way to making sure I'm on the same page as you. |
Hiii, @commonsensesoftware I tried again and can still see the issue, let me help you repro the issue in OrdersController of https://github.com/microsoft/aspnet-api-versioning/tree/master/samples/aspnetcore
Expected behaviour: It errors out saying media type is not correct(Unsupported media Type) Actual Behaviour: It errored out with 405 Method not allowed saying api version is not correct(I didnot even change the api-version)
You should be able to repro the issue with this. Let me know if you have any queries. |
I'll reiterate that /// <summary>
/// Updates an existing order.
/// </summary>
/// <param name="order">The order to update.</param>
/// <returns>The created order.</returns>
/// <response code="204">The order was successfully updated.</response>
/// <response code="400">The order is invalid.</response>
/// <response code="404">The order does not exist.</response>
[MapToApiVersion( "1.0" )]
[HttpPatch( "{id:int}" )]
[Produces( "application/json" )]
[Consumes( "application/merge-patch+json" )]
[ProducesResponseType( 204 )]
[ProducesResponseType( 400 )]
[ProducesResponseType( 404 )]
public IActionResult Patch( int id, [FromBody] Order order ) => NoContent(); I then tested it with the HTTP REPL instead of the Swagger UI. These are from the default endpoint Defaulting to
|
@commonsensesoftware So I have created 3 different PRs to see how API versioning lib is behaving according to changes. Setup-1
PR: BlackRider97#1 It is working correctly as expected. Case-1 Correct content type Case-2 Incorrect content type Setup-2
PR: BlackRider97#2 It is NOT WORKING AT ALL as expected. Case-1 Correct content type Case-2 Incorrect content type Setup-3
PR: BlackRider97#3 It is NOT WORKING PARTIALLY as expected. Case-1 Correct content type Case-2 Incorrect content type Now @PranjaliSoni is talking about setup-3. For her it is not working as expected when wrong content type is passed. |
@BlackRider97, @PranjaliSoni thanks for the updated information; this is useful. This was a very deep investigation. Lucky you, you're the winner of two bugs! The first bug is in API Versioning and will happen when:
The reason you get 405 instead of 400 is because there is other matching candidates. It is possible that methods are added or removed between versions, which is the reason for 405 (e.g. It should be possible to properly report 415 instead of 405, but unfortunately I'm not currently able to fix it due to a bug I've discovered (and reported) in dotnet/aspnetcore#33865. Something in the routing system is filtering out possible candidates. The lack of all the candidate information is preventing me from properly detecting 415 across API versions (which is this special case). The possible, temporary workaround is:
This is a yucky, lame solution, but I'm blocked up stream. I suspect it may take weeks or even months for the ASP.NET team to fix this issue. We'll have to see how much of a priority they make it. You might be able to flex your muscle on their repo. The linked issue illustrates how to repro the issue, even without API Versioning. |
I just wanted to circle back around and provide a quick update on this issue. After a lot of researching, I can confirm that this is indeed a bug and a misunderstanding (on my part) about what extensions to the routing system are necessary. I've made the necessary changes, which you can now find in the new main branch. Expect the fix to be available in the next major release ( On a positive note, The code should be ready if you want to play around with it. I'm actively working getting things setup to publish new packages. |
The Preview 2 release available and you can pick up the changes from the new package Asp.Versioning.Mvc. While I could make the change in I've lifted this use case into the test suite. I believe it will now do what you expect in a more consistent manner for The latest examples can be found here. |
|
We are using 4.1.1 version of APiVersioning and 3.1 version of Asp.NetCore
I have a controller with an action which accepts only Content-Type "application/merge-patch+json"
So while quering for the given api with content type "application/merge-patch+json".. It works fine
but quering for some other valid content type for example "application/json" gives 400 bad request and
following output:
and on using APiVersioning version as "5.0.0"
I got the same response with different status code.
Can you please help me with this?
The text was updated successfully, but these errors were encountered: