Sibling errors should not be added after propagation #1184
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is built on top of:
GraphQL.js output is not (currently) stable after an operation terminates: more errors may be added to the result after the promise has resolved!
Reproduction with `graphql` module `test.mts`
(I've formatted this output for brevity)
The reason for this: though we note in the spec that you may cancel sibling execution positions, we don't do that in GraphQL.js; and furthermore, we even process errors from the result and add them to the errors list!
This is particularly problematic for client-side "throw on error". Given this schema:
And the same spec-valid result as above:
Technically the
Test.b
field is the field that causeddata.test
to be null - it's non-nullable, so it triggered error propagation - but without looking at the schema we can't determine this.Solution: recommend that servers don't keep adding to
errors
after error propagation has occurred. This would mean:errors
after the operation has "completed"