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(core): Handle text/* type fetch responses #2456

Merged
merged 5 commits into from
May 24, 2022
Merged

Conversation

kitten
Copy link
Member

@kitten kitten commented May 24, 2022

Resolve #2442
Reopen of #2444

Summary

This adds special handling for text/* responses. Optimally, a GraphQL endpoint should never under normal circumstances return a text response.
Also, adding a separate case for cases where JSON.parse fails leads to two things:

  • We'd be assuming that 200 OK responses can return non-JSON (which shouldn't be true)
  • We'd be assuming that we always want the text response as an error when the result is not JSON
    Instead, we can add a special case for text/* to explicitly return the result then and otherwise keep our old logic.
    Furthermore, in case response.statusText isn't present, we can add another fallback case of passing through the
    original error.

Set of changes

  • Add text/* handling to fetchSource

JoviDeCroock and others added 4 commits May 24, 2022 16:55
The outer `catch` was still responsible for handling errors,
so we can avoid duplication here and still use `statusText`.
We should instead add a separate case for `text/*` contents.
Those should never really be returned by endpoints, but if they
are we can return them as it was in this branch before.
@kitten kitten requested a review from JoviDeCroock May 24, 2022 15:58
@changeset-bot
Copy link

changeset-bot bot commented May 24, 2022

🦋 Changeset detected

Latest commit: 11d472b

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

This PR includes changesets to release 1 package
Name Type
@urql/core 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

@kitten kitten merged commit 2695fb9 into main May 24, 2022
@kitten kitten deleted the handle-text-responses branch May 24, 2022 16:44
@urql-ci urql-ci mentioned this pull request May 24, 2022
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.

Body parsing error for non-JSON 401 response
2 participants