Open runar-rkmedia opened 1 year ago
daniilkerna
commented
1 year ago TypeError: Cannot read properties of null (reading 'hasOwnProperty')
This issue happens to me frequently when refresh the page and send the request. When I debug onError triggered, networkError was null.
Can someone help me on this please?
jerelmiller
commented
1 year ago Hey @runar-rkmedia and @daniilkerna 👋
Would either of you be able to provide a reproduction of the issue? While its certainly possible isApolloError might need some modification, its not clear to me the conditions in which onError would be called but both networkError and graphqlErrors are both null. This seems concerning to me since at least one of these properties should be set at all times.
A reproduction would go a long way to helping us figure out why the error gets in this state to begin with. You can use our error template as a starting point. Any help would be appreciated. Thanks!
peteygao
commented
1 year ago I don't have the time to shrink the issue down to a minimal reproducible example, but I can also confirm this is occurring in our application:
graphql: 15.8.0 and tried both @apollo/client: 3.7.3 and latest as of writing @apollo/client: 3.7.9 to see if this issue was fixed before I stumbled upon this thread.
Side note: This error has consistently been thrown in our application, it's only until today when I tried to upgrade to Node 18.14.2 that this issue actually became a blocker by crashing the application. Previously on Node 16.19.0 (which is being EOL'd later this year), the error would be thrown in the console but the application still "worked" (for a definition of "worked": that is to say, the UI renders and our customers can still use the app; I wasn't happy about this weird errant error constantly being thrown in our app, but I thought perhaps this was a known issue that y'all were working on fixing so didn't give it too much thought at the time).
For the time being we've rolled back the Node version to unblock ourselves. Not sure if this solution will work for anyone else.
Good luck with the bug hunting 🫡
AnielloFalcone
commented
8 months ago I also faced the issue, I don't actually know whose fault it is but here is what I've found:
const authLink = new ApolloLink((operation, forward) => {
return new Observable<FetchResult>((observer) => {
auth.refresh()
.then(() => {
const token = auth.token;
operation.setContext(({headers = {}}) => ({
headers: {
...headers,
authorization: token ? `Bearer ${token}` : '',
'Content-Type': 'application/json'
}
}));
// Call the next link in the chain
const subscription = forward(operation).subscribe({
next: observer.next.bind(observer),
error: observer.error.bind(observer),
complete: observer.complete.bind(observer)
});
return () => {
if (subscription) subscription.unsubscribe();
};
})
.catch((error) => {
observer.error(error);
console.error(error);
});
});
});For some reason the promise get rejected even if there is no failed request in the network tab, but the error returned in the catch here is undefined.
This leads ApolloClient to throw the TypeError: Cannot read properties of undefined (reading 'hasOwnProperty') because the error the observer receives is undefined.
I added a check to only send the error to the observer if there is an actual error and the Apollo error is gone.
const authLink = new ApolloLink((operation, forward) => {
return new Observable<FetchResult>((observer) => {
auth.refresh()
.then(() => {
const token = auth.token;
operation.setContext(({headers = {}}) => ({
headers: {
...headers,
authorization: token ? `Bearer ${token}` : '',
'Content-Type': 'application/json'
}
}));
// Call the next link in the chain
const subscription = forward(operation).subscribe({
next: observer.next.bind(observer),
error: observer.error.bind(observer),
complete: observer.complete.bind(observer)
});
return () => {
if (subscription) subscription.unsubscribe();
};
})
.catch((error) => {
if (error) {
observer.error(error);
console.error(error);
}
});
});
});Could it be an issue in the ApolloLink at this point?
jerelmiller
commented
8 months ago @AnielloFalcone thanks so much for that code snippet. I have a general idea what might be happening here, assuming this is the case for all the reports above.
isApolloError assumes the error passed into it is an actual error value, so if it receives null or undefined, it would blow up:
This is called by our QueryManager in a chunk of code that is run whenever observer.error is called from the link chain:
Seeing that the conditional check on the error value seems to address the issue, this makes me think that the null or undefined is sneaking through to that check in the library. I'd argue that this is a mistake and observer.error should always receive an error value, otherwise its easy to miss actual issues in your system. In this case, that auth.refresh() promise rejection is seen as a success by Apollo, even though the promise rejection likely indicates the session wasn't actually refreshed.
I'd challenge you to investigate why that promise created by auth.refresh() doesn't reject with a more helpful error first of all for the reasons stated above. I know this doesn't address the library end of things, but it would provide you with a more helpful way of tracking the issue.
We could consider adding some defensive code against this, but this might lead to some weird situations where a user passing null to observer.error would report that value as such through to the end query then blame it on Apollo thinking its a bug in the library. I think that observer.error should always be called with a valid error. Perhaps a solution/compromise might be to add some defensive code in that isApolloError check with a warning on null/undefined values to ensure that valid values are passed through on the user's end.
@runar-rkmedia @daniilkerna @peteygao would any of you be able to confirm that the value passed to observer.error in your situations is null or undefined when you see this issue arise?
AnielloFalcone
commented
8 months ago Hi @jerelmiller thanks for your reply.
I will try to understand why that happens, to me it seems a weird combination of events.
We have the same auth.refresh() call in other parts of the app and I will try to see if also there the catch is triggered but without any error.
Let you know asap.
This looks alot like #6414
Intended outcome:
Running a graphql-query with apollo-client with an authorization-header.
Actual outcome:
When loading/refreshing the page, sometimes the QueryManager seems to crash in
isApolloErrordue to theerr-variable beingundefined.It only happens sometimes, which makes it hard to debug here.
How to reproduce the issue:
Versions
Error with stacktrace:
module.js:62 Uncaught TypeError: Cannot read properties of undefined (reading 'hasOwnProperty') at isApolloError (index.ts:10:14) at QueryManager.ts:1107:13 at both (asyncMap.ts:30:30) at asyncMap.ts:19:47 at new Promise ()
at Object.then (asyncMap.ts:19:16)
at Object.error (asyncMap.ts:31:39)
at notifySubscription (module.js:137:18)
at onNotify (module.js:176:3)
at SubscriptionObserver2.error (module.js:229:5)
isApolloError @ index.ts:10
(anonymous) @ QueryManager.ts:1107
both @ asyncMap.ts:30
(anonymous) @ asyncMap.ts:19
then @ asyncMap.ts:19
(anonymous) @ asyncMap.ts:31
notifySubscription @ module.js:137
onNotify @ module.js:176
error @ module.js:229
(anonymous) @ iteration.ts:13
iterateObserversSafely @ iteration.ts:13
error @ Concast.ts:183
notifySubscription @ module.js:137
onNotify @ module.js:176
error @ module.js:229
error @ index.ts:75
notifySubscription @ module.js:137
onNotify @ module.js:176
error @ module.js:229
setTimeout (async)
hostReportError @ module.js:61
notifySubscription @ module.js:146
onNotify @ module.js:176
error @ module.js:229
(anonymous) @ iteration.ts:13
iterateObserversSafely @ iteration.ts:13
ObservableQuery2.reportError @ ObservableQuery.ts:907
error @ ObservableQuery.ts:844
(anonymous) @ iteration.ts:13
iterateObserversSafely @ iteration.ts:13
error @ Concast.ts:183
notifySubscription @ module.js:137
onNotify @ module.js:176
error @ module.js:229
(anonymous) @ asyncMap.ts:44
Promise.catch (async)
(anonymous) @ asyncMap.ts:43
notifySubscription @ module.js:137
onNotify @ module.js:176
error @ module.js:229
(anonymous) @ iteration.ts:13
iterateObserversSafely @ iteration.ts:13
error @ Concast.ts:183
notifySubscription @ module.js:137
onNotify @ module.js:176
error @ module.js:229
error @ index.ts:75
notifySubscription @ module.js:137
onNotify @ module.js:176
error @ module.js:229
Promise.catch (async)
(anonymous) @ index.ts:28
Subscription2 @ module.js:190
subscribe @ module.js:264
(anonymous) @ index.ts:35
Subscription2 @ module.js:190
subscribe @ module.js:264
complete @ Concast.ts:211
Concast2.start @ Concast.ts:104
Concast2 @ Concast.ts:84
QueryManager2.getObservableFromLink @ QueryManager.ts:990
QueryManager2.getResultsFromLink @ QueryManager.ts:1061
resultsFromLink @ QueryManager.ts:1439
QueryManager2.fetchQueryByPolicy @ QueryManager.ts:1471
fromVariables @ QueryManager.ts:1166
QueryManager2.fetchQueryObservable @ QueryManager.ts:1207
ObservableQuery2.fetch @ ObservableQuery.ts:699
ObservableQuery2.reobserve @ ObservableQuery.ts:838
(anonymous) @ ObservableQuery.ts:140
Subscription2 @ module.js:190
subscribe @ module.js:264
(anonymous) @ useQuery.ts:199
(anonymous) @ useSyncExternalStore.ts:98
invokePassiveEffectCreate @ react-dom.development.js:23487
callCallback2 @ react-dom.development.js:3945
invokeGuardedCallbackDev @ react-dom.development.js:3994
invokeGuardedCallback @ react-dom.development.js:4056
flushPassiveEffectsImpl @ react-dom.development.js:23574
unstable_runWithPriority @ scheduler.development.js:468
runWithPriority$1 @ react-dom.development.js:11276
flushPassiveEffects @ react-dom.development.js:23447
(anonymous) @ react-dom.development.js:23324
workLoop @ scheduler.development.js:417
flushWork @ scheduler.development.js:390
performWorkUntilDeadline @ scheduler.development.js:157
react_devtools_backend.js:4012 ApolloError {statusCode: undefined, response: undefined, reqId: undefined, durationMs: undefined, requestStart: undefined, …}
overrideMethod @ react_devtools_backend.js:4012
(anonymous) @ Apollo.tsx:179
error @ index.ts:57
notifySubscription @ module.js:137
onNotify @ module.js:176
error @ module.js:229
Promise.catch (async)
(anonymous) @ index.ts:28
Subscription2 @ module.js:190
subscribe @ module.js:264
(anonymous) @ index.ts:35
Subscription2 @ module.js:190
subscribe @ module.js:264
complete @ Concast.ts:211
Concast2.start @ Concast.ts:104
Concast2 @ Concast.ts:84
QueryManager2.getObservableFromLink @ QueryManager.ts:990
QueryManager2.getResultsFromLink @ QueryManager.ts:1061
resultsFromLink @ QueryManager.ts:1439
QueryManager2.fetchQueryByPolicy @ QueryManager.ts:1471
fromVariables @ QueryManager.ts:1166
QueryManager2.fetchQueryObservable @ QueryManager.ts:1207
ObservableQuery2.fetch @ ObservableQuery.ts:699
ObservableQuery2.reobserve @ ObservableQuery.ts:838
(anonymous) @ ObservableQuery.ts:140
Subscription2 @ module.js:190
subscribe @ module.js:264
(anonymous) @ useQuery.ts:199
(anonymous) @ useSyncExternalStore.ts:98
invokePassiveEffectCreate @ react-dom.development.js:23487
callCallback2 @ react-dom.development.js:3945
invokeGuardedCallbackDev @ react-dom.development.js:3994
invokeGuardedCallback @ react-dom.development.js:4056
flushPassiveEffectsImpl @ react-dom.development.js:23574
unstable_runWithPriority @ scheduler.development.js:468
runWithPriority$1 @ react-dom.development.js:11276
flushPassiveEffects @ react-dom.development.js:23447
performSyncWorkOnRoot @ react-dom.development.js:22269
(anonymous) @ react-dom.development.js:11327
unstable_runWithPriority @ scheduler.development.js:468
runWithPriority$1 @ react-dom.development.js:11276
flushSyncCallbackQueueImpl @ react-dom.development.js:11322
workLoop @ scheduler.development.js:417
flushWork @ scheduler.development.js:390
performWorkUntilDeadline @ scheduler.development.js:157
```