Decouple websocket liveliness check from y-js document message (on client)

[nthouliss]

Checklist

[ ] Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/ [x] Try to report your issue in the correct repository. Yjs consists of many modules. When in doubt, report it to https://github.com/yjs/yjs/issues/

Is your feature request related to a problem? Please describe. As far as I'm aware the use of Awareness in y-js is an optional add-on. I've created my own websocket server for use with the default y-websocket client and the server doesn't have a use for awareness. However this leads into an issue with disconnects by the client, since y-websocket currently disconnects when it hasn't received a message within a certain amount of time and without awareness this message can be longer than the timeout set by the client.

Describe the solution you'd like I'd like the websocket liveliness connection to be de-coupled from the y-js document on the y-websocket client. The ws library on the server already has a ping call that the client will automatically respond to with ping (as part of the websocket standard). They suggest this in the documentation (for both the server and the client).

https://github.com/websockets/ws#how-to-detect-and-close-broken-connections

Describe alternatives you've considered Adding the awareness to the server anyway. However, I think having the server y-js document handling the liveliness connection is an anti-pattern, this liveliness should be tracked on the websocket connection. Like what is suggested on the ws library

This also goes against the concept that awareness is optional in y-js (again assuming I'm correct on this).

Additional context Adding screenshot of ws documentation

[dmonad]

Hi @nthouliss,

that's right. Awareness is not part of Yjs and should be viewed as a separate CRDT. Authors of providers may use it, or implement a different approach if they choose to do so. While the documentation of ws suggests using the native ping/pong feature, not all browsers (none, as far as I know) support listening to ping/pong events and can't send pings to the server. Browsers only automatically respond to a ping message. In y-websocket, awareness acts as a better ping/pong replacement with very little overhead. It does make sense to let awareness handle liveness checks because that's a native feature of the protocol. If you don't want to use awareness, you can choose to simply not use awareness. Then, y-websocket will use Awareness only as a better ping/pong replacement.

I want to keep the codebase simple, so I'm closing this ticket because I don't want to complicate the codebase by adding an alternative liveness check.

[nthouliss]

While the documentation of ws suggests using the native ping/pong feature, not all browsers (none, as far as I know) support listening to ping/pong events and can't send pings to the server. Browsers only automatically respond to a ping message.

The native websocket library supports listening to pong events. I have observed this behaviour myself from the y-websocket client when sending pings from the server.

By just removing these lines:

https://github.com/yjs/y-websocket/blob/f133fb2db5e4867f1ea7ff8166a0687fd1676fdd/src/y-websocket.js#L297-L303

I can see that the connection is kept alive. The example sends these events and putting in a console log I can see that the client is responding (tested on Chrome, Firefox, and Safari). I'll see if I get together an example to share to show this.

Could the client not keep track of when it last received a ping from the server and start trying to re-connect? The logic would likely be very similar except without the use of awareness. I don't see why it's necessary for the client to send a ping, and it doesn't appear that this happens anyway in the provider code. Socket.io moved away from ping/pong initiated by the client because of the flakiness.

If you don't want to use awareness, you can choose to simply not use awareness.

This is true for the client-side, however not for the server. You'd have to send dead update messages on the document to keep the connection alive. Which is partly why I raised this request, as I'm using the websocket provider but not the server that is part of this repo.

I want to keep the codebase simple, so I'm closing this ticket because I don't want to complicate the codebase by adding an alternative liveness check.

Oh yeah, no expectation at all to add this as an alternative.

[nthouliss]

I thought further about your comment and I realise what you meant by the ping event on the client Websocket library. I forgot that the onmessage event isn't triggered with the ping event from a server and is only handled from the browser.

However, is there any chance that a custom 'ping' event (i.e. a ping message from the server) could be supported in order to decouple the awareness from this? I understand why it's being done, but it does mean that awareness MUST be supported on the server for a y-websocket provider.

(I'm also willing to do these changes myself, I'm not sure of the process around contributing to this repo)

[dmonad]

The server really just needs to forward the awareness messages. It doesn't even have to parse them. As I said, they serve as a ping-pong replacement with minimal overhead and some additional functionality.

From my perspective, it doesn't make sense to add another ping/pong alternative in this repo for me to maintain, just to solve a unique edge case that can be solved by your server implementation in three lines of code (really, simply forward all awareness messages to all clients in the room, that's it..).

If you want to stay compatible with y-websocket, you must implement the awareness protocol anyway. Otherwise, you can simply work on a fork of y-websocket. The code is - by design - really not that complicated.

[nthouliss]

It wasn't my intention to suggest this as an alternative. When I saw the behaviour I just thought it odd that the liveliness checks would be handled this way and thought that perhaps a different method would be a better replacement (not to add another one that you would have to maintain) and only meant to present my case as an example.

I can see that my messages came across as insistent and argumentative, but that definitely wasn't my intent and I apologize. I obviously should have given this idea (and my comments) more thought.

[dmonad]

It's completely okay to discuss, so really no worries :wink:

When I saw the behavior I just thought it odd that the liveliness checks would be handled this way

Yeah, it's super odd. But in a way, it's also fairly elegant to reuse existing messages that must be sent (IMHO).

I understand your point, I think. What I learned as a maintainer, is that I should not solve edge-cases on my side if they can be solved by your implementation. It's understandably super frustrating if the "fix" would be so easy. However, the cost of adding or changing something sometimes has bigger repercussions.

[nthouliss]

What I learned as a maintainer, is that I should not solve edge-cases on my side if they can be solved by your implementation. It's understandably super frustrating if the "fix" would be so easy. However, the cost of adding or changing something sometimes has bigger repercussions.

I understand what you mean and have definitely been in your shoes with similar work. I guess I just needed the reminder 😅

Thanks for the responses and your patience. I really do appreciate the work that you have done and are still doing 😄