MM-32591: Reliable Websockets: Client side changes

[agnivade]

We send connection_id and sequence_number as part of the query params.

On receiving an event, we check if it's a hello packet, and if so,
we inspect the connection_id sent with it. If it's different to what
we have already, then we make the API calls and set sequence to 0.

And if the sequence number is different to what we expect, then
we just disconnect the connection, and expect the server to send
from where we left off.

NONE

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

[agnivade]

This needs to be changed to actually take from the server config ServiceSettings.EnableReliableWebSockets

[agnivade]

Hey @devinbinnie - I am handing this over to you. All that's needed is to actually take from the server config instead of a hardcoded boolean flag. There are 2 lint failures, but they might actually change when we change it to take from config.

Feel free to polish it however way you feel. But the logic is all there, and does not need to change.

[devinbinnie]

@agnivade Alright I've setup the correct server config item to drive the use of reliable websockets. I also needed to make a change on the server branch wsfinal to add the item to the client config.

One question: currently if the server changes the config item and the websocket drops, reconnection won't work properly until the webapp is refreshed. Are we okay with that or should we be trying to find something more robust to make sure the switch over is seamless. I don't see the case coming up very often outside of a server admin turning on the feature and rebooting the server, disconnecting everyone.

[agnivade]

Great question. Doesn't the webapp listen to config change events? If the admin changes the setting and reboots the server, the connection should try to reconnect and in the initialize function, getConfig(store.getState()); should return the new state?

[devinbinnie]

Great question. Doesn't the webapp listen to config change events? If the admin changes the setting and reboots the server, the connection should try to reconnect and in the initialize function, getConfig(store.getState()); should return the new state?

Yes you're right, looks like it will call that again.
Funny enough, our reconnect logic really slows down after 8 attempts, going to every 3 minutes instead of 3 seconds (we multiply by the fail count twice), I wonder if that should change.

[agnivade]

Yeah, I am aware of that. I don't want to touch that at this point. It should be a proper exponential backoff. Will look into it once the basic implementation gets in.

[hmhealey]

@agnivade Does this need QA review here or is that being done elsewhere?

[agnivade]

Does this need QA review here or is that being done elsewhere?

Yeah Saturn is doing it on the server PR. He will take all the branches.

[agnivade]

/update-branch

[saturninoabril]

Tested and passed.