[MM-53994] Fix potential stuck session due to ws client leak

[streamer45]

Summary

This was a fun one. A session was stuck in the call state after the client explicitly left the call and the RTC connection was closed, only to reconnect on the ws channel every now and then hours after the fact.

They key issue here was that we were improperly using the state of the WebSocket object field (callsClient.ws.ws) as a way to differentiate between an explicit disconnect (e.g. user leaving call) vs an event triggered disconnect (e.g. network issue). Unfortunately the logic didn't account for the potential edge case of an explicit disconnect happening right after a network disconnect.

The sequence of events was roughly the following:

1. A websocket disconnect happens (network/service related).
   a. this.ws is set to null in the ws.onclose handler.
   b. close() gets called and initializes the reconnect handler.
2. User explicitly leaves call before the ws client is able to reconnect.
   a. callsClient.disconnect() gets called.
   b. callsClient.ws.close() gets called.
   c. callsClients.ws.ws is null due to step 1a so we try to reconnect again instead of terminating the ws connection.
3. callsClient object is cleared from window but essentially leaked given its ws client was never properly closed and will endlessly try to reconnect on any future disconnect.

Note: I am aware the above sequence was likely more useful to me, happy to explain anything unclear.

To fix the above we move the reconnect logic into its own method and solely rely on callsClient.ws.closed to decide whether we should reconnect or not.

In addition to the above, we also fix a related issue on the server side which would have prevented the stuck session (not the client side leak though). The problem here was that a reconnected session may have have changed its websocket connection ID which would then differ from the original session ID used to track the RTC session.

Big thanks to @wiggin77 for providing client logs which turned out to be key into figure out what was going on.

Ticket Link

https://mattermost.atlassian.net/browse/MM-53994

[codecov-commenter]

Codecov Report

Patch coverage has no change and project coverage change: -0.03% ⚠️

Comparison is base (5c059da) 5.62% compared to head (099ef84) 5.60%.

Additional details and impacted files

@@           Coverage Diff            @@
##            main    #492      +/-   ##
========================================
- Coverage   5.62%   5.60%   -0.03%     
========================================
  Files         21      21              
  Lines       4178    4193      +15     
========================================
  Hits         235     235              
- Misses      3926    3941      +15     
  Partials      17      17              

Files Changed	Coverage Δ
server/rtcd.go	6.82% <0.00%> (ø)
server/session.go	0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cpoile]

Ah, makes sense. Good comments.

[cpoile]

I suppose we would we have the same problem on mobile?

[streamer45]

Need to check that, thanks.

[streamer45]

Sent mattermost/mattermost-mobile#7513, great catch :)