Fix can not reconnect websocket after turn off then turn on wifi. Thi…

[namanh-asher]

Summary

On some Samsung devices, when turning off and then turning on Wi-Fi (or when Wi-Fi is not stable), it cannot auto-reconnect.

Ticket Link

none

Checklist

• Added or updated unit tests (required for all new features)
• Has UI changes
• Includes text changes and localization file updates
• Have tested against the 5 core themes to ensure consistency between them.

Device Information

Android - Samsung galaxy s10 plus - Android 12

Screenshots

Before fix:

• 4G on => 4G off => 4G on: ✅
• 4G on => 4G off => Wifi on: ✅
• Wifi on => Wifi off => Wifi on: ❌
• Wifi on => Wifi off => 4G on: ❌

Before-fix-reconnect-websocket.mp4

After fix:

• 4G on => 4G off => 4G on: ✅
• 4G on => 4G off => Wifi on: ✅
• Wifi on => Wifi off => Wifi on: ✅
• Wifi on => Wifi off => 4G on: ✅

After-fix-reconnect-websocket.mp4

Release Note

Fix cannot auto-reconnect websocket after when turning off and then turning on Wi-Fi (or when Wi-Fi is not stable) on some Samsung device

[mattermost-build]

Hello @namanh-asher,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[enahum]

why set the connection as undefined on every error? shouldn't this be for a specific error instead?

[namanh-asher]

@enahum @larkox, there are two reasons why I've added this.conn = undefined here:

• When I added some logs, I noticed that the initialize function returns early at the line if (this.conn === client).
• I'm not entirely sure about when onError is called, but if it happens, it means the connection is closed, and we should restart the connection, right? So, I think just reset the local variable

When I examined the logs, I noticed a difference between 4G and Wi-Fi:

• When I turned off 4G, only onClose is called.
• When I turned off Wi-Fi, onClose is called, and then, approximately 1 or 2 seconds later, onError is called.
  This difference might depend on the device because I've only encountered this error on Samsung devices. Other Android and iOS devices seem to work fine.

[enahum]

I don't think we need to set the connection to undefined unless needed based on the error, in this case seems that the error is caused by the device not being able to resolve the host. Also you mention that onClose is being called and in that case the connection should be set to undefined on line 185, so it makes even less sense

[namanh-asher]

As you can see in the terminal log above, when onClose is called, this.conn is set to undefined, and then initialize is called right after it, updating this.conn. However, 1 or 2 seconds later, onError is called, which triggers another call to initialize. At this point, it returns early because this.conn is the same as before, causing cant reconnect.

I wonder, in the other case of onError, can the WebSocket keep connecting and receive events, or does the WebSocket close every time onError is called?

[enahum]

When erroing out depending on the error it could definitely connect again, so I guess you should check yhe type of error

[larkox]

So... my concern is what happens if we call conn.open() twice. Will it just be handled on the inners of the library and just ignore if the websocket is already opened or trying to open? Or somehow will "connect twice" and send two onConnect events, etc...? Or will it just directly throw an exception and break the manager?

So... if the answer is the first scenario... we can fire and forget.
For the other two, I would make sure the state of the connection is CLOSED (otherwise, I guess we are connecting somewhere else) and make sure we cancel the retry timeout (so open is not called again due to the retries).

But it would be great if you could verify what is the state of the connection when you get there (probably closed, but since your device is giving weird results, it would be great to verify).

[namanh-asher]

@larkox I just tried, use setTimeout for 3 seconds, and then I called conn.open() again when the current readyState is OPEN. It fired onOpen one more time. So maybe we have to check CLOSED.
I cleared this.connectionTimeout, but I'm not sure if there is a timeout to open the WebSocket defined outside of this file.

[larkox]

Any other open should go through the initialize function, and that should handled by the close check you will make, so... I think we are safe on that side.

Thank you very much for checking all these cases for us!

[MuhammadQasim1122]

I also faced this issue, just commenting my fix, I just put this condition if the client was same and the client was open, then we would return else, the same flow would continue:

if (this.conn === client && this.conn?.readyState === WebSocketReadyState.OPEN) {
                return
            }

Just putting it out there, if it makes any difference.

[namanh-asher]

@larkox @enahum I just pushed a new commit, plz take a look. I tested it, and it works fine

@MuhammadQasim1122 wow, it look so easy 😄

[larkox]

I would love to understand better why this works. My guess is that the next time initialize is called, on line 73 it just moves forward because the readyState is not Closed. That would explain why setting the connection to undefined works, since that if statement will not be true. Am I right? If so, what is the readyState in that situation? Any idea how we get to that state?

[larkox]

@yasserfaraazkhan Do you have a Samsung device to test this? If not, do you know if anyone in the QA team has one to test this?

Regarding the QA steps, the description of the issue is really detailed, but if you need anything let me know.

[yasserfaraazkhan]

@yasserfaraazkhan Do you have a Samsung device to test this? If not, do you know if anyone in the QA team has one to test this?

Regarding the QA steps, the description of the issue is really detailed, but if you need anything let me know.

@larkox I dont have Samsung.
I've a different android phone. I'll try this in previous version of app and then check the fix.

[mattermost-build]

Building app in separate branch.

[yasserfaraazkhan]

I've tested these scenarios,

Wifi on => Wifi off => Wifi on: ✅
Wifi on => Wifi off => 4G on: ✅

the connection gets restored when switching. the device had a weak mobile data service.

[larkox]

@yasserfaraazkhan This ticket doesn't have any Jira ticket for QA tracking. Do you want me to create one, or will you create it?

[yasserfaraazkhan]

Hi @larkox , no worries 😃
I can create one for tracking