one failed subscription cancels all existing ongoing subscriptions

[tseric]

Bug report

client cancels an existing subscription, server confirmed it was stopped and apollo-ios library removed that id from its record, but due to (possible?) race condition, there is still some subscription update coming through (for that already cancelled subscription), this causes an error because the id of this subscription update no longer exists. Currently the apollo-ios will just kill all the exiting subscriptions

Versions

Please fill in the versions you're currently using:

• apollo-ios SDK version: 0.53.0
• Xcode version: Xcode 13.3
• Swift version: Swift 5
• Package manager: SPM

Steps to reproduce

1. have two subscriptions going on the same screen of an ios app, let's call this subscription A and subscription B
2. both A and B are functioning well receiving updates from the web socket
3. Let's cancel subscription A
4. You will see, subscription A canceled, but update for subscription A still comes in (just one or two updates through) (not always 100%, reproducible but will happen)
5. breakpoint shows because the incoming update in bullet 4 above, it could not find the subscription id in its record, so it throws an error, which results on cancelling all subscriptions, i.e. it got rid of subscription B

Further details

1. This is intermittent, i.e. sometimes you would cancel successfully, and no updates comes afterwards, but sometimes last few updates do come through
2. this is not a desired solution, subscription B should not be cancelled
3. ** most important question**
   should the correct behavior be
   a. there should be absolutely no updates from Subscription A after its been cancelled, and therefore we have a bug
   b. there can be some updates from subscription A after its been cancelled, but should not cancel all the existing subscriptions (also a bug?)

more details flow:

1. both subscriptions A and B are running
2. A is cancelled and will execute this line to remove id from the entry
   https://github.com/apollographql/apollo-ios/blob/0.53.0/Sources/ApolloWebSocket/WebSocketTransport.swift#L319-L320

see 1 and 2 in a diagram below

3. based on the diagram above, when the subsequent update for subscription A (id 21) comes after its cancelled, it will go to this error block, because it could not find this id (id was removed in bullet no2)
   https://github.com/apollographql/apollo-ios/blob/0.53.0/Sources/ApolloWebSocket/WebSocketTransport.swift#L150
   notice it will fail in the second if condition, because there is no entry

4. so if it will come to the else block, and cancel all existing subscriptions i.e. including B that we want it to continue to work and stream

[calvincestari]

I think this issue may be closely related to, or a duplicate of, #2274.

[tseric]

thanks @calvincestari for your reply. yes i did also see that issue before posting, it sounded very similar but I decided to debug it more and post this just in case it is unrelated.

For now, as a workaround, at least for my scenario, I have added another list to track those that were removed from the subscription list, and i just check if the update that comes back has that matching id in the "removedList" and if it does it will just "continue" instead of cancelling all other subscriptions.

So for now, I will have this and will wait for official fix .

[aloukissas]

To note, the graphql-ws spec indeed mentions that this race is possible and that the client should ignore these:

Note: The asynchronous nature of the full-duplex connection means that a client can send a Complete message to the server even when messages are in-flight to the client, or when the server has itself completed the operation (via a Error or Complete message). Both client and server must therefore be prepared to receive (and ignore) messages for operations that they consider already completed.

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.