From 46b500db395dbce6e75d5fba29424e8aec09637a Mon Sep 17 00:00:00 2001 From: Fil Maj Date: Fri, 26 Jan 2024 10:30:53 -0500 Subject: [PATCH] socket-mode: fix bug when `apps.connections.open` returns an error and won't retry (#1735) --- packages/socket-mode/src/SocketModeClient.ts | 9 ++-- packages/socket-mode/test/integration.spec.js | 48 ++++++++++++++++--- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/packages/socket-mode/src/SocketModeClient.ts b/packages/socket-mode/src/SocketModeClient.ts index 67b899c36..a0dde63b3 100644 --- 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 index c61504c6b..b85197bd0 100644 --- 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 = () => {};