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

Makes error handling more consistent #9941

Closed

Conversation

MrDoomBringer
Copy link
Contributor

@MrDoomBringer MrDoomBringer commented Jul 25, 2022

See this Issue for more information: #9870

This PR aims to make the networkError and graphQLErrors fields on the result object behave as expected for all react hooks - on any 4/500 error code networkError will be populated, and if there's any GraphQL error from the server, it will be present in that field.

Behavior currently looks good with useQuery, useSubscription, and ApolloLink.
I tried to make the implementation as non-intrusive as possible, both for backwards compatability and to hopefully prevent regressions by touching too much code.

TODO: Add test coverage

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@hwillson
Copy link
Member

Super cool @MrDoomBringer (which is an appropriate GH username for working on error PR's 😂). Let us know when you think this is ready for review (holding off for now since it's still in draft). Thanks!

@MrDoomBringer MrDoomBringer changed the title [Draft] Makes error handling more consistent Makes error handling more consistent Jul 29, 2022
@MrDoomBringer MrDoomBringer marked this pull request as ready for review July 29, 2022 08:47
@hwillson hwillson requested review from hwillson and benjamn August 12, 2022 15:47
Mutations basic run

ApolloLink

Revert "Mutations basic run"

This reverts commit b3a130d.

Test support

Rename error_network -> errorNetwork

onError link support

bump bundlesize from 29.95 -> 29.98KB

Consolidate errorNetwork away
@MrDoomBringer MrDoomBringer marked this pull request as draft August 22, 2022 16:59
@alessbell
Copy link
Contributor

@MrDoomBringer feel free to ping me when the conflicts are resolved and I'll take a look 👍

@MrDoomBringer
Copy link
Contributor Author

Hey @alessbell! I think this PR needs a bit more work, so I'll be putting it on the backburner for now. Definitely some good preliminary work in here through that should be useful when it's time to revisit it.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"GraphQLErrors" field will often not populate, errors instead stuck in "NetworkError"
5 participants