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

Added Error Handler that will throw GraphQL errors #6506

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

Conversation

sudeepjd
Copy link

@sudeepjd sudeepjd commented Nov 19, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

Bugs / Features

What is the current behavior?

Refine's GraphQL package is loosely based on nestjs-query. However, uses urql under the hood. urql uses 2 different types of error handling mechanisms, an errorExchange in exchanges or response.error on the result. The way refine is built using Tanstack query it is expecting an exception new Error to be thrown in order to be caught and treated as an error. But urql does not do this out of the box, and so refine always treats any error as a success, which impact the notifications provider.

What is the new behavior?

Once the error is received by client from the query or mutation, it passes the response.error to an error handler function that parses the error and throws it as Tanstack Query is expecting so that it is treated as an error and not as a success.

Also added in a new data-provider-graphql example to use the graphql data provider.

fixes (#6493): [BUG] @refinedev/graphql Error Handling does not work as it should

Notes for reviewers

  • I have added in an error differentiator prefix. [Code], [Network], [GraphQL] so the error can be classified and handled accordingly, this may be a breaking change for anyone on the [Code] who has tightly coupled with the "Operation is required." is required string.
  • Tests have been updated to validate the changes made.
  • Documentation has been updated to add in sections for Handling errors based on the prefixes, managing retries and the Authentication has been updated as it was using the nestjs-query mechanism for a custom fetch instead of using the fetchOptions in urql.
  • Thank you for allowing me to make this fix,

Added in [Code], [Network], [GraphQL] error prefixes to differentiate errors

test(graphql): added in tests to validate graphql changes made

BREAKING CHANGE: use [Code] prefix to denote Code errors thrown.
Copy link

changeset-bot bot commented Nov 19, 2024

🦋 Changeset detected

Latest commit: cc9fafd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@refinedev/graphql Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@sudeepjd
Copy link
Author

sudeepjd commented Nov 19, 2024

Dear Refine Team,

Please check this draft PR and if ok, will submit it through. It only has the required changes for the graphql package, tests and the corresponding documentation updates.

Added in Error Handling, Retry using Errors, and updated the Authentication documentation to use urql
@BatuhanW
Copy link
Member

Hey @sudeepjd thanks for the PR! I'll make some time to review it tomorrow.

@BatuhanW BatuhanW self-assigned this Nov 20, 2024
@BatuhanW BatuhanW linked an issue Nov 20, 2024 that may be closed by this pull request
Copy link
Member

@BatuhanW BatuhanW left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! LGTM. You can also add the example you previously added, only thing you need to do is to run pnpm install in the root folder, then it will add your example and its dependencies to pnpm-lock.yaml.

@sudeepjd
Copy link
Author

sudeepjd commented Nov 21, 2024

Thanks for the PR! LGTM. You can also add the example you previously added, only thing you need to do is to run pnpm install in the root folder, then it will add your example and its dependencies to pnpm-lock.yaml.

I have now added back in the example/data-provider-graphql and have also updated the documentation on the GraphQL package to point to the new example.

However, there are a few edits to the pnpm-lock.yaml file apart from what would normally be added in for the data-provider-graphql example. I am not sure if those are intended to be changed or not.

@sudeepjd sudeepjd marked this pull request as ready for review November 21, 2024 04:06
@sudeepjd sudeepjd requested a review from a team as a code owner November 21, 2024 04:06
@BatuhanW
Copy link
Member

Thanks for the PR! LGTM. You can also add the example you previously added, only thing you need to do is to run pnpm install in the root folder, then it will add your example and its dependencies to pnpm-lock.yaml.

I have now added back in the example/data-provider-graphql and have also updated the documentation on the GraphQL package to point to the new example.

However, there are a few edits to the pnpm-lock.yaml file apart from what would normally be added in for the data-provider-graphql example. I am not sure if those are intended to be changed or not.

Those edits are not problem, although annoying. No worries.

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.

[BUG] @refinedev/graphql Error Handling does not work as it should
2 participants