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

Don't crash if some (but not all) Error Codes are loaded #11291

Merged
merged 4 commits into from
Oct 16, 2023

Conversation

ArioA
Copy link
Contributor

@ArioA ArioA commented Oct 16, 2023

Problem

I call loadErrorMessages() but not loadDevErrorMessages() on application startup. When a "dev" error code is encountered, I get the following error:

| TypeError: Cannot read properties of undefined (reading 'message')
|     at handler (.../node_modules/@apollo/client/dev/dev.cjs:613:41)
|     at getErrorMsg (.../node_modules/@apollo/client/utilities/globals/globals.cjs:85:44)
|     at Function.warn (.../node_modules/@apollo/client/utilities/globals/globals.cjs:48:16)
|     at Object.useSubscription (.../node_modules/@apollo/client/react/hooks/hooks.cjs:550:63)
|     ...

Proposed Solution

It seems reasonable for unloaded error codes revert to the fallback, rather than causing the app to crash.

@apollo-cla
Copy link

@ArioA: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@netlify
Copy link

netlify bot commented Oct 16, 2023

👷 Deploy request for apollo-client-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 2792bda

@changeset-bot
Copy link

changeset-bot bot commented Oct 16, 2023

🦋 Changeset detected

Latest commit: 2792bda

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

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

Copy link
Member

@phryneas phryneas left a comment

Choose a reason for hiding this comment

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

That's a very good catch, thank you for the PR!

Could you accept the CLA? I'll see that I fix it up for merging :)

@ArioA
Copy link
Contributor Author

ArioA commented Oct 16, 2023

@phryneas thanks! 😃 I've accepted the CLA
image

@phryneas
Copy link
Member

And I've added a Changeset :)

@phryneas
Copy link
Member

Those last two failing PRs are not your fault - getting this merged :)

@phryneas phryneas merged commit 2be7eaf into apollographql:main Oct 16, 2023
5 of 6 checks passed
@github-actions github-actions bot mentioned this pull request Oct 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 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.

3 participants