Skip to content

Inconsistent handling of 405 on version-neutral endpoints #159

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
shreeena opened this issue Jul 5, 2017 · 4 comments
Closed

Inconsistent handling of 405 on version-neutral endpoints #159

shreeena opened this issue Jul 5, 2017 · 4 comments
Assignees

Comments

@shreeena
Copy link

shreeena commented Jul 5, 2017

Using 1.1.0 on ASP.NET Core:

When requesting a version-neutral endpoint with an unsupported HTTP method, a 400 response is returned with the following body:

{
    "error": {
        "code": "ApiVersionUnspecified",
        "message": "An API version is required, but was not specified."
    }
}

On a versioned endpoint with an unsupported HTTP method, a 405 response is returned with the following body:

{
    "error": {
        "code": "UnsupportedApiVersion",
        "message": "The HTTP resource that matches the request URI '{requestUri}' does not support the API version '1.0'."
    }
}

In issue #65, it seems we have decided to use 405's over 400's. Could we keep version-neutral endpoints consistent with this? In addition, the response body is quite misleading.

As for the response body attached to the 405 -- could we make the message a little less misleading as well?

Thanks.

@commonsensesoftware
Copy link
Collaborator

Thanks for the detailed information. I have confirmed that this is, in fact, a bug. It's a rather unique edge case that requires a pretty specific combination; specifically, you have to make a request to an API version-neutral controller, without an API version, and request an unsupported HTTP method. If the client specifies an API version - any API version, you get the expected behavior. An interesting and good find.

I believe I have the necessary fix in place. I've added this scenario to the test cases. I should have the update out soon.

There isn't much conceptual difference between a 400 for an unsupported API version and a 405. The main argument for 405 is better parity with how Web API did things and to provide better client feedback. HTTP methods aren't directly part of routing (but some devs seem to think so). 405 says that the requested route exists, but the HTTP method you requested is not supported by the current API version. For a version-neutral API, that means that no routes support the requested HTTP method, since such a controller accepts all API versions.

I've tried to keep the number of expected error codes low to make things easy to understand. The primary use case of the error responses is for human beings. There isn't really anything defensive a client can do to handle these types of responses. For this particular edge case, I do believe that the message can be a little more succinct. The response for this case will now be:

Code: UnsupportedApiVersion
Message: The HTTP resource that matches the request URI '{requestUri}' is unsupported.

I'm not sure if that's more inline with what you were thinking. I'm open to suggestions.

Error responses have been somewhat of a debate. The default behavior is 400 in most cases as was recommended in a specification that pre-dates the Microsoft REST Guidelines. Most agree that this is a reasonable approach. However, some have specifically asked for 404 (Not Found), 405 (Method Not Allowed), and even 406 (Not Acceptable) when versioning by media type. I'm not sure there is a one size fits all, so I've afforded for extension points where that behavior can be changed. You can also argue that the default response should be 501 (Not Implemented).

@shreeena
Copy link
Author

shreeena commented Jul 6, 2017

Thanks for clarifying! I wasn't thinking of the 405 in terms of a specific API version, but that makes more sense now. As for the message, what I originally had in mind was the message I was used to seeing, something along the lines of
The requested resource does not support http method 'GET'.
but what you have suggested will work as well. My concern with the message is probably due to the way I think about versions as well. I view the version as a part of the resource, and the verb as the action taken against that resource. In that light, the current 405 message misleads me into thinking that the versioned resource doesn't support a particular version, as opposed to a particular verb against that resource. I feel like it would be less misleading if the verb was included as part of the message.

@commonsensesoftware
Copy link
Collaborator

I can see that viewpoint. I'm not sure if you're aware, but ASP.NET Core doesn't even officially support 405. It's been a long-standing issue tracked on #388; however, feature #6046 is hoping to address this.

That means for API versioning I had to invent my own method of 405 support. Since I already had to consider all candidates for API version, it wasn't particularly tough to support 405 correctly. Basically, when all the candidates are tallied up, if no API version matches and no candidate contains the requested HTTP method, then the response is 405 instead of 400. This might change when ASP.NET Core 2.0 is released.

I suppose the HTTP methods could be considered more obvious if they are in the message, but I make sure that I return the Allow header in all related 405 responses. This will tell you which HTTP methods are allowed as required by the RFC 2616 §14.7 (e.g. the HTTP spec). It may not be as obvious as within the message, but I believe that would satisfy your requirement, yes?

@commonsensesoftware
Copy link
Collaborator

FYI, package version 1.2.1 with the fix has been published. Thanks for the support and reporting the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants