fix(socket-mode): terminate closing connections earlier if normal close responses fail (#2565)

Author: zimeg

.changeset/full-spiders-enter.md

@@ -0,0 +1,7 @@
+---
+"@slack/socket-mode": patch
+---
+
+fix: terminate closing connections earlier if normal close responses fail
+
+If Slack doesn't respond to a close frame, the WebSocket connection is now force-terminated instead of waiting for a response that won't arrive. Since [disconnects are expected](https://docs.slack.dev/apis/events-api/using-socket-mode/#disconnect) every few hours, this avoids repeated "pong wasn't received" warnings and speeds up reconnection.

packages/socket-mode/src/SlackWebSocket.ts

@@ -159,11 +159,16 @@ export class SlackWebSocket {
       if (this.closeFrameReceived) {
         this.logger.debug('Terminating WebSocket (close frame received).');
         this.terminate();
+      } else if (this.websocket.readyState === WebSocket.CLOSING) {
+        // A close frame was already sent but the peer hasn't responded. Force-terminate rather than
+        // waiting for the ws library's closeTimeout (~30s) while the ping monitor logs repeated warnings.
+        this.logger.debug('Terminating WebSocket (close frame sent but no response, force-terminating).');
+        this.terminate();
       } else {
         // If we haven't received a close frame yet, then we send one to the peer, expecting to receive a close frame
         // in response.
         this.logger.debug('Sending close frame (status=1000).');
-        this.websocket.close(1000); // send a close frame, 1000=Normal Closure
+        this.websocket.close(1000); // 1000 = Normal Closure
       }
     } else {
       this.logger.debug('WebSocket already disconnected, flushing remainder.');

packages/socket-mode/test/integration.test.js

@@ -405,6 +405,49 @@ describe('Integration tests with a WebSocket server', { timeout: 30000 }, () =>
           await reconnectedWaiter;
           await client.disconnect();
         });
+        it('should reconnect if server becomes completely unresponsive and does not respond to close frames', async () => {
+          wss.close();
+          // Use noServer mode so we get access to the raw socket via the upgrade event.
+          // After sending hello, we pause the socket to simulate a fully unresponsive server
+          // that won't respond to pings OR close frames.
+          wss = new WebSocketServer({ noServer: true, autoPong: false });
+          const unresponsiveWsServer = createServer();
+          let rawSocket = null;
+          unresponsiveWsServer.on('upgrade', (req, socket, head) => {
+            rawSocket = socket;
+            wss.handleUpgrade(req, socket, head, (ws) => {
+              ws.on('error', () => {});
+              ws.send(JSON.stringify({ type: 'hello' }));
+              exposed_ws_connection = ws;
+              // Make the server completely unresponsive: it won't process any incoming
+              // data, including ping frames and close frames.
+              socket.pause();
+            });
+          });
+          await new Promise((res) => unresponsiveWsServer.listen(WSS_PORT, res));
+          await client.start();
+          let closeCount = 0;
+          client.on('close', () => {
+            closeCount++;
+          });
+          // Swap in a working WSS for the reconnection attempt
+          client.on('reconnecting', () => {
+            if (rawSocket) rawSocket.destroy();
+            unresponsiveWsServer.close();
+            wss.close();
+            wss = new WebSocketServer({ port: WSS_PORT });
+            wss.on('connection', (ws) => {
+              ws.on('error', () => {});
+              ws.send(JSON.stringify({ type: 'hello' }));
+              exposed_ws_connection = ws;
+            });
+          });
+          const reconnectedWaiter = new Promise((res) => client.on('connected', res));
+          await reconnectedWaiter;
+          // The force-terminate should produce 1 close event per reconnection attempt
+          assert.strictEqual(closeCount, 1);
+          await client.disconnect();
+        });
         it('should reconnect if server does not respond with `pong` message within specified client ping timeout after initially responding with `pong`', async () => {
           wss.close();
           // override the web socket server so that it DOESNT auto-respond to ping messages with a pong, except for the first time