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.

Sign up for GitHub

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

Jump to bottom

graphQLErrors property passed to onError is sometimes an empty string #10590

Open
6XGate opened this issue Feb 21, 2023 · 7 comments · May be fixed by #11329
Open

graphQLErrors property passed to onError is sometimes an empty string #10590

6XGate opened this issue Feb 21, 2023 · 7 comments · May be fixed by #11329
Labels
🐞 bug 🙏 help-wanted

Comments

@6XGate
Copy link

6XGate commented Feb 21, 2023 •
edited

Issue Description

The type definitions for the onError link shows that graphQLErrors should always be an array of errors. Unfortunately when some servers return a 500 error, it will be an empty string.

Link to Reproduction

See repro steps

Currently I have to use the following code to prevent this from happening

onError(({ networkError }) => {
  if (networkError != null && 'result' in networkError && typeof networkError.result !== 'object') {
    networkError.result = { }
  }
})

Reproduction Steps

It is difficult to create a reproduction case with the given templates. Though I can if needed. But, the offending code is in at /src/link/error/index.ts:63 and may be a lack response parsing sanity checking or sanitation earlier on. It may also be that using a chaining of && should be replaced with something that ensures the networkError result is an object and not something else.
@jerelmiller jerelmiller added 🐞 bug 🏓 awaiting-team-response requires input from the apollo team labels Feb 21, 2023
@hichemBAALI
Copy link

hichemBAALI commented Mar 8, 2023 •
edited

Any update here. I am still having the graphQLErrors: undefined when authLink fails (token expires). Please let me know if anyone faced the same issue

@6XGate
Copy link
Author

6XGate commented Mar 9, 2023 •
edited

Any update here. I am still having the graphQLErrors: undefined when authLink fails (token expires). Please let me know if anyone faced the same issue

That would likely come in as networkError if the error is indicate via an HTTP status code like 401 or 403. AFAIK, getting those will results in an undefined graphQLErrors value. But, I filled this issue due to sometimes getting an empty string in those cases rather than undefined.

@jerelmiller
Copy link
Member

@6XGate this definitely seems like a bug, good catch! We should absolutely make sure that graphqlErrors never get set as an empty string in the event the server response is empty.

Would you be open to submitting a PR to get this fixed? I'd be more than happy to review it.

@6XGate
Copy link
Author

6XGate commented Mar 15, 2023

Can if I get a chance.

@bignimbus bignimbus removed the 🏓 awaiting-team-response requires input from the apollo team label Mar 16, 2023
@euc-callum
Copy link

We're also seeing this for 502 errors 👍

@fe-jcorreia
Copy link

This issue was fixed for new versions?

@jerelmiller
Copy link
Member

@fe-jcorreia nope this is still an issue. I'm just realizing though that we have #11329 open to address this. We'll see if we can get a review on that one soon. Thanks for the nudge.

@jerelmiller jerelmiller linked a pull request Mar 28, 2024 that will close this issue
fix: graphQLErrors in Error Link if networkError.result is an empty string #11329
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🐞 bug 🙏 help-wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: graphQLErrors in Error Link if networkError.result is an empty string PaLy/apollo-client
6 participants
@jerelmiller @bignimbus @6XGate @hichemBAALI @fe-jcorreia @euc-callum