Skip to content

subscriptions: Convert GraphQL errors to nil if they are empty #25

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

Merged
merged 1 commit into from
Jan 5, 2018

Conversation

Jannis
Copy link
Contributor

@Jannis Jannis commented Jan 5, 2018

This reduces the likelyhood of servers that use graphqlws to send
an empty array of errors along with subscription updates to clients.

This also makes subscriptions work better in GraphiQL, where
previously, the empty array of errors would make
GraphiQL-Subscriptions-Fetcher would treat all updates as errors:

https://github.com/apollographql/GraphiQL-Subscriptions-Fetcher/blob/master/src/fetcher.ts#L36

(An empty array is considered truthy).

This reduces the likelyhood of servers that use graphqlws to send
an empty array of errors along with subscription updates to clients.

This also makes subscriptions work better in GraphiQL, where
previously, the empty array of errors would make
GraphiQL-Subscriptions-Fetcher would treat all updates as errors:

https://github.com/apollographql/GraphiQL-Subscriptions-Fetcher/blob/master/src/fetcher.ts#L36

(An empty array is considered truthy).
@Jannis Jannis added the bug label Jan 5, 2018
@Jannis Jannis self-assigned this Jan 5, 2018
@Jannis Jannis requested review from yanivtal and Zerim January 5, 2018 13:23
@ghost ghost added the pending review label Jan 5, 2018
@Jannis
Copy link
Contributor Author

Jannis commented Jan 5, 2018

Tests are passing and the change was also tested with GraphiQL and the pending example PR (#24).

@Jannis Jannis merged commit 5c30d06 into master Jan 5, 2018
@ghost ghost removed the pending review label Jan 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant