grapher-react icon indicating copy to clipboard operation
grapher-react copied to clipboard

Prevent situations where a logout can trigger unsubscribe of ready() data

Open theodorDiaconu opened this issue 7 years ago • 2 comments

Flow:

  • User logged out
  • Subscription changed -> returned error
  • Component Autorun re-rrun (because .fetch() was reactive) and .ready() and .fetch() were run in the same autorun context
  • Left hanging subscription run due to memory leak

This was the fix, but I'm not sure exactly why, I leave it here to merge it later.

  const subscriptionError = new ReactiveVar();

  return withTracker((props) => {
    const query = handler(props);

    const subscriptionHandle = query.subscribe({
      onStop(err) {
        if (err) {
          subscriptionError.set(err);
        }
      },
      onReady() {
        subscriptionError.set(null);
      },
    });

    const isReady = subscriptionHandle.ready();

    return {
      isReady,
      query,
      config,
      props,
      subscriptionError,
    };
  })(dataTracker(errorTracker(QueryComponent)));
}```

```const dataTracker = withTracker(({ query, config, props, isReady, subscriptionError }) => ({
  grapher: {
    data: query.fetch(),
    isLoading: !isReady,
    error: subscriptionError,
  },
  query,
  config,
  props,
}));```

theodorDiaconu avatar Jun 19 '18 11:06 theodorDiaconu

Just tried to clone this package and make these changes but it did not work.

Here's my setup:

  • Layout -> query for current logged in user
    • Subcomponent -> query for data that has a firewall checking for a logged in user

When the user logs out, the subcomponent query is rerun, and the firewall throws an error, crashign the app on the client.

Floriferous avatar Jul 17 '18 15:07 Floriferous

@Floriferous it's normal for the subcomponent to re-run.

You logout -> Your subscriptions get updated. If you did not unmount the component, it may take some time before React unmounts it thus triggering unsubscribe. I would advise you to do something like history.push('/login'); Meteor.logout(), so you already unmount anything unnecessary.

theodorDiaconu avatar Jul 18 '18 03:07 theodorDiaconu