Fix vpn reconnecting issue #1993

simmkyu · 2022-02-07T23:48:59Z

Issue #:
#1985

Description of changes:

During retries, the Chime SDK for JavaScript calls the WebSocket.close API on the previous WebSocket object, waits for the WebSocket close event, and creates a new WebSocket object. The issue was that the close event did not arrive in Chrome and Chromium environments when toggling WiFi on a VPN connection. (This behavior is reproducible in a minimal WebSocket app in Chrome 97.0.4692.99.)

With this PR, SDK will continue creating a WebSocket if the WebSocket close event does not arrive for two seconds.

Testing:

Can these tested using a demo application? Please provide reproducible step-by-step instructions.

Testing scenario 1: Reconnection when the WebSocket connectivity is stable

Open the Chime SDK serverless demo and join a meeting.

Open the browser console and run the following script to force retrying the connection.

app.audioVideo.audioVideoController.handleMeetingSessionStatus({
  isFailure: () => true,
  isTerminal: () => false,
  statusCode: () => 17,
  isAudioConnectionFailure: () => false,
  toString: () => 'Error',
});

Ensure that the attendee is present in the meeting.

⠀

Testing scenario 2: Reconnection when the WebSocket connectivity is not stable

Connect to VPN.
Open the Chime SDK serverless demo and join a meeting.
Open the browser console and filter the console by transition. For example, you can type transition in the top input of the Chrome console.
Turn your WiFi off.
Wait until SDK outputs the Reconnect message to the console. SDK starts to retry the connection due to no internet access.
```
[INFO] SDK - transitioning from Connected to Connecting with Reconnect
```
Turn your WiFi on. Ensure that your VPN is online again.
Ensure that the attendee becomes present soon after VPN is online. Before this PR, SDK timed out and failed to reconnect to the session.

Checklist:

Have you successfully run npm run build:release locally?
Yes
Do you add, modify, or delete public API definitions? If yes, has that been reviewed and approved?
No
Do you change the wire protocol, e.g. the request method? If yes, has that been reviewed and approved?
No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…e the "close" event

ltrung · 2022-02-08T00:01:10Z

src/websocketadapter/WebSocketAdapter.ts

+   * @param handler The handler being assigned.
+   * @param eventListener The event listener to add.
+   */
+  removeEventListener(handler: string, eventListener: EventListener): void;


unnecessary probably but shouldn't this be optional ?

A good catch!

ltrung · 2022-02-08T00:22:10Z

src/signalingclient/DefaultSignalingClient.ts

+      };
+
+      // Remove the existing close handler to prevent SDK from opening a new connection.
+      this.webSocket?.removeEventListener('close', this.closeEventHandler);


Do we need to check for undefined/null webSocket here (as the check above in the if condition seems to assume it exists and same as the add below).

oops. Not this.webSocket? but I should've checked the existence of removeEventListener.

I'll add the /* istanbul ignore next */ since there's no point of testing a WebSocket object without removeEventListener. Builders cannot pass the custom WebSocket object.

We should just mark these classes internal in next major version :)

ltrung · 2022-02-08T00:23:01Z

src/signalingclient/DefaultSignalingClient.ts

+      this.webSocket.addEventListener('close', handler);
+      scheduler.start(() => {
+        handler(
+          new CloseEvent('close', { wasClean: false, code: 1006, reason: '', bubbles: false })


Can we document what close code 1006 is and why we choose it here?

Added a comment

…at a builder's subclass does not get broken

simmkyu · 2022-02-08T00:51:39Z

test/signalingclient/DefaultSignalingClient.test.ts

+      testObjects.signalingClient.openConnection(testObjects.request);
+    });
+
+    it('opens a new connection after closing the first one', done => {


Added one more test to ensure that SDK can open a connection twice without delay.

simmkyu added 2 commits February 7, 2022 15:30

Continue to reconnect even if the signaling WebSocket does not receiv…

8bb7abc

…e the "close" event

Keep the original signaling-client behavior as much as possible

1e3237d

simmkyu requested a review from a team as a code owner February 7, 2022 23:48

ltrung reviewed Feb 8, 2022

View reviewed changes

Make the "WebSocketAdapter.removeEventListener" optional to ensure th…

6890779

…at a builder's subclass does not get broken

simmkyu force-pushed the fix-vpn-reconnecting-issue branch from 29ad75b to 6890779 Compare February 8, 2022 00:50

simmkyu commented Feb 8, 2022

View reviewed changes

ltrung approved these changes Feb 8, 2022

View reviewed changes

simmkyu merged commit 87fbd66 into main Feb 8, 2022

simmkyu deleted the fix-vpn-reconnecting-issue branch February 8, 2022 01:09

jing-chen1 mentioned this pull request Feb 8, 2022

iOS device cant automatically reconnect to call after restore internet connection #1931

Closed

4 tasks

xuesichao mentioned this pull request Feb 18, 2022

Occasionally cannot start the meeting on Chrome #2033

Closed

4 tasks

This was referenced Feb 22, 2022

useMeetingStatus doesn't give updated status on ICEGatheringTimeoutWorkaround aws/amazon-chime-sdk-component-library-react#692

Closed

Unavailable audio with an event named receivingAudioDropped #2051

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix vpn reconnecting issue #1993

Fix vpn reconnecting issue #1993

simmkyu commented Feb 7, 2022

ltrung Feb 8, 2022

simmkyu Feb 8, 2022

ltrung Feb 8, 2022

simmkyu Feb 8, 2022

simmkyu Feb 8, 2022

ltrung Feb 8, 2022

ltrung Feb 8, 2022

simmkyu Feb 8, 2022

simmkyu Feb 8, 2022 •

edited

Loading

Fix vpn reconnecting issue #1993

Fix vpn reconnecting issue #1993

Conversation

simmkyu commented Feb 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simmkyu Feb 8, 2022 • edited Loading

Choose a reason for hiding this comment

simmkyu Feb 8, 2022 •

edited

Loading