fix: removed device bug#1887
Conversation
…core is closed and we don't run into unfixable errors.
| // Navigate away before leaveProject closes the hypercores on | ||
| // the backend in order to unmount subscribed components | ||
| // and run cleanup, avoiding SESSION_CLOSED errors | ||
| navigation.popToTop(); |
There was a problem hiding this comment.
this comment is not fully correct. We navigate here from the ProjectRemovalListener, which resets the navigation so the only components mounted are this and the home screen.
Your comment is still right that previously we were leaving the project and then attempting to navigate away, which was causing the problem. This component uses useOwnRoleInProject, and useOwnRoleinSettings. Those both take the project id from the nav params (which is the old project). So we leave the project, but this component tries to use the hook using the nav param, hence why the errors were being called.
I think the better fix would to actually be to NOT pass the projectID via nav param and use the project ID from the useActiveProject(). That way we are not ever using a stale id.
But more importantly its potentially buggy to unmount a screen and then run the action (leave project). It just causes a race condition where the screen could unmount before the leave project mutation is called
There was a problem hiding this comment.
Ok sounds good. Much better idea! c8448b4
…ve project and not a stale one.
closes #1875
The fix is to navigate away before calling leaveProject.mutate() to make sure that React unmounts any subscribed components and runs cleanup effects before the backend tears down the project's hypercores.
The problem was with navigating AFTER we leave the project, leaveProject could close the cores while components were still mounted, causing SESSION_CLOSED errors to bubble up to the error boundary and lock the user in a crash loop.
Without the navigation , in that component, the React components are still mounted for that project. And the stuff that is still mounted like ClientApiProvider, LocalDiscoveryProvider are making ipc calls on the closed cores and that is what is causing the errors. You can see it in the session breadcrumbs in sentry. And the component stack in the error shows ClientApiProvider at the root, with ExchangeScreenContent and ProjectRemovalListener in separate traces — multiple still-mounted components all hitting the same closed cores and causing crashing errors.
But if you navigate away first, then the components are all unmounted and won't make those calls.
The error is caused because once you leave a project the backend closes the cores right away. No grace period
Here is the error in Sentry: https://awana-digital.sentry.io/issues/7434615855/?project=4507170965618688&query=SESSION_CLOSED&referrer=issue-stream
I was only able to find the error once in Sentry, but it definitely comes from the ProjectRemovalListener, so I am 90% sure it is the right error.
I was not able to reproduce this unfortunately, but this fix should definitely make it so this error does not happen again.
It happens rarely because most of the time,
setActiveProjectId(defaultProject.projectId)propagates to rest of the app, however, there is a window of time every once in a while (in the case of the logged error and the error showing on Sentry) because of the asynchronous calling of useSingleProject (between setActiveProjectId firing and useSingleProject resolving the new project API) where components re-render with the new projectId but still hold a reference to the old projectApi. Unmounting the components with navigation is slightly more protective. So this fix will catch it every time.Another fix could happen in the comapeo/core or ipc.
leaveProjectcallskClearData() which closes hypercores. If there are any in-flight IPC sessions still open, they throw SESSION_CLOSED. So we could catch SESSION_CLOSED errors in the IPC layer rather than propagating them as unhandled exceptions. Or perhaps there are other ways to fix it but it could be also helped by a backend fix too, I think.