-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Add a new helper that detects network errors from the link chain #12561
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
Conversation
🦋 Changeset detectedLatest commit: 6ba252b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
commit: |
src/errors/NetworkError.ts
Outdated
| import type { ErrorLike } from "@apollo/client"; | ||
| import { CombinedGraphQLErrors } from "@apollo/client"; | ||
|
|
||
| const registry = new WeakSet(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debating with myself if we can find a way around this global.
Even though it's a WeakSet, this might become a memory hog if WeakSet were ever to be polyfilled in a non-weak way for a runtime that didn't support it, and in a long-running process that would mean that all errors accumulate indefinitely.
My first thought would be to add this as a property on ApolloClient, but that way, the NetworkError.is api wouldn't work anymore, but it would have to be something like client.isNetworkError. Not the end of the world, but not very consistent either.
I'll experiment around for a bit.
|
Important note: this introduces a circularity that needs to be fixed: |
src/errors/NetworkError.ts
Outdated
| const registry = new WeakSet(); | ||
|
|
||
| /** @internal Please do not use directly. */ | ||
| export function registerNetworkError(error: ErrorLike) { | ||
| if (!CombinedGraphQLErrors.is(error)) { | ||
| registry.add(error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const registry = new WeakSet(); | |
| /** @internal Please do not use directly. */ | |
| export function registerNetworkError(error: ErrorLike) { | |
| if (!CombinedGraphQLErrors.is(error)) { | |
| registry.add(error); | |
| const registry = new WeakSet(); | |
| const networkErrorSymbol = Symbol.for("apollo.networkError"); | |
| /** @internal Please do not use directly. */ | |
| export function registerNetworkError(error: ErrorLike) { | |
| if (!CombinedGraphQLErrors.is(error)) { | |
| if (Object.isExtensible(error)) { | |
| Object.defineProperty(error, networkErrorSymbol, { | |
| value: true, | |
| enumerable: false, | |
| writable: false, | |
| configurable: true, | |
| }); | |
| } else { | |
| registry.add(error); | |
| } |
(together with)
is: (error: unknown) =>
(error && (error as any)[networkErrorSymbol]) ||
registry.has(error as ErrorLike),Something like this could be a memory-defensive version that only uses registry as a last resort and tries to "mark" errors first.
On the other hand, I might be way overhinking this, too. Environments where WeakSet needs to be polyfilled are probably not that common, and where I can imagine they exist (e.g. edge runtimes), they probably also are not very long-lived.
We really need a list of alternative JS runtimes and their features at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep this in our back pocket. If we end up in situations where the WeakSet becomes a problem, using that defensive fallback is a great solution.
NetworkError instances7027d12 to
3692d1f
Compare
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
3692d1f to
9140dba
Compare
size-limit report 📦
|

This would help with #12555
This PR adds the ability to detect if an error was emitted by the link chain. This can be useful if your application uses custom errors in other areas of the codebase that would otherwise get difficult to differentiate. Use
NetworkError.isto detect if an error is the link chain.Take this example:
Since Apollo Client 4.0 no longer wraps errors by default, it is near impossible to distinguish whether the error came from the link chain or the custom error in the function.
By introducing this new helper, the check becomes: