y-websocket icon indicating copy to clipboard operation
y-websocket copied to clipboard

Prevent trying to open too many connections to the server when the server is closing the websocket on its own

Open jggc opened this issue 3 years ago • 1 comments

See #67

Currently when the server decides to close the websocket after an authentication failure y-websocket immediately retries to connect since the wsUnsuccessfulReconnects is reinitialized as soon as the onopen event occurs.

This pr adds a 1 second delay before resetting the wsUnsuccessfulReconnects which will allow for the reconnection timeout to be greater than 0 and avoid trying to reopen the connection on every tick which can easily overload the server with a few clients.

If the connection is closed within 1 second of opening, the resetting of wsUnsuccessfulReconnects is canceled and the number of wsUnsuccessfulReconnects will keep growing.

setTimeout solution, really?

I'm usually very much against using timers to "fix" a behavior, it's almost always wrong but here the goal is to determine when a connection is successful. A connection that fails to authenticate is obviously not a successful connection and since we can't authenticate using headers with websockets (Except by hacking the protocol header, which I think stinks a little) we need to establish the websocket connection before closing it because of an authentication failure.

The other main reason why I think setTimeout is a proper solution here is that it specifically only affects the reconnection interval logic. It was obviously not intended to try reconnecting hundreds of times a second and this fix still allows for the exact same logic to happen, and to consider a 1 second connection a "successful connection" to reset the reconnection interval algorithm.

Huly®: YJS-727

jggc avatar Jun 14 '21 21:06 jggc

Agreed, this seems necessary. Temporary fix on my side is to add a 2s delay server-side before closing the ws if the auth isn't valid.

astahmer avatar Aug 12 '21 01:08 astahmer