From c90c987d3ebc1397b0ee98e2f50fced7a03c65ae Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 26 Aug 2026 16:31:31 -0400 Subject: [PATCH 1/7] test(socket-mode): add failing tests reproducing socket leak (#2709) First TDD step for issue #2709. Adds and refines red tests reproducing the three defects behind the WebSocket socket leak introduced by the ws -> undici migration: - disconnect() has no close-handshake timeout, so an unresponsive peer leaves the connection hung and 'close' never fires - cleanup() never destroys the underlying socket - the 'close' reconnect path has no active-connection guard and no timer dedup, so stale/duplicate 'close' events can spawn extra connections All four tests fail for their intended reasons; source fixes follow in a later pass. Co-Authored-By: Claude --- .../socket-mode/src/SlackWebSocket.test.ts | 113 ++++++++++++++++++ .../socket-mode/src/SocketModeClient.test.ts | 29 ++++- 2 files changed, 141 insertions(+), 1 deletion(-) diff --git a/packages/socket-mode/src/SlackWebSocket.test.ts b/packages/socket-mode/src/SlackWebSocket.test.ts index 5a3563bcb..ecb9fbff8 100644 --- a/packages/socket-mode/src/SlackWebSocket.test.ts +++ b/packages/socket-mode/src/SlackWebSocket.test.ts @@ -93,4 +93,117 @@ describe('SlackWebSocket', () => { sinon.assert.calledOnce(discStub); }); }); + + describe('disconnect() with an unresponsive peer (issue #2709)', () => { + // Peer accepts our close frame but never replies with its own: close() moves to CLOSING + // and no 'close' event is ever dispatched. + class DeadPeerWS extends EventTarget { + static CONNECTING = 0; + static OPEN = 1; + static CLOSING = 2; + static CLOSED = 3; + readyState = 1; + closeCalls = 0; + close() { + this.closeCalls += 1; + this.readyState = DeadPeerWS.CLOSING; + } + send(_data: string) {} + } + + it('should force cleanup and emit "close" when the peer never replies with a close frame', () => { + const clock = sandbox.useFakeTimers(); + const ws = new DeadPeerWS(); + SlackWebSocket = proxyquire.load('./SlackWebSocket', { + undici: { + WebSocket: class Fake { + static CONNECTING = 0; + static OPEN = 1; + static CLOSING = 2; + static CLOSED = 3; + constructor() { + // biome-ignore lint/correctness/noConstructorReturn: for test mocking purposes + return ws; + } + }, + CloseEvent, + ErrorEvent, + MessageEvent, + ping: () => {}, + }, + }).SlackWebSocket; + const client = new EventEmitter(); + let closeEmitted = false; + client.on('close', () => { + closeEmitted = true; + }); + const sws = new SlackWebSocket({ + url: 'ws://127.0.0.1/', + client, + clientPingTimeoutMS: 60000, + serverPingTimeoutMS: 60000, + }); + sws.connect(); + + sws.disconnect(); + assert.strictEqual(ws.closeCalls, 1, 'a close frame should have been sent to the peer'); + assert.strictEqual(sws.readyState, DeadPeerWS.CLOSING, 'socket should be CLOSING after sending close frame'); + assert.strictEqual(closeEmitted, false, 'close must not fire before the handshake completes or times out'); + + clock.tick(60000); + + assert.strictEqual( + closeEmitted, + true, + 'expected disconnect() to force cleanup and emit "close" after the peer failed to reply', + ); + }); + }); + + describe('cleanup() with an active connection (issue #2709)', () => { + // Live peer exposing its raw transport so we can assert the socket is torn down, not just dropped. + class LivePeerWS extends EventTarget { + static CONNECTING = 0; + static OPEN = 1; + static CLOSING = 2; + static CLOSED = 3; + readyState = 1; + socket = { destroy: sandbox.spy() }; + close() {} + send(_data: string) {} + } + + it('should destroy the underlying socket during cleanup', () => { + const ws = new LivePeerWS(); + SlackWebSocket = proxyquire.load('./SlackWebSocket', { + undici: { + WebSocket: class Fake { + static CONNECTING = 0; + static OPEN = 1; + static CLOSING = 2; + static CLOSED = 3; + constructor() { + // biome-ignore lint/correctness/noConstructorReturn: for test mocking purposes + return ws; + } + }, + CloseEvent, + ErrorEvent, + MessageEvent, + ping: () => {}, + }, + }).SlackWebSocket; + const sws = new SlackWebSocket({ + url: 'ws://127.0.0.1/', + client: new EventEmitter(), + clientPingTimeoutMS: 1, + serverPingTimeoutMS: 1, + }); + sws.connect(); + + ws.dispatchEvent(new CloseEvent('close', { code: 1000 })); + + sinon.assert.calledOnce(ws.socket.destroy); + }); + }); }); diff --git a/packages/socket-mode/src/SocketModeClient.test.ts b/packages/socket-mode/src/SocketModeClient.test.ts index 368644556..e68ff270f 100644 --- a/packages/socket-mode/src/SocketModeClient.test.ts +++ b/packages/socket-mode/src/SocketModeClient.test.ts @@ -5,7 +5,8 @@ import type { FetchFunction } from '@slack/web-api'; import proxyquire from 'proxyquire'; import sinon from 'sinon'; -import logModule from './logger'; +import logModule, { LogLevel } from './logger'; +import type { SlackWebSocket } from './SlackWebSocket'; import { SocketModeClient } from './SocketModeClient'; import type { SocketModeDispatcher } from './SocketModeOptions'; @@ -386,6 +387,32 @@ describe('SocketModeClient', () => { }); }); }); + + describe("reconnection on 'close' (issue #2709)", () => { + it('should not reconnect on a stale close event while the current connection is still active', () => { + const client = new SocketModeClient({ appToken: 'xapp-', logLevel: LogLevel.ERROR }); + client.websocket = { isActive: () => true } as unknown as SlackWebSocket; + const startStub = sandbox.stub(client, 'start').resolves(undefined as never); + const clock = sandbox.useFakeTimers(); + + client.emit('close'); + clock.tick(1_000_000); + + sinon.assert.notCalled(startStub); + }); + + it('should schedule at most one reconnect when multiple close events fire', () => { + const client = new SocketModeClient({ appToken: 'xapp-', logLevel: LogLevel.ERROR }); + const startStub = sandbox.stub(client, 'start').resolves(undefined as never); + const clock = sandbox.useFakeTimers(); + + client.emit('close'); + client.emit('close'); + clock.tick(1_000_000); + + sinon.assert.calledOnce(startStub); + }); + }); }); async function sleep(ms: number): Promise { From dbb2015d9d1f45484ff0123c69c50e86cdec354d Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 26 Aug 2026 17:00:57 -0400 Subject: [PATCH 2/7] fix(socket-mode): tear down leaked sockets on disconnect (#2709) Porting from `ws` to undici's WebSocket in 3.0.0 dropped every force-close mechanism, leaving ESTABLISHED TCP sockets to accumulate toward Slack's per-app connection cap. Three independent defects, each with a preceding failing test: - disconnect() sent a close frame with no timeout, so a dead peer left the socket in CLOSING forever. Arm a 30s close-handshake timeout that forces cleanup. - cleanup() never destroyed the underlying TCP socket. When no user dispatcher is supplied, capture the raw socket via a custom Agent connector and destroy it (plus the Agent) on cleanup. A user-supplied dispatcher owns its socket and relies on the close-handshake timeout; this limitation is documented on the dispatcher option. - the 'close' handler reconnected on every close. Guard against stale closes while still active and dedupe overlapping closes so at most one reconnect is scheduled. Co-Authored-By: Claude --- .../socket-mode/src/SlackWebSocket.test.ts | 45 +++------------ packages/socket-mode/src/SlackWebSocket.ts | 57 +++++++++++++++++-- packages/socket-mode/src/SocketModeClient.ts | 2 + 3 files changed, 63 insertions(+), 41 deletions(-) diff --git a/packages/socket-mode/src/SlackWebSocket.test.ts b/packages/socket-mode/src/SlackWebSocket.test.ts index ecb9fbff8..b231d3e1b 100644 --- a/packages/socket-mode/src/SlackWebSocket.test.ts +++ b/packages/socket-mode/src/SlackWebSocket.test.ts @@ -160,50 +160,23 @@ describe('SlackWebSocket', () => { }); }); - describe('cleanup() with an active connection (issue #2709)', () => { - // Live peer exposing its raw transport so we can assert the socket is torn down, not just dropped. - class LivePeerWS extends EventTarget { - static CONNECTING = 0; - static OPEN = 1; - static CLOSING = 2; - static CLOSED = 3; - readyState = 1; - socket = { destroy: sandbox.spy() }; - close() {} - send(_data: string) {} - } - - it('should destroy the underlying socket during cleanup', () => { - const ws = new LivePeerWS(); - SlackWebSocket = proxyquire.load('./SlackWebSocket', { - undici: { - WebSocket: class Fake { - static CONNECTING = 0; - static OPEN = 1; - static CLOSING = 2; - static CLOSED = 3; - constructor() { - // biome-ignore lint/correctness/noConstructorReturn: for test mocking purposes - return ws; - } - }, - CloseEvent, - ErrorEvent, - MessageEvent, - ping: () => {}, - }, - }).SlackWebSocket; + describe('cleanup() with a captured socket (issue #2709)', () => { + it('should destroy the captured underlying socket during cleanup', () => { const sws = new SlackWebSocket({ url: 'ws://127.0.0.1/', client: new EventEmitter(), clientPingTimeoutMS: 1, serverPingTimeoutMS: 1, }); - sws.connect(); + const destroy = sandbox.spy(); + (sws as unknown as { capturedSocket: { destroy: () => void; destroyed: boolean } }).capturedSocket = { + destroy, + destroyed: false, + }; - ws.dispatchEvent(new CloseEvent('close', { code: 1000 })); + sws.disconnect(); - sinon.assert.calledOnce(ws.socket.destroy); + sinon.assert.calledOnce(destroy); }); }); }); diff --git a/packages/socket-mode/src/SlackWebSocket.ts b/packages/socket-mode/src/SlackWebSocket.ts index 3fcb24a13..b529380ad 100644 --- a/packages/socket-mode/src/SlackWebSocket.ts +++ b/packages/socket-mode/src/SlackWebSocket.ts @@ -1,7 +1,8 @@ import { channel } from 'node:diagnostics_channel'; +import type { Socket } from 'node:net'; import type { EventEmitter } from 'eventemitter3'; -import { CloseEvent, type Dispatcher, ErrorEvent, MessageEvent, ping, WebSocket } from 'undici'; +import { Agent, buildConnector, CloseEvent, type Dispatcher, ErrorEvent, MessageEvent, ping, WebSocket } from 'undici'; import { SMWebsocketError } from './errors'; import log, { type Logger, LogLevel } from './logger'; @@ -9,6 +10,8 @@ import type { SocketModeDispatcher } from './SocketModeOptions'; export const WS_READY_STATES = ['CONNECTING', 'OPEN', 'CLOSING', 'CLOSED']; +const CLOSE_HANDSHAKE_TIMEOUT_MS = 30_000; + interface PingPongMessage { websocket: WebSocket; payload: Buffer; @@ -38,7 +41,12 @@ export interface SlackWebSocketOptions { logger?: Logger; /** @description Delay between this client sending a `ping` message, in milliseconds. */ pingInterval?: number; - /** @description An undici Dispatcher used to establish the WebSocket connection (e.g. ProxyAgent). */ + /** + * @description An undici Dispatcher used to establish the WebSocket connection (e.g. ProxyAgent). + * When omitted, this class creates its own Agent and force-destroys the underlying socket on cleanup. + * When supplied, the socket is owned by your dispatcher and cannot be force-closed here; a stalled close + * handshake instead falls back to a timeout before cleanup runs. + */ dispatcher?: SocketModeDispatcher; /** @description Whether this WebSocket should DEBUG log ping and pong events. `false` by default. */ pingPongLoggingEnabled?: boolean; @@ -68,6 +76,10 @@ export class SlackWebSocket { private websocket: WebSocket | null; + private capturedSocket: Socket | null = null; + + private ownAgent: Agent | null = null; + /** * The last timetamp that this WebSocket received pong from the server */ @@ -89,6 +101,11 @@ export class SlackWebSocket { */ private clientPingTimeout: NodeJS.Timeout | undefined; + /** + * Reference to the timer that force-closes the connection if the peer never completes the close handshake + */ + private closeHandshakeTimeout: ReturnType | undefined; + private openHandler: (() => void) | null = null; private errorHandler: ((event: Event) => void) | null = null; private messageHandler: ((event: Event) => void) | null = null; @@ -136,7 +153,26 @@ export class SlackWebSocket { public connect(): void { this.logger.debug('Initiating new WebSocket connection.'); - this.websocket = new WebSocket(this.options.url, { dispatcher: this.options.dispatcher as Dispatcher }); + let dispatcher: Dispatcher; + if (this.options.dispatcher) { + dispatcher = this.options.dispatcher as Dispatcher; + } else { + const baseConnect = buildConnector({}); + this.ownAgent = new Agent({ + connect: (opts, cb) => { + baseConnect(opts, (err, socket) => { + if (err) { + cb(err, null); + return; + } + this.capturedSocket = socket as Socket; + cb(null, socket as Socket); + }); + }, + }); + dispatcher = this.ownAgent; + } + this.websocket = new WebSocket(this.options.url, { dispatcher }); this.openHandler = () => { this.logger.debug('WebSocket open event received (connection established)!'); @@ -216,8 +252,6 @@ export class SlackWebSocket { this.logger.debug('Terminating WebSocket (close frame received).'); this.cleanup(); } 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.cleanup(); } else { @@ -225,6 +259,10 @@ export class SlackWebSocket { // in response. this.logger.debug('Sending close frame (status=1000).'); this.websocket.close(1000); // 1000 = Normal Closure + this.closeHandshakeTimeout = setTimeout(() => { + this.logger.warn('Peer did not complete the close handshake in time; forcing cleanup.'); + this.cleanup(); + }, CLOSE_HANDSHAKE_TIMEOUT_MS); } } else { this.logger.debug('WebSocket already disconnected, flushing remainder.'); @@ -250,9 +288,18 @@ export class SlackWebSocket { if (this.pongHandler) SlackWebSocket.pongChannel.unsubscribe(this.pongHandler); this.pingHandler = null; this.pongHandler = null; + if (this.capturedSocket && !this.capturedSocket.destroyed) { + this.capturedSocket.destroy(); + } + this.capturedSocket = null; + if (this.ownAgent) { + this.ownAgent.destroy().catch(() => {}); + this.ownAgent = null; + } this.websocket = null; clearTimeout(this.serverPingTimeout); clearInterval(this.clientPingTimeout); + clearTimeout(this.closeHandshakeTimeout); // Emit event back to client letting it know connection has closed (in case it needs to reconnect if // reconnecting is enabled) this.options.client.emit('close'); diff --git a/packages/socket-mode/src/SocketModeClient.ts b/packages/socket-mode/src/SocketModeClient.ts index 9f07c58c4..93bf8c7c0 100644 --- a/packages/socket-mode/src/SocketModeClient.ts +++ b/packages/socket-mode/src/SocketModeClient.ts @@ -157,6 +157,8 @@ export class SocketModeClient extends EventEmitter { }); this.on('close', () => { // Underlying WebSocket connection was closed, possibly reconnect. + if (this.websocket?.isActive()) return; + if (this.reconnectionTimer) return; if (!this.shuttingDown && this.autoReconnectEnabled) { this.delayReconnectAttempt(this.start); } else { From 3341c4e9ba7f162827760fa1568e5fc71a916292 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Thu, 27 Aug 2026 12:22:39 -0400 Subject: [PATCH 3/7] chore(socket-mode): extract buildDefaultDispatcher() and drop redundant Agent tracking Internal refactor of SlackWebSocket with no public API or behavior change. - Lift the inline undici `Agent` construction in `connect()` into a private `buildDefaultDispatcher()` helper, reducing `connect()` to a two-line dispatcher selection. - Remove the `ownAgent` field and its `cleanup()` teardown. undici already evicts and closes its pooled dispatcher when the client disconnects at WebSocket upgrade, so `Agent.destroy()` was a no-op on established connections; the real teardown remains `defaultSocket.destroy()`. - Rename `capturedSocket` -> `defaultSocket` (the socket is only captured on the default path) and widen its type to `Socket | TLSSocket | null` to match undici's connector callback, so the helper stores and forwards the socket without casts. Co-Authored-By: Claude --- .changeset/socket-mode-default-dispatcher.md | 7 +++ .../socket-mode/src/SlackWebSocket.test.ts | 2 +- packages/socket-mode/src/SlackWebSocket.ts | 58 +++++++++---------- 3 files changed, 37 insertions(+), 30 deletions(-) create mode 100644 .changeset/socket-mode-default-dispatcher.md diff --git a/.changeset/socket-mode-default-dispatcher.md b/.changeset/socket-mode-default-dispatcher.md new file mode 100644 index 000000000..6f8d9a677 --- /dev/null +++ b/.changeset/socket-mode-default-dispatcher.md @@ -0,0 +1,7 @@ +--- +"@slack/socket-mode": patch +--- + +chore(socket-mode): extract default dispatcher into `buildDefaultDispatcher()` and drop redundant Agent tracking + +Internal refactor with no public API or behavior change. The inline undici `Agent` construction in `connect()` is lifted into a private `buildDefaultDispatcher()` helper, and the now-redundant `ownAgent` field and its `cleanup()` teardown are removed — undici already evicts and closes the pooled dispatcher at WebSocket upgrade, so the real teardown remains `defaultSocket.destroy()` (unchanged). diff --git a/packages/socket-mode/src/SlackWebSocket.test.ts b/packages/socket-mode/src/SlackWebSocket.test.ts index b231d3e1b..fbbbd63a1 100644 --- a/packages/socket-mode/src/SlackWebSocket.test.ts +++ b/packages/socket-mode/src/SlackWebSocket.test.ts @@ -169,7 +169,7 @@ describe('SlackWebSocket', () => { serverPingTimeoutMS: 1, }); const destroy = sandbox.spy(); - (sws as unknown as { capturedSocket: { destroy: () => void; destroyed: boolean } }).capturedSocket = { + (sws as unknown as { defaultSocket: { destroy: () => void; destroyed: boolean } }).defaultSocket = { destroy, destroyed: false, }; diff --git a/packages/socket-mode/src/SlackWebSocket.ts b/packages/socket-mode/src/SlackWebSocket.ts index b529380ad..e64f49c71 100644 --- a/packages/socket-mode/src/SlackWebSocket.ts +++ b/packages/socket-mode/src/SlackWebSocket.ts @@ -1,5 +1,6 @@ import { channel } from 'node:diagnostics_channel'; import type { Socket } from 'node:net'; +import type { TLSSocket } from 'node:tls'; import type { EventEmitter } from 'eventemitter3'; import { Agent, buildConnector, CloseEvent, type Dispatcher, ErrorEvent, MessageEvent, ping, WebSocket } from 'undici'; @@ -76,9 +77,7 @@ export class SlackWebSocket { private websocket: WebSocket | null; - private capturedSocket: Socket | null = null; - - private ownAgent: Agent | null = null; + private defaultSocket: Socket | TLSSocket | null = null; /** * The last timetamp that this WebSocket received pong from the server @@ -153,25 +152,9 @@ export class SlackWebSocket { public connect(): void { this.logger.debug('Initiating new WebSocket connection.'); - let dispatcher: Dispatcher; - if (this.options.dispatcher) { - dispatcher = this.options.dispatcher as Dispatcher; - } else { - const baseConnect = buildConnector({}); - this.ownAgent = new Agent({ - connect: (opts, cb) => { - baseConnect(opts, (err, socket) => { - if (err) { - cb(err, null); - return; - } - this.capturedSocket = socket as Socket; - cb(null, socket as Socket); - }); - }, - }); - dispatcher = this.ownAgent; - } + const dispatcher: Dispatcher = this.options.dispatcher + ? (this.options.dispatcher as Dispatcher) + : this.buildDefaultDispatcher(); this.websocket = new WebSocket(this.options.url, { dispatcher }); this.openHandler = () => { @@ -241,6 +224,27 @@ export class SlackWebSocket { SlackWebSocket.pongChannel.subscribe(this.pongHandler); } + /** + * The `connect` hook captures the underlying socket into `this.defaultSocket` so `cleanup()` can + * force-destroy it: undici's `WebSocket` hides its socket and detaches it from the pool at upgrade, + * leaving no other way to close a stalled peer. + */ + private buildDefaultDispatcher(): Dispatcher { + const baseConnect = buildConnector({}); + return new Agent({ + connect: (opts, cb) => { + baseConnect(opts, (err, socket) => { + if (!socket) { + cb(err ?? new Error('Socket Mode connector returned no socket'), null); + return; + } + this.defaultSocket = socket; + cb(null, socket); + }); + }, + }); + } + /** * Disconnects the WebSocket connection with Slack, if connected. */ @@ -288,14 +292,10 @@ export class SlackWebSocket { if (this.pongHandler) SlackWebSocket.pongChannel.unsubscribe(this.pongHandler); this.pingHandler = null; this.pongHandler = null; - if (this.capturedSocket && !this.capturedSocket.destroyed) { - this.capturedSocket.destroy(); - } - this.capturedSocket = null; - if (this.ownAgent) { - this.ownAgent.destroy().catch(() => {}); - this.ownAgent = null; + if (this.defaultSocket && !this.defaultSocket.destroyed) { + this.defaultSocket.destroy(); } + this.defaultSocket = null; this.websocket = null; clearTimeout(this.serverPingTimeout); clearInterval(this.clientPingTimeout); From 86cd21cfed04bd0253977755340fc51ad84e7bf8 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Thu, 27 Aug 2026 12:46:02 -0400 Subject: [PATCH 4/7] docs(socket-mode): tighten dispatcher option JSDoc Reframe the `dispatcher` option comment around overriding the default dispatcher, dropping the omitted/supplied split for a shorter, clearer note on the one practical consequence (force-close vs. timeout fallback). Co-Authored-By: Claude --- packages/socket-mode/src/SlackWebSocket.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/socket-mode/src/SlackWebSocket.ts b/packages/socket-mode/src/SlackWebSocket.ts index e64f49c71..18398a30a 100644 --- a/packages/socket-mode/src/SlackWebSocket.ts +++ b/packages/socket-mode/src/SlackWebSocket.ts @@ -44,9 +44,8 @@ export interface SlackWebSocketOptions { pingInterval?: number; /** * @description An undici Dispatcher used to establish the WebSocket connection (e.g. ProxyAgent). - * When omitted, this class creates its own Agent and force-destroys the underlying socket on cleanup. - * When supplied, the socket is owned by your dispatcher and cannot be force-closed here; a stalled close - * handshake instead falls back to a timeout before cleanup runs. + * Overrides the default dispatcher, which force-destroys the underlying socket on cleanup; a custom + * dispatcher cannot, so a stalled close handshake falls back to a timeout instead. */ dispatcher?: SocketModeDispatcher; /** @description Whether this WebSocket should DEBUG log ping and pong events. `false` by default. */ From e4936e4974af9fb64010f7a1370ee6fdd4b90972 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Thu, 27 Aug 2026 13:09:43 -0400 Subject: [PATCH 5/7] chore(socket-mode): reframe changeset as the #2709 socket-leak fix Replace the internal-refactor changeset with one describing the user-facing fix: leaked TCP sockets on disconnect (#2709). Co-Authored-By: Claude --- .changeset/socket-mode-default-dispatcher.md | 7 ------- .changeset/socket-mode-disconnect-socket-leak.md | 9 +++++++++ 2 files changed, 9 insertions(+), 7 deletions(-) delete mode 100644 .changeset/socket-mode-default-dispatcher.md create mode 100644 .changeset/socket-mode-disconnect-socket-leak.md diff --git a/.changeset/socket-mode-default-dispatcher.md b/.changeset/socket-mode-default-dispatcher.md deleted file mode 100644 index 6f8d9a677..000000000 --- a/.changeset/socket-mode-default-dispatcher.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -"@slack/socket-mode": patch ---- - -chore(socket-mode): extract default dispatcher into `buildDefaultDispatcher()` and drop redundant Agent tracking - -Internal refactor with no public API or behavior change. The inline undici `Agent` construction in `connect()` is lifted into a private `buildDefaultDispatcher()` helper, and the now-redundant `ownAgent` field and its `cleanup()` teardown are removed — undici already evicts and closes the pooled dispatcher at WebSocket upgrade, so the real teardown remains `defaultSocket.destroy()` (unchanged). diff --git a/.changeset/socket-mode-disconnect-socket-leak.md b/.changeset/socket-mode-disconnect-socket-leak.md new file mode 100644 index 000000000..dfc768344 --- /dev/null +++ b/.changeset/socket-mode-disconnect-socket-leak.md @@ -0,0 +1,9 @@ +--- +"@slack/socket-mode": patch +--- + +Fix leaked TCP sockets on disconnect ([#2709](https://github.com/slackapi/node-slack-sdk/issues/2709)). Moving from `ws` to undici's `WebSocket` in 3.0.0 dropped every force-close mechanism, so `ESTABLISHED` sockets were never torn down and accumulated toward Slack's per-app connection cap. + +- `disconnect()` now arms a 30s close-handshake timeout, so a peer that never replies to the close frame no longer leaves the socket stuck in `CLOSING` indefinitely. +- On the default dispatcher path, the underlying TCP socket is now captured and force-destroyed during cleanup. A user-supplied `dispatcher` owns its own socket and instead relies on the close-handshake timeout — this limitation is documented on the `dispatcher` option. +- The `close` handler no longer schedules a reconnect on stale or overlapping close events, so at most one reconnect is scheduled per disconnect. From 4962460509e336127fb8f76a7653d7282e2d8d9d Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Thu, 27 Aug 2026 13:09:43 -0400 Subject: [PATCH 6/7] refactor(socket-mode): rename connect callback param to `callback` Rename the undici connector callback param from `cb` to `callback` in buildDefaultDispatcher() for readability. No behavior change. Co-Authored-By: Claude --- packages/socket-mode/src/SlackWebSocket.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/socket-mode/src/SlackWebSocket.ts b/packages/socket-mode/src/SlackWebSocket.ts index 18398a30a..4340db39b 100644 --- a/packages/socket-mode/src/SlackWebSocket.ts +++ b/packages/socket-mode/src/SlackWebSocket.ts @@ -231,14 +231,14 @@ export class SlackWebSocket { private buildDefaultDispatcher(): Dispatcher { const baseConnect = buildConnector({}); return new Agent({ - connect: (opts, cb) => { + connect: (opts, callback) => { baseConnect(opts, (err, socket) => { if (!socket) { - cb(err ?? new Error('Socket Mode connector returned no socket'), null); + callback(err ?? new Error('Socket Mode connector returned no socket'), null); return; } this.defaultSocket = socket; - cb(null, socket); + callback(null, socket); }); }, }); From 654a27b420bb756e8fd6809940e9874ed23f7786 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Thu, 27 Aug 2026 14:04:32 -0400 Subject: [PATCH 7/7] test(socket-mode): cover buildDefaultDispatcher() connect hook Add unit tests for the default dispatcher's connect hook, the core of the #2709 socket-leak fix that had no direct coverage: success captures the socket into defaultSocket and calls back (null, socket); a connector error propagates without capturing; a (null, null) result synthesizes a "returned no socket" Error. Also drop the redundant "(issue #2709)" suffix from the two adjacent describe titles for consistency. Co-Authored-By: Claude --- .../socket-mode/src/SlackWebSocket.test.ts | 72 ++++++++++++++++++- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/packages/socket-mode/src/SlackWebSocket.test.ts b/packages/socket-mode/src/SlackWebSocket.test.ts index fbbbd63a1..3ed500919 100644 --- a/packages/socket-mode/src/SlackWebSocket.test.ts +++ b/packages/socket-mode/src/SlackWebSocket.test.ts @@ -94,7 +94,7 @@ describe('SlackWebSocket', () => { }); }); - describe('disconnect() with an unresponsive peer (issue #2709)', () => { + describe('disconnect() with an unresponsive peer', () => { // Peer accepts our close frame but never replies with its own: close() moves to CLOSING // and no 'close' event is ever dispatched. class DeadPeerWS extends EventTarget { @@ -160,7 +160,7 @@ describe('SlackWebSocket', () => { }); }); - describe('cleanup() with a captured socket (issue #2709)', () => { + describe('cleanup() with a captured socket', () => { it('should destroy the captured underlying socket during cleanup', () => { const sws = new SlackWebSocket({ url: 'ws://127.0.0.1/', @@ -179,4 +179,72 @@ describe('SlackWebSocket', () => { sinon.assert.calledOnce(destroy); }); }); + + describe('buildDefaultDispatcher() connect hook', () => { + type ConnectCb = (err: Error | null, socket: unknown) => void; + type ConnectFn = (opts: unknown, cb: ConnectCb) => void; + + function loadWith(baseConnect: ConnectFn) { + let capturedConnect!: ConnectFn; + const SWS = proxyquire.load('./SlackWebSocket', { + undici: { + WebSocket: WSMock, + CloseEvent, + ErrorEvent, + MessageEvent, + ping: () => {}, + buildConnector: () => baseConnect, + Agent: class { + constructor(o: { connect: ConnectFn }) { + capturedConnect = o.connect; + } + }, + }, + }).SlackWebSocket; + const sws = new SWS({ + url: 'ws://127.0.0.1/', + client: new EventEmitter(), + clientPingTimeoutMS: 1, + serverPingTimeoutMS: 1, + }); + sws.connect(); // no dispatcher provided => builds the default dispatcher => captures the hook + return { sws, invoke: (cb: ConnectCb) => capturedConnect({}, cb) }; + } + + it('should capture the socket and call back with (null, socket) on success', () => { + const fakeSocket = { destroy() {}, destroyed: false }; + const { sws, invoke } = loadWith((_opts, cb) => cb(null, fakeSocket)); + let cbErr: unknown = 'unset'; + let cbSocket: unknown = 'unset'; + invoke((err, socket) => { + cbErr = err; + cbSocket = socket; + }); + assert.strictEqual(cbErr, null); + assert.strictEqual(cbSocket, fakeSocket); + assert.strictEqual((sws as unknown as { defaultSocket: unknown }).defaultSocket, fakeSocket); + }); + + it('should propagate the connector error and not capture a socket', () => { + const boom = new Error('connect failed'); + const { sws, invoke } = loadWith((_opts, cb) => cb(boom, null)); + let received: unknown = 'unset'; + invoke((err) => { + received = err; + }); + assert.strictEqual(received, boom); + assert.strictEqual((sws as unknown as { defaultSocket: unknown }).defaultSocket, null); + }); + + it('should synthesize an error when the connector yields no socket and no error', () => { + const { sws, invoke } = loadWith((_opts, cb) => cb(null, null)); + let received: unknown; + invoke((err) => { + received = err; + }); + assert.ok(received instanceof Error); + assert.match((received as Error).message, /returned no socket/); + assert.strictEqual((sws as unknown as { defaultSocket: unknown }).defaultSocket, null); + }); + }); });