Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/full-spiders-enter.md
Original file line number Diff line number Diff line change
@@ -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.
7 changes: 6 additions & 1 deletion packages/socket-mode/src/SlackWebSocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 💯

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.');
Expand Down
43 changes: 43 additions & 0 deletions packages/socket-mode/test/integration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Comment thread
zimeg marked this conversation as resolved.
});
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
Expand Down