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

Merged
merged 7 commits into from
Jan 14, 2025

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.

@enisdenjo enisdenjo changed the base branch from master to pkgroll-bump-stuff January 13, 2025 14:01
Copy link
Contributor Author

@benjie benjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@enisdenjo enisdenjo merged commit 848715b into enisdenjo:pkgroll-bump-stuff Jan 14, 2025
5 checks passed
enisdenjo added a commit that referenced this pull request Jan 14, 2025
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.

2 participants