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 in error handling to the GraphQL dataprovider. #6500

Closed
wants to merge 2 commits into from

Conversation

sudeepjd
Copy link

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.

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

Notes for reviewers

  • My very first PR so pardon any errors. Please let me know if anything needs to be changed.
  • I have also 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.
  • As part of making this change, I noticed that there wasn't a data-provider-graphql example as it did not exist and would be useful for others as an example for GraphQL, so I have added it in.
  • Thank you for allowing me to make this fix, I have noticed that the docs will also need to be updated. Will submit another PR for the documention update.

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

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

feat(data-provider-graphql): added data-provider-graphql example for testing out the graphql data provider

BREAKING CHANGE: use [Code] prefix to denote Code errors thrown.
Added in [Code], [Network], [GraphQL] error prefixes to differentiate errors
test(graphql): added in tests to validate graphql changes made

feat(data-provider-graphql): added data-provider-graphql example for testing out the graphql data provider

BREAKING CHANGE: use [Code] prefix to denote Code errors thrown.
@sudeepjd sudeepjd requested a review from a team as a code owner November 18, 2024 15:13
Copy link

changeset-bot bot commented Nov 18, 2024

🦋 Changeset detected

Latest commit: 0dc7c76

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 Patch

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

Looks like there is a build issue with respect to the pnpm-lock.yaml file on the new example data-provider-graphql that I have added in. But in the repo there is no pnpm-lock.yaml file. I have the following 2 options

  1. If someone can help me how to solve this issue, it would be good.
  2. I can separate out the @refinedev/graphql changes as well as the new example/data-provider-graphql into a separate PRs so atleast the fix to the Error issue that this resolves will go through and the new example/data-provider-graphql example can be figured out separately.

Please let me know how to proceed on this.

Thanks.

@sudeepjd
Copy link
Author

Closing this pull request.. Will open individual ones for the update of @refinedev/graphql and the data-provider-graqhql example separately, so that already the bug fix on the errors goes through.

@sudeepjd sudeepjd closed this Nov 19, 2024
@BatuhanW
Copy link
Member

Looks like there is a build issue with respect to the pnpm-lock.yaml file on the new example data-provider-graphql that I have added in. But in the repo there is no pnpm-lock.yaml file. I have the following 2 options

  1. If someone can help me how to solve this issue, it would be good.
  2. I can separate out the @refinedev/graphql changes as well as the new example/data-provider-graphql into a separate PRs so atleast the fix to the Error issue that this resolves will go through and the new example/data-provider-graphql example can be figured out separately.

Please let me know how to proceed on this.

Thanks.

@sudeepjd your example is actually correct. You just need to run pnpm install in the root directory. Since this is a monorepo, we only have lockfile in the root. Once you run it, it will add your example and their dependencies to the pnpm-lock.yaml in the root folder.

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