fix(socket-mode): route ping/pong diagnostics by identity, not instanceof (#2743)

.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`).

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);
+    });
+  });
 });

packages/socket-mode/src/SlackWebSocket.ts

@@ -18,11 +18,11 @@ interface PingPongMessage {
   payload: Buffer;
 }
 
-function isPingPongMessage(message: unknown): message is PingPongMessage {
+function isMessageForSocket(message: unknown, websocket: WebSocket): message is PingPongMessage {
   if (typeof message !== 'object' || message === null) {
     return false;
   }
-  if (!('websocket' in message && message.websocket instanceof WebSocket)) {
+  if (!('websocket' in message && message.websocket === websocket)) {
     return false;
   }
   if (!('payload' in message && Buffer.isBuffer(message.payload))) {
@@ -194,29 +194,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();
     };