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

fix(server): onError should return GraphQLFormattedError #599

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

benjie
Copy link
Contributor

@benjie benjie commented Nov 26, 2024

The types sent over the wire should be formatted types - specifically errors should be piped through formatError and the types shouldn't imply that they're instanceof GraphQLError - they're just data.

Warning

This is a breaking change if anyone's onError callback is relying on instanceof GraphQLError checks (seems extremely unlikely?), but otherwise it should be safe because GraphQLError conforms to GraphQLFormattedError.

Tip

I don't actually care about formatError being called in core (feel free to revert that), I just care that the type of the onError hook allows a formatted error be returned for consistency, and that the payload type should not imply the errors are instanceof GraphQLError.

Note

The payloads that send ExecutionResult should also be sending FormattedExecutionResult but that seemed like a larger change so I didn't make it.

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

Successfully merging this pull request may close these issues.

1 participant