Replace 'query error' with 'request error' #803
Conversation
|
For posterity from live discussion: Agreement on this change - a further improvement would be to at some point describe what a "request error" is |
benjie
force-pushed
the
query-ambiguity-request-error
branch
from
6e652d3 to
3832d8d
Compare
|
I've added a definition for request errors and linked it from the first time it's mentioned (I'm not sure if it should be linked from each place?). I've also reviewed the content in section 7 and updated some of the uses of "operation" to "request" where they may apply before the operation can even be determined. |
spec/Section 7 -- Response.md
Outdated
| If the request encountered any errors, the response map must contain an | ||
| entry with key `errors`. The value of this entry is described in the "Errors" | ||
| section. If the operation completed without encountering any errors, this entry | ||
| must not be present. |
There was a problem hiding this comment.
Note the request might encounter an error before the operation can be determined (for example if operationName is not given but the document contains multiple operations, or of the document contains no operations), hence 'If the request encountered any errors.' However the final sentence is still correct - it does indeed relate to the operation completing without errors - for the operation to even begin there must be no errors before this point. Since it's valid both ways (and there's no ambiguity) I left this final sentence unedited.
leebyron
added
🏁 Accepted (RFC 3)
Saw "throw" / "raise" / "occur" / "encounter" - most dominant verb is "raise" so standardizing that.
leebyron
force-pushed
the
query-ambiguity-request-error
branch
from
efbbc0b to
b66f783
Compare
leebyron
force-pushed
the
query-ambiguity-request-error
branch
from
8956149 to
c63b6a5
Compare
|
This looks good to me 👍 |
| with names not defined by a field of this input object type, otherwise an error | ||
| must be thrown. | ||
| with names not defined by a field of this input object type, otherwise a | ||
| response error must be raised. |
There was a problem hiding this comment.
@benjie I noticed this term "response error" which is used only once in the spec and was introduced in this PR (which was the PR that introduced the term "request error"). Was this a mistake and should also say "request error"?
There was a problem hiding this comment.
Indeed, I suspect it should either be request error or field error. I'd have to read it carefully to figure out which - I'll add this to my list to review. Thanks for spotting!
There was a problem hiding this comment.
It should have been request error indeed. Thanks for spotting!
This is a specific change from #777 applied throughout the spec.
The term "query error" feels limited to GraphQL query operations; however the error could be raised from mutations or subscriptions too, in fact it can also be raised when it's not clear what operation is to be executed (e.g. when there's a document with multiple operations, but no
operationNamewas provided). As such, the error does not relate specifically to "query", or even "operation", but to the entire GraphQL request.Relates to graphql/graphql-wg#546