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

Error handling for unmounted component #2258

Closed
jer-sen opened this issue Oct 5, 2017 · 14 comments
Closed

Error handling for unmounted component #2258

jer-sen opened this issue Oct 5, 2017 · 14 comments

Comments

@jer-sen
Copy link
Contributor

jer-sen commented Oct 5, 2017

When a component is unmounted, error observer is removed from the query (in graphql wrapper component function componentWillUnmount). Thus, if an "in flight" request arrives with an error, there is no way to catch it and avoid the console.error red box message "Unhandled error ...".

At least the error message should explain what's really happening (unmounting could keep an error observer subscribed to adapt the error message to this case).
The error message could also be removed or only be a console.warn.
An option could be added to the graphql function to specify an error observer for errors after unmounting the wrapped component.

@jbaxleyiii
Copy link
Contributor

@Jay1337 interesting! Do you have a reproduction for this? I think it should be fixed in the 2.0 though since the new interface is an observable which will correctly clean up errors

@jer-sen
Copy link
Contributor Author

jer-sen commented Oct 11, 2017

@jbaxleyiii I'm only working with 2.0 beta and it's not fixed...

Sorry I don't have (time to make) a simple reproduction (it would require server and client code). However, it could be an app with a toggle button that display a React component when you press it. The React component is wrapped with a wrong graphql query. The server wait 10s before sending the response. Thus, if you press the button and then immediately "unpress" the button (so that the component is unmounted), after 10s you will get a console.error.

@stale
Copy link

stale bot commented Nov 1, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@jer-sen
Copy link
Contributor Author

jer-sen commented Nov 1, 2017

Hey !

@stale stale bot removed the no-recent-activity label Nov 1, 2017
@leethree
Copy link

I'm also seeing unhandled ApolloError when component is unmounted. I'm using apollo-client v2.0.2 and react-apollo v2.0.1. Here's a stack trace for the error:

ExceptionsManager.js:73 Unhandled error Network error: Network request failed Error: Network error: Network request failed

reactConsoleErrorHandler @ ExceptionsManager.js:73
console.(anonymous function) @ console.js:32
console.error @ YellowBox.js:71
ObservableQuery.onSubscribe.observer._subscription._observer.error @ apollo.umd.js:380
error @ zen-observable.js:174
ObservableQuery.onSubscribe @ apollo.umd.js:387
(anonymous) @ apollo.umd.js:128
Subscription @ zen-observable.js:103
subscribe @ zen-observable.js:229
ObservableQueryRecycler.recycle @ react-apollo.browser.umd.js:54
GraphQL.componentWillUnmount @ react-apollo.browser.umd.js:327
callComponentWillUnmountWithTimerInDev @ ReactNativeFiber-dev.js:2000
_invokeGuardedCallback @ ReactNativeFiber-dev.js:73
invokeGuardedCallback @ ReactNativeFiber-dev.js:47
safelyCallComponentWillUnmount @ ReactNativeFiber-dev.js:2003
commitUnmount @ ReactNativeFiber-dev.js:2127
commitNestedUnmounts @ ReactNativeFiber-dev.js:2071
unmountHostComponents @ ReactNativeFiber-dev.js:2100
commitDeletion @ ReactNativeFiber-dev.js:2119
commitAllHostEffects @ ReactNativeFiber-dev.js:2447
_invokeGuardedCallback @ ReactNativeFiber-dev.js:73
invokeGuardedCallback @ ReactNativeFiber-dev.js:47
commitAllWork @ ReactNativeFiber-dev.js:2478
workLoop @ ReactNativeFiber-dev.js:2555
_invokeGuardedCallback @ ReactNativeFiber-dev.js:73
invokeGuardedCallback @ ReactNativeFiber-dev.js:47
performWork @ ReactNativeFiber-dev.js:2593
scheduleUpdateImpl @ ReactNativeFiber-dev.js:2728
scheduleUpdate @ ReactNativeFiber-dev.js:2711
enqueueSetState @ ReactNativeFiber-dev.js:1514
ReactComponent.setState @ react.development.js:218
_onTransitionEnd @ Transitioner.js:234
proxiedMethod @ createPrototypeProxy.js:44
cb @ AnimatedImplementation.js:353
(anonymous) @ AnimatedValue.js:267
__debouncedOnEnd @ Animation.js:59
__invokeCallback @ MessageQueue.js:347
(anonymous) @ MessageQueue.js:137
__guard @ MessageQueue.js:265
invokeCallbackAndReturnFlushedQueue @ MessageQueue.js:136
(anonymous) @ debuggerWorker.js:72
17:23:39.303 

In particular, ObservableQueryRecycler.recycle calling subscribe looks suspicious to me.

@GuillemotF
Copy link

I made a reproduction with the error template
It reproduces a component throwing an unhandled network error when unmounting and not updating after an error (related to #2513)

@monotv
Copy link

monotv commented Nov 22, 2017

An unhandled network error when unmounting a component that failed to fetch this in particular is killing Apollo for me. The docs suggest logging the user out on a 401 error. Logging out often means unmounting scenes. This results in an unhandled error. So the docs are contradicted by how the client behaves.

@n1ru4l
Copy link
Contributor

n1ru4l commented Feb 22, 2018

This issue is still present in apollo-client 2.2.5 combined with react-apollo 2.1.0-beta.2.

Steps to reproduce:

  1. Mount component (fetch-policy: ignore)
  2. Disconnect from internet
  3. Refetch query
  4. Wait until fetch failes and data.error contains the network error
  5. Unmount component

Related lines of code:

console.error('Unhandled error', error.message, error.stack);

Commenting out this line does prevent the error. However if you navigate back to the unmounted scene the error property on data does not include the latest error.

CC @jbaxleyiii

@thomhos
Copy link

thomhos commented Feb 24, 2018

I'm experiencing this issue as well.

  1. Turn off graphql server to simulate a network outage
  2. Refresh page which runs a query
  3. Error is visible and rendered
  4. Navigate away, component throws and client crashes
  5. Come back to page, and no errors are present although network is still out

I'm not sure where to look for a solution / workaround for this. Wrapping with a HoC or using apollo-link-error doesn't seem to stop the throwing.

@n1ru4l
Copy link
Contributor

n1ru4l commented Feb 24, 2018

I will try to create a failing test case for this behaviour in the react-apollo repository, altough I think this might be an issue with apollo-client I cannot understand how I would reproduce this error by only using apollo-client. The current workaround solution that I am using is described in #1622 (use apollo-link-retry), which ironically might also be the best solution for my use case. Nevertheless this behaviour is a very critical issue for applications that could face flaky internet connections.

@Gregoirevda
Copy link
Contributor

Same problem here. Any failing request will throw an Unhandled exception in RN after you switch a route (unmount a component)

@n1ru4l
Copy link
Contributor

n1ru4l commented Feb 26, 2018

This should be fixed in 2.1.0-beta.3

@thomhos
Copy link

thomhos commented Feb 26, 2018

Correct! Query recycler is gone it seems :)

@hwillson
Copy link
Member

As mentioned in #2258 (comment), this issue should now be resolved. Let us know if anyone is still encountering this problem. Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants