socket-mode: fix bug when apps.connections.open returns an error an…

…d won't retry (#1735)
slackapi · Jan 26, 2024 · 46b500d · 46b500d
1 parent a6f2b28
commit 46b500d
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 11 deletions.
diff --git a/packages/socket-mode/src/SocketModeClient.ts b/packages/socket-mode/src/SocketModeClient.ts
@@ -202,15 +202,16 @@ export class SocketModeClient extends EventEmitter {
           .transitionTo(ConnectingState.Reconnecting).withCondition(this.reconnectingCondition.bind(this))
           .transitionTo(ConnectingState.Failed)
     .state(ConnectingState.Reconnecting)
-      .do(async () => {
-        // Trying to reconnect after waiting for a bit...
+      .do(() => new Promise((res, _rej) => {
         this.numOfConsecutiveReconnectionFailures += 1;
         const millisBeforeRetry = this.clientPingTimeoutMillis * this.numOfConsecutiveReconnectionFailures;
         this.logger.debug(`Before trying to reconnect, this client will wait for ${millisBeforeRetry} milliseconds`);
         setTimeout(() => {
-          this.emit(ConnectingState.Authenticating);
+          this.logger.debug('Resolving reconnecting state to continue with reconnect...');
+          res(true);
         }, millisBeforeRetry);
-      })
+      }))
+      .onSuccess().transitionTo(ConnectingState.Authenticating)
       .onFailure().transitionTo(ConnectingState.Failed)
     .state(ConnectingState.Authenticated)
       .onEnter(this.configureAuthenticatedWebSocket.bind(this))

diff --git a/packages/socket-mode/test/integration.spec.js b/packages/socket-mode/test/integration.spec.js
@@ -8,13 +8,7 @@ const sinon = require('sinon');
 const HTTP_PORT = 12345;
 const WSS_PORT = 23456;
 // Basic HTTP server to 'fake' behaviour of `apps.connections.open` endpoint
-const server = createServer((req, res) => {
-  res.writeHead(200, { 'content-type': 'application/json' });
-  res.end(JSON.stringify({
-    ok: true,
-    url: `ws://localhost:${WSS_PORT}/`,
-  }));
-});
+let server = null;
 
 // Basic WS server to fake slack WS endpoint
 let wss = null;
@@ -25,6 +19,13 @@ let client = null;
 
 describe('Integration tests with a WebSocket server', () => {
   beforeEach(() => {
+    server = createServer((req, res) => {
+      res.writeHead(200, { 'content-type': 'application/json' });
+      res.end(JSON.stringify({
+        ok: true,
+        url: `ws://localhost:${WSS_PORT}/`,
+      }));
+    });
     server.listen(HTTP_PORT);
     wss = new WebSocketServer({ port: WSS_PORT });
     wss.on('connection', (ws) => {
@@ -38,7 +39,9 @@ describe('Integration tests with a WebSocket server', () => {
   });
   afterEach(() => {
     server.close();
+    server = null;
     wss.close();
+    wss = null;
     exposed_ws_connection = null;
     client = null;
   });
@@ -69,6 +72,37 @@ describe('Integration tests with a WebSocket server', () => {
       await client.disconnect();
     });
   });
+  describe('catastrophic server behaviour', () => {
+    beforeEach(() => {
+      client = new SocketModeClient({ appToken: 'whatever', logLevel: LogLevel.ERROR, clientOptions: {
+        slackApiUrl: `http://localhost:${HTTP_PORT}/`
+      }, clientPingTimeout: 25});
+    });
+    it('should retry if retrieving a WSS URL fails', async () => {
+      // Shut down the main WS-endpoint-retrieval server - we will customize its behaviour for this test
+      server.close();
+      let num_attempts = 0;
+      server = createServer((req, res) => {
+        num_attempts += 1;
+        res.writeHead(200, { 'content-type': 'application/json' });
+        if (num_attempts < 3) { 
+          res.end(JSON.stringify({
+            ok: false,
+            error: "fatal_error",
+          }));
+        } else {
+          res.end(JSON.stringify({
+            ok: true,
+            url: `ws://localhost:${WSS_PORT}/`,
+          }));
+        }
+      });
+      server.listen(HTTP_PORT);
+      await client.start();
+      assert.equal(num_attempts, 3);
+      await client.disconnect();
+    });
+  });
   describe('failure modes / unexpected messages sent to client', () => { 
     let debugLoggerSpy = sinon.stub();
     const noop = () => {};