meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.25k stars 2.48k forks source link

Handling an abnormally close of the websocket in janus.js #3157

Closed simi1401 closed 11 months ago

simi1401 commented 1 year ago

Added functionality to try the next server if the websocket connection is unexpectedly terminated. Reconnection will only be done when the websocket is closed with closecode 1006. This status code is returned, for example, when the network connection, a reverse proxy or the Janus gateway itself fails. This behaviour was tested with Chrome, Edge and Firefox.

januscla commented 1 year ago

Thanks for your contribution, @simi1401! Please make sure you sign our CLA, as it's a required step before we can merge this.

lminiero commented 1 year ago

Mh I'm not sure this works. I can see the advantage of doing that when you're first trying to connect, but what if you lose connectivity while you've been in a session for a while? In that case the application may want to have a say in what to do, e.g., reconnect to the same server and reclaim the session. With this patch you automatically reconnect somewhere else from scratch, which I'm not convinced about.

tmatth commented 1 year ago

Mh I'm not sure this works. I can see the advantage of doing that when you're first trying to connect, but what if you lose connectivity while you've been in a session for a while? In that case the application may want to have a say in what to do, e.g., reconnect to the same server and reclaim the session. With this patch you automatically reconnect somewhere else from scratch, which I'm not convinced about.

Maybe this behaviour should be behind a default-false option (like autoTryOtherServers but catchier)

simi1401 commented 1 year ago

A flag is probably useful, but I think the flag should also be used with the http API. In the error method of the httpAPICall method, the next server will also be attempted.

What do you think?

lminiero commented 1 year ago

I don't have a strong opinion, especially considering I personally never used the feature, so I guess this is mostly up to who instead does. My only note was about that possibly unexpected behaviour depending on the state, but for what I know it could be what who uses the feature would indeed expect.

lminiero commented 1 year ago

@simi1401 is there any update on this? Thanks!