useSubscription hook fails to receive expected data if reusing Concast observer from unmounting useSubscription hooks under certain conditions

[Hsifnus]

Issue Description

In React, if the current ApolloClient has query deduplication enabled, and one component with an ongoing useSubscription hook that has yet to complete unmounts at the same time as another component using a useSubscription hook with exactly the same document and variables as the first component's hook, then this recently mounted component's subscription will be unable to receive incoming data from the subscription endpoint.

After digging through with inspector breakpoints what happens in the Apollo client source code during this interaction, I found that this sequence of events consistently occurs:

• Initially, the old useSubscription hook mounts and sets up its Concast observable via ApolloClient.subscribe. This Concast will always start out with exactly one source that is immediately consumed to establish a source subscription within the Concast.
• Later, the new useSubscription hook mounts and sees the old subscription still using its Concast for same document and variables as the new subscription and therefore (under query deduplication) reuses that Concast within the ApolloClient.subscribe call used to set the observable state in the new useSubscription hook.
• Old hook unmounts and unsubscribes its Subscription obtained earlier from subscribing to its Concast. Assuming that the old subscription was the only subscription linked to the Concast at the time, the Concast now completes and performs all of the associated cleanup logic... except for unsubscribing from its source subscription, which is deferred via setTimeout.
• New hook subscribes to the reused Concast observable from two steps back.
  • As an aside - since completes do not populate this.latest within a Concast, this.latest can remain null within the reused Concast if the Concast had not received any source subscription values before the old hook unmount and in that case prevents the new subscription from immediately completing via Concast.deliverLastMessage.
• The deferred unsubscription of the Concast source observable triggered by the old hook unmounting now finalizes the Concast's cleanup, indefinitely cutting off the new subscription hook from receiving any data through the reused Concast.

Interestingly, the error handler of the current Concast implementation has this code comment:

// Delay unsubscribing from the underlying subscription slightly,
// so that immediately subscribing another observer can keep the
// subscription active.

Under the current error and complete handler logic, when immediately subscribing another observer to the Concast after removing the last existing observer from the Concast, the source subscription of the Concast will always be unsubscribed once setTimeout kicks in, which is contrary to what the above comment describes.

So far, I have made a branch of my personal fork in an attempt to address this issue in the spirit of the above code comment by preserving the Concast subscription if a new observer subscribes to the Concast right as the last existing observer unsubscribes and if the subscription is still open to begin with. It seems to fix the useSubscription issue mentioned here and appears to work well with existing tests in the repo, but whether it is the best approach to this general issue is a matter to be discussed here.

Link to Reproduction

https://codesandbox.io/s/test-apollo-graphql-subscriptions-forked-sxst71?file=/src/index.js

Reproduction Steps

Normally, the subscription data should appear on the screen after a period of loading, regardless of whether "Component A" or "Component B" is shown.

The public subscription endpoint only publishes one value before completing, so reproducing the bug requires the user to click the "Click here to toggle" text before the current subscription hook receives that value. If done right, the displayed component's useSubscription hook will be indefinitely stuck in the loading state until the components are toggled again.

[lovasoa]

I think this is a duplicate of the issue I reported yesterday: #10693

I reported my issue directly using the lower level APIs, but looking at the description of the issue here, it sounds like it's just an instance of the react wrapper triggering #10693.

[Hsifnus]

Good point - I'll close this issue and link to it in that issue then.

[alessbell]

Hi @Hsifnus 👋 Your PR was just released in 3.7.13. I was doing some clean-up and just found and forked your reproduction link - unfortunately, it looks like I'm still able to reproduce that particular bug (stuck on loading: true) with the latest version including your fix. Would you be able to confirm whether this has indeed solved your issue? Thanks!

Edit: I'm seeing the same (old) behavior with the 0.0.0-pr-10718-20230426133633 snapshot release as well, if I'm understanding your reproduction case correctly.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.