Skip to content
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

Wrong status code for 'invalid request payload' errors? #464

Open
joepie91 opened this issue Apr 28, 2019 · 0 comments
Open

Wrong status code for 'invalid request payload' errors? #464

joepie91 opened this issue Apr 28, 2019 · 0 comments
Labels
A-Client-Server Issues affecting the CS API clarification An area where the expected behaviour is understood, but the spec could do with being more explicit

Comments

@joepie91
Copy link

The specification suggests in various places to use the 400 status code for invalid request payloads, eg. the M_UNKNOWN error for a bad login type on the login route, or the M_MISSING_PARAM error on the /pushers/set route.

The more appropriate status code here would probably be 422 instead:

The 422 (Unprocessable Entity) status code means the server understands the content type of the request entity (hence a 415(Unsupported Media Type) status code is inappropriate), and the syntax of the request entity is correct (thus a 400 (Bad Request) status code is inappropriate) but was unable to process the contained instructions. For example, this error condition may occur if an XML request body contains well-formed (i.e., syntactically correct), but semantically erroneous, XML instructions.

(source)

This part of the specification claims that...

Errors are generally best expressed by their error code rather than the HTTP status code returned. When encountering the error code M_UNKNOWN, clients should prefer the HTTP status code as a more reliable reference for what the issue was.

For example, if the client receives an error code of M_NOT_FOUND but the request gave a 400 Bad Request status code, the client should treat the error as if the resource was not found. However, if the client were to receive an error code of M_UNKNOWN with a 400 Bad Request, the client should assume that the request being made was invalid.

... which means that in this case, some of the currently-400 errors could theoretically be safely represented as 422s (namely, those with an error code other than M_UNKNOWN), but all of the M_UNKNOWN errors - like that in the login route - could not be.

However, with the current ambiguity in the specification on whether the HTTP status codes are normative or non-normative, there's a good chance that some clients are relying on these sort of errors having a 400 status code either way, M_UNKNOWN or not. Which means that this seems like it would need a specification change to resolve, unless there's some sort of existing guidance on this that I'm not aware of.

@turt2live turt2live added clarification An area where the expected behaviour is understood, but the spec could do with being more explicit A-Client-Server Issues affecting the CS API labels Apr 29, 2019
@richvdh richvdh transferred this issue from matrix-org/matrix-spec-proposals Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Client-Server Issues affecting the CS API clarification An area where the expected behaviour is understood, but the spec could do with being more explicit
Projects
None yet
Development

No branches or pull requests

2 participants