fix: resubscribe on reconnect

[aphex]

Initial POC of how reconnecting could work, likely has some bad side-effects though

Closes #2776

🎯 Changes

First we don't queue a subscription.stop if we are not connected to the socket. If it is queued this line here https://github.com/trpc/trpc/blob/next/packages/client/src/links/wsLink.ts#L124 will keep the request to subscribe from being sent as they both share the same id

Second we prevent the error callback if we have not closed "normally". This will prevent this method from being called here https://github.com/trpc/trpc/blob/next/packages/client/src/links/wsLink.ts#L319 which keep things from being unsubscribed while the socket tried to reconnect.

What changes are made in this PR? Is it a feature or a bug fix?

✅ Checklist

• I have followed the steps listed in the Contributing guide.
• If necessary, I have added documentation related to the changes made.
• I have added or updated the tests related to the changes made.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
next-prisma-starter	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Nov 26, 2022 at 8:57PM (UTC)
www	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Nov 26, 2022 at 8:57PM (UTC)

[aphex]

fixed up linting and trying to sneak in a minor TS change here https://github.com/trpc/trpc/pull/2783/files#diff-d5eb300ee573b9c1ced9ca84f2cb9be7746b78404935ac11c96c3bfa7d42a205R47

I would like to have access to this type in my plugin, so can we kindly export it :)

[juliusmarminge]

fixed up linting and trying to sneak in a minor TS change here https://github.com/trpc/trpc/pull/2783/files#diff-d5eb300ee573b9c1ced9ca84f2cb9be7746b78404935ac11c96c3bfa7d42a205R47

I would like to have access to this type in my plugin, so can we kindly export it :)

I think we are trying to keep these sort of types (I.e types needed for plugins and shouldn't necessarily be used in apps) exported from /shared (but maybe /integrations) would be a better name 🤔

[aphex]

@juliusmarminge While you're considering that I wonder if you all would be up for bringing these types over from my composable project into tRPC proper? They let you flatten a router into a dot notation string and then get procedure info back from the flatten version like namespace1.namespace2.procedure.

This lets other implementations reference procedures my a single string but also keep type safety and auto-complete.

Maybe the naming is slightly wrong as its called inferProcedureNames but its both procedures and subscriptions. Whatever name needs to be used though would be nice not to have to repeat these in different projects.

https://github.com/aphex/use-trpc/blob/main/src/types.ts#L6

[KATT]

It looks good to me and I'm happy no existing tests fail, but I would like some tests to proves the new behavior is correct

It'd go in here: https://github.com/trpc/trpc/blob/next/packages/server/test/interop/websockets.test.ts

(We haven't fully migrated all tests to v10 but thanks to interop we can still test the behavior)

[aphex]

Hey all bringing my thoughts over from discord to work through here. The focus now is not only getting resubscribe to work but to clean up the stopped and complete states.

As James mentioned stopped and complete are very similar. It is possible we could just have one of them however I am guessing they are supposed to describe two different situations.

Stopped - When a subscription is not actively monitored anymore. This can happen when it is unsubscribed from or if the socket server loses connection to the client or the client isn't pinging and drops due to inactivity.

Complete - When the backing observable for the subscription has completed. Essentially saying there is no more data here to send. This is a pretty rare situation as I doubt most subscriptions "complete" like this, but I don't see why it shouldn't be possible.

So technically I think a subscription could be completed and stopped. Though it does seem fair to say if a subscription is completed, it should just be unsubscribed automatically anyway.

Right now completed and stopped are mixed into a few things. Particularly when it comes to errors and disconnects.

Also since disconnecting is mixed up in this we cannot count on the server to push state to the client. So both need to be aware of these concepts.

So this is how I could see it playing out.

Client side subscribe -> onStarted
Client side unsubscribe -> onStopped
Client side detects disconnect that was not intentional -> error thrown -> onStopped
Client side detects disconnect that was intentional by the server -> error thrown -> onStopped
Client side disconnects and it was that was intentional by the client -> onStopped
Server side observable completes -> onComplete -> onStopped (maybe? 🤷‍♂️ )

For non-intentional disconnects the client will retry connecting, once reconnected is will subscribe again -> onStarted
For intentional disconnects it will not try to reconnect (maybe it should? 🤷‍♂️ )

That seems to separate start/stop and complete as complete is now only an indication that the observable has no more data.

However I could also see an argument that a subscription is just a single observable anyway so in the case of unsubscribe, disconnect, or complete it is always just a onComplete to the client. Subscribe technically does create a new observable. The downside to this is there is no sure way to know if an observable has actually completed, in the sense that there is no more data to be sent.

Perhaps someone is using this observable like a timer, they are going to next every second but then complete after a minute. This could be for more standard stateful servers like a hosted nodeJS express server. For the serverless version of this observables are pretty much ignored and subscriptions are fired by other events (dynamo stream, sns, sqs, etc) however it could be done with a step function or cron and still have the concept of complete

Let me know your thoughts, we can get this all cleared up. also for the record there is a websocket test alright to test disconnect and reconnect/resubscribe. It was just marked skip as its noted to be flakey. So we will make sure that tests is cleaned up and add a few others around this start/stopped/complete state once we land on a direction.

[KATT]

@jlalmes, have you had a chance to look at this yet? :)

[willosof]

Where would be the best place to throw money to get this resolved? 🤑

[aphex]

@KATT no worries on that, but thanks for the offer! Happy to help this project get better. Where is this now? Does it need testing or we all good to go?

Also from that long comment I had above, where did this land? What are the expected results for all the different stop,start, and complete states. I can pull this PR and test to confirm if needed.

[KATT]

@KATT no worries on that, but thanks for the offer! Happy to help this project get better. Where is this now? Does it need testing or we all good to go?

I'll merge it. The tests are pretty extensive so pretty sure it doesn't break any existing behavior.

Also from that long comment I had above, where did this land? What are the expected results for all the different stop,start, and complete states. I can pull this PR and test to confirm if needed.

• open - WS conn is open and we can send messages
• connecting - we're waiting to connect
• closed we've explicitly told the connection to close (but it won't be actually closed until it has finished the pending requests)