ueberdosis / hocuspocus

The CRDT Yjs WebSocket backend for conflict-free real-time collaboration in your app.
https://tiptap.dev/docs/hocuspocus/introduction
MIT License
1.19k stars 115 forks source link

provider.destroy will reopen connection #803

Open olee opened 5 months ago

olee commented 5 months ago

Description When issuing a "destroy" on HocuspocusProvider, the underlying websocket connection will be closed as well, however it will always try to reconnect again. The faulty code seems to be the onClose event: https://github.com/ueberdosis/hocuspocus/blob/fa2fb2ed11e8b936eaccb711a52a7cf6cab7b770/packages/provider/src/HocuspocusProviderWebsocket.ts#L526-L529

When I debugged the code, I noticed that event.reason == 'provider_initiated', so I think the code above there just needs a check for this type of close reason and set shouldConnect to false.

Steps to reproduce the bug Steps to reproduce the behavior:

  1. Create HocuspocusProvider
  2. Call HocuspocusProvider.destroy

Expected behavior Connection is properly closed and not reestablished.

olee commented 5 months ago

I noticed that I need to explicitly set preserveConnection option to false to cause the connection to be closed on destroy.

On that note though, I now instead want to try to preserve the connection and even though preserveConnection is set to true, the connection is closed for a moment each time a HocuspocusProvider is destroyed.

EDIT: Tracing this a bit more it seems that when destroying a HocuspocusProvider a CloseMessage on the document which is fine. However, it seems this causes the server to directly close the websocket connection, even though it still might be reused: https://github.com/ueberdosis/hocuspocus/blob/fa2fb2ed11e8b936eaccb711a52a7cf6cab7b770/packages/server/src/MessageReceiver.ts#L96-L102

What is the correct solution when I want to keep my websocket connection alive to use it for multiple HocuspocusProviders?

chwzr commented 3 months ago

Have the same use case here: We are doing multiplexing with one HocuspocusProviderWebsocket and multiple HocuspocusProvider. However when we want to sync to a different document we destroy the HocuspocusProvider and create a new one. This works fine, but this triggers the Websocket to close ( like @olee traced already is done by the server handling the CLOSE message).

In our case we have open an overlay in our application which shows that the HocuspocusProviderWebsocket is not connected. When we change the document this overlay is triggered for 1 second which is annoying.

I think the correct place to fix this is in the CLOSE Message - the client needs to choose whether it wants the server to close the connection or not, depending on other HocuspocusProviders registered to that HocuspocusProviderWebsocket it should not close the connection per default. What do you think?