Fix messagingSession infinite reconnect loop on error and also add ba…

…ckwards compatibility with AWS JS SDK V2 for getMessagingSessionEndpoint
aws · Sep 20, 2022 · 69cc694 · 69cc694
1 parent c3e4fdc
commit 69cc694
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `MessagingSession` reconnect loop did not break on error past reconnect deadline. Infinite reconnect loop was caused due to `firstConnectionAttemptTimestamp` not being set as `startedConnectionAttempt` was not invoked. Check https://github.com/aws/amazon-chime-sdk-js/issues/2372 for details.
+- `MessagingSession` `getMessagingSessionEndpoint` call is now backwards compatible with AWS JS SDK v2.
 - Use a default "playback" `latencyHint` when creating the `AudioContext` on Windows. Also adds a `setDefaultLatencyHint` API to `DefaultDeviceController` to allow for overriding.
 
 ## [3.7.0] - 2022-07-05

diff --git a/src/messagingsession/DefaultMessagingSession.ts b/src/messagingsession/DefaultMessagingSession.ts
@@ -108,12 +108,32 @@ export default class DefaultMessagingSession implements MessagingSession {
 
   private async startConnecting(reconnecting: boolean): Promise<void> {
     let endpointUrl = this.configuration.endpointUrl;
+
+    // Moving this reconnect logic can potentially result into an infinite reconnect loop on errors.
+    // Check https://github.com/aws/amazon-chime-sdk-js/issues/2372 for details.
+    if (!reconnecting) {
+      this.reconnectController.reset();
+    }
+    if (this.reconnectController.hasStartedConnectionAttempt()) {
+      this.reconnectController.startedConnectionAttempt(false);
+    } else {
+      this.reconnectController.startedConnectionAttempt(true);
+    }
     // reconnect needs to re-resolve endpoint url, which will also refresh credentials on client if they are expired
     if (reconnecting || endpointUrl === undefined) {
       try {
         if (this.configuration.chimeClient.getMessagingSessionEndpoint instanceof Function) {
-          endpointUrl = (await this.configuration.chimeClient.getMessagingSessionEndpoint())
-            .Endpoint.Url;
+          const response = await this.configuration.chimeClient.getMessagingSessionEndpoint();
+          // Check for aws sdk v3 with v2 style compatibility first
+          if (response.Endpoint?.Url) {
+            endpointUrl = response.Endpoint.Url;
+          } else {
+            // Make aws sdk v2 call
+            const endpoint = await this.configuration.chimeClient
+              .getMessagingSessionEndpoint()
+              .promise();
+            endpointUrl = endpoint.Endpoint.Url;
+          }
         } else {
           endpointUrl = (
             await this.configuration.chimeClient.send(new GetMessagingSessionEndpointCommand({}))

diff --git a/test/messagingsession/DefaultMessagingSession.test.ts b/test/messagingsession/DefaultMessagingSession.test.ts
@@ -64,7 +64,7 @@ describe('DefaultMessagingSession', () => {
     },
   };
 
-  const v2ChimeClient = {
+  const v3ChimeClientV2style = {
     config: {
       region: 'us-east-1',
       credentials: {
@@ -85,6 +85,30 @@ describe('DefaultMessagingSession', () => {
     },
   };
 
+  const v2ChimeClient = {
+    config: {
+      region: 'us-east-1',
+      credentials: {
+        accessKeyId: 'accessKey',
+        secretAccessKey: 'secretKey',
+        sessionToken: 'sessionToken',
+      },
+    },
+    getMessagingSessionEndpoint: function () {
+      return {
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        promise: async function (): Promise<any> {
+          getMessSessionCnt++;
+          return {
+            Endpoint: {
+              Url: ENDPOINT_URL,
+            },
+          };
+        },
+      };
+    },
+  };
+
   class TestSigV4 implements SigV4 {
     lastSignedQueryParams: Map<string, string[]>;
 
@@ -170,6 +194,21 @@ describe('DefaultMessagingSession', () => {
       });
     });
 
+    it('Can start with v3 client with v2 style', done => {
+      configuration.chimeClient = v3ChimeClientV2style;
+      messagingSession.addObserver({
+        messagingSessionDidStart(): void {
+          expect(getMessSessionCnt).to.be.eq(1);
+          done();
+        },
+      });
+      messagingSession.start().then(() => {
+        new TimeoutScheduler(10).start(() => {
+          webSocket.send(SESSION_SUBSCRIBED_MSG);
+        });
+      });
+    });
+
     it('Can start with v2 client', done => {
       configuration.chimeClient = v2ChimeClient;
       messagingSession.addObserver({
@@ -299,21 +338,21 @@ describe('DefaultMessagingSession', () => {
     });
 
     it('can reconnect with failures on getMessagingSession', done => {
-      configuration.chimeClient = v2ChimeClient;
+      configuration.chimeClient = v3ChimeClientV2style;
       let didStartCount = 0;
       let didStartConnecting = 0;
-      const savedClientBehavior = v2ChimeClient.getMessagingSessionEndpoint;
+      const savedClientBehavior = v3ChimeClientV2style.getMessagingSessionEndpoint;
       messagingSession.addObserver({
         messagingSessionDidStartConnecting(reconnecting: boolean): void {
           didStartConnecting++;
           if (!reconnecting) {
             webSocket.addEventListener('open', () => {
               webSocket.close(1006);
-              v2ChimeClient.getMessagingSessionEndpoint = function () {
+              v3ChimeClientV2style.getMessagingSessionEndpoint = function () {
                 throw 'some error';
               };
               setTimeout(function () {
-                v2ChimeClient.getMessagingSessionEndpoint = savedClientBehavior;
+                v3ChimeClientV2style.getMessagingSessionEndpoint = savedClientBehavior;
               }, 100);
             });
           } else {