From 6250e4ce3899f2305bceb8a581f502638b82ed53 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Tue, 22 Sep 2026 11:27:56 -0400 Subject: [PATCH 1/3] fix(socket-mode): route ping/pong diagnostics by identity, not instanceof (#2743) The undici ping/pong diagnostics channels are process-global: every undici WebSocket in the process publishes to them, including Node's built-in global WebSocket, which is a separate undici copy from the SDK's import. The old guard checked `message.websocket instanceof WebSocket` before the identity check, so a frame from any other undici copy failed instanceof and logged a spurious WARN on an otherwise healthy Slack connection (issue reporter: a nostr-tools relay pinging every 30s). Match by reference identity (`message.websocket === this.websocket`) instead, folding identity and shape into one guard that returns silently for frames that are not this socket's. Slack's own frames are unaffected. Adds tests covering foreign-copy and same-socket frames on both channels. Co-Authored-By: Claude --- .changeset/socket-mode-pingpong-instanceof.md | 5 ++ .../socket-mode/src/SlackWebSocket.test.ts | 72 +++++++++++++++++++ packages/socket-mode/src/SlackWebSocket.ts | 40 ++++------- 3 files changed, 92 insertions(+), 25 deletions(-) create mode 100644 .changeset/socket-mode-pingpong-instanceof.md diff --git a/.changeset/socket-mode-pingpong-instanceof.md b/.changeset/socket-mode-pingpong-instanceof.md new file mode 100644 index 000000000..4d68185c3 --- /dev/null +++ b/.changeset/socket-mode-pingpong-instanceof.md @@ -0,0 +1,5 @@ +--- +"@slack/socket-mode": patch +--- + +fix(socket-mode): stop spurious ping/pong WARN from other undici WebSockets ([#2743](https://github.com/slackapi/node-slack-sdk/issues/2743)). Route diagnostics frames by reference identity instead of `instanceof`, which failed across undici copies (e.g. Node's global `WebSocket`). diff --git a/packages/socket-mode/src/SlackWebSocket.test.ts b/packages/socket-mode/src/SlackWebSocket.test.ts index 3ed500919..a1fde4e4f 100644 --- a/packages/socket-mode/src/SlackWebSocket.test.ts +++ b/packages/socket-mode/src/SlackWebSocket.test.ts @@ -1,4 +1,5 @@ import assert from 'node:assert/strict'; +import { channel } from 'node:diagnostics_channel'; import { afterEach, beforeEach, describe, it } from 'node:test'; import { ConsoleLogger } from '@slack/logger'; import EventEmitter from 'eventemitter3'; @@ -247,4 +248,75 @@ describe('SlackWebSocket', () => { assert.strictEqual((sws as unknown as { defaultSocket: unknown }).defaultSocket, null); }); }); + + describe('ping/pong diagnostics channel filtering', () => { + const pingChannel = channel('undici:websocket:ping'); + const pongChannel = channel('undici:websocket:pong'); + + function connect() { + const ws = new WSMock(); + const SWS = proxyquire.load('./SlackWebSocket', { + undici: { + WebSocket: class Fake { + constructor() { + // biome-ignore lint/correctness/noConstructorReturn: for test mocking purposes + return ws; + } + }, + CloseEvent, + ErrorEvent, + MessageEvent, + ping: () => {}, + }, + }).SlackWebSocket; + const logger = new ConsoleLogger(); + const warn = sandbox.spy(logger, 'warn'); + const sws = new SWS({ + url: 'ws://127.0.0.1/', + client: new EventEmitter(), + clientPingTimeoutMS: 1, + serverPingTimeoutMS: 1, + logger, + }); + const monitorPingFromSlack = sandbox.stub( + sws as unknown as { monitorPingFromSlack: () => void }, + 'monitorPingFromSlack', + ); + const lastPong = () => (sws as unknown as { lastPongReceivedTimestamp?: number }).lastPongReceivedTimestamp; + sws.connect(); + return { ws, warn, monitorPingFromSlack, lastPong }; + } + + // A plain object stands in for a WebSocket from a different undici copy: it fails `instanceof` our + // import, which is exactly the frame that used to warn. A WSMock instance would pass `instanceof`. + const foreignSocket = {}; + + it('ignores a ping for another undici copy socket, without warning', () => { + const { warn, monitorPingFromSlack } = connect(); + pingChannel.publish({ websocket: foreignSocket, payload: Buffer.from('x') }); + sinon.assert.notCalled(monitorPingFromSlack); + sinon.assert.notCalled(warn); + }); + + it('processes a ping for this socket', () => { + const { ws, warn, monitorPingFromSlack } = connect(); + pingChannel.publish({ websocket: ws, payload: Buffer.from('x') }); + sinon.assert.calledOnce(monitorPingFromSlack); + sinon.assert.notCalled(warn); + }); + + it('ignores a pong for another undici copy socket, without warning', () => { + const { warn, lastPong } = connect(); + pongChannel.publish({ websocket: foreignSocket, payload: Buffer.from('x') }); + assert.strictEqual(lastPong(), undefined); + sinon.assert.notCalled(warn); + }); + + it('processes a pong for this socket', () => { + const { ws, warn, lastPong } = connect(); + pongChannel.publish({ websocket: ws, payload: Buffer.from('x') }); + assert.strictEqual(typeof lastPong(), 'number'); + sinon.assert.notCalled(warn); + }); + }); }); diff --git a/packages/socket-mode/src/SlackWebSocket.ts b/packages/socket-mode/src/SlackWebSocket.ts index 4340db39b..d175497e6 100644 --- a/packages/socket-mode/src/SlackWebSocket.ts +++ b/packages/socket-mode/src/SlackWebSocket.ts @@ -18,17 +18,16 @@ interface PingPongMessage { payload: Buffer; } -function isPingPongMessage(message: unknown): message is PingPongMessage { - if (typeof message !== 'object' || message === null) { - return false; - } - if (!('websocket' in message && message.websocket instanceof WebSocket)) { - return false; - } - if (!('payload' in message && Buffer.isBuffer(message.payload))) { - return false; - } - return true; +// Match by reference identity, not `instanceof`: the channels are process-global and a different undici copy (e.g. Node's global WebSocket) fails `instanceof` against our import. +function isMessageForSocket(message: unknown, websocket: WebSocket): message is PingPongMessage { + return ( + typeof message === 'object' && + message !== null && + 'websocket' in message && + message.websocket === websocket && + 'payload' in message && + Buffer.isBuffer(message.payload) + ); } export interface SlackWebSocketOptions { @@ -194,29 +193,20 @@ export class SlackWebSocket { }; this.websocket.addEventListener('close', this.closeHandler); - // Subscribe to undici diagnostics_channel for WebSocket ping/pong frame events. - // These channels fire for ALL undici WebSocket instances, so we filter by matching instance. + // These channels fire for every undici WebSocket in the process, so filter to this socket's frames. this.pingHandler = (message: unknown) => { - if (!isPingPongMessage(message)) { - this.logger.warn('Received unexpected ping diagnostics message format'); - return; - } - if (message.websocket !== this.websocket) return; + if (!this.websocket || !isMessageForSocket(message, this.websocket)) return; if (this.options.pingPongLoggingEnabled) { - this.logger.debug(`WebSocket received ping from Slack server (data: ${message.payload?.toString()})`); + this.logger.debug(`WebSocket received ping from Slack server (data: ${message.payload.toString()})`); } this.monitorPingFromSlack(); }; SlackWebSocket.pingChannel.subscribe(this.pingHandler); this.pongHandler = (message: unknown) => { - if (!isPingPongMessage(message)) { - this.logger.warn('Received unexpected pong diagnostics message format'); - return; - } - if (message.websocket !== this.websocket) return; + if (!this.websocket || !isMessageForSocket(message, this.websocket)) return; if (this.options.pingPongLoggingEnabled) { - this.logger.debug(`WebSocket received pong from Slack server (data: ${message.payload?.toString()})`); + this.logger.debug(`WebSocket received pong from Slack server (data: ${message.payload.toString()})`); } this.lastPongReceivedTimestamp = Date.now(); }; From 855b583eb18bed6d15017f3510d6048be4ce6fb8 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Tue, 22 Sep 2026 13:10:33 -0400 Subject: [PATCH 2/3] refactor(socket-mode): use sequential guard clauses in isMessageForSocket Same logic, expressed as early-return if statements instead of one boolean chain. Reads more clearly and matches the guard shape the file used before. Co-Authored-By: Claude --- packages/socket-mode/src/SlackWebSocket.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/socket-mode/src/SlackWebSocket.ts b/packages/socket-mode/src/SlackWebSocket.ts index d175497e6..1f01490f9 100644 --- a/packages/socket-mode/src/SlackWebSocket.ts +++ b/packages/socket-mode/src/SlackWebSocket.ts @@ -20,14 +20,16 @@ interface PingPongMessage { // Match by reference identity, not `instanceof`: the channels are process-global and a different undici copy (e.g. Node's global WebSocket) fails `instanceof` against our import. function isMessageForSocket(message: unknown, websocket: WebSocket): message is PingPongMessage { - return ( - typeof message === 'object' && - message !== null && - 'websocket' in message && - message.websocket === websocket && - 'payload' in message && - Buffer.isBuffer(message.payload) - ); + if (typeof message !== 'object' || message === null) { + return false; + } + if (!('websocket' in message && message.websocket === websocket)) { + return false; + } + if (!('payload' in message && Buffer.isBuffer(message.payload))) { + return false; + } + return true; } export interface SlackWebSocketOptions { From 93c6d407d939626de76df1b74be4b57230b6b3dc Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Tue, 22 Sep 2026 13:10:56 -0400 Subject: [PATCH 3/3] docs(socket-mode): drop the isMessageForSocket comment The function name and guard clauses carry the intent; the comment was redundant. Co-Authored-By: Claude --- packages/socket-mode/src/SlackWebSocket.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/socket-mode/src/SlackWebSocket.ts b/packages/socket-mode/src/SlackWebSocket.ts index 1f01490f9..af6c1d1f7 100644 --- a/packages/socket-mode/src/SlackWebSocket.ts +++ b/packages/socket-mode/src/SlackWebSocket.ts @@ -18,7 +18,6 @@ interface PingPongMessage { payload: Buffer; } -// Match by reference identity, not `instanceof`: the channels are process-global and a different undici copy (e.g. Node's global WebSocket) fails `instanceof` against our import. function isMessageForSocket(message: unknown, websocket: WebSocket): message is PingPongMessage { if (typeof message !== 'object' || message === null) { return false;