Split 'invalid request body or parsing failure' into three examples #200
Conversation
| Requests that fail to pass _GraphQL validation_ SHOULD be denied execution with | ||
| a status code of `400` (Bad Request). |
There was a problem hiding this comment.
This reads different from the outcome described in the other scenarios, but it really is the same? Perhaps we can describe the expected outcome once for all those failure modes - since it is the same - and only detail what the failure mode entails.
There was a problem hiding this comment.
Agreed. Is there a difference between should result in status code '400' (Bad Request). and SHOULD be denied execution with a status code of '400' (Bad Request)?
There was a problem hiding this comment.
In my opinion this is concretely different from the others - you can execute this one, you cannot execute the others (because they weren't parseable so you could not determine what to execute. And so the denial of execution is what you're doing (you can't deny execution to something that you couldn't have executed in the first place).
However, "result in" would also cover it, so if you both feel strongly that wording is better/clearer I'd be happy to make the change.
There was a problem hiding this comment.
I can see the difference, how about a note that explains it?
There was a problem hiding this comment.
I'm not sure what to put in the note.
There was a problem hiding this comment.
How about:
Note: In this case, it would be feasible for the server to attempt execution despite validation errors.
Although generally not recommended, exceptions might be necessary to support legacy clients.
| Requests that fail to pass _GraphQL validation_ SHOULD be denied execution with | ||
| a status code of `400` (Bad Request). |
There was a problem hiding this comment.
Agreed. Is there a difference between should result in status code '400' (Bad Request). and SHOULD be denied execution with a status code of '400' (Bad Request)?
Co-authored-by: Gabriel McAdams <[email protected]>
|
Suggest we merge this and @spawnia follow up with a note PR? |
I was in the process of typing up a suggestion for a note right before you asked this 🤯 |
Following @glasser's feedback here #175 (comment) I've split the "Invalid request body or parsing failure" example into three separate examples, and added spec-md references.
I believe this change does not include any normative changes, it's purely editorial.
Incidentally these H5's are horrible:
https://graphql.github.io/graphql-over-http/draft/#sec-Operation-cannot-be-determined
We should probably hoist to a higher level somewhere? (In a separate PR.)