fix(eio,eio-client): do not close the connection while packets are still being received

packages/engine.io-client/lib/socket.ts

@@ -348,6 +348,13 @@ export class SocketWithoutUpgrade extends Emitter<
    * callback is not fired on time. This can happen for example when a laptop is suspended or when a phone is locked.
    */
   private _pingTimeoutTime = Infinity;
+  /**
+   * Timestamp of the last packet received from the server, of any type. Used
+   * by the ping timeout check as a cheap liveness signal, so a server that is
+   * clearly still sending data isn't considered dead just because its next
+   * ping happens to be delayed (e.g. behind a backlog on its own end).
+   */
+  private _lastPacketTime: number = Date.now();
   private clearTimeoutFn: typeof clearTimeout;
   private readonly _beforeunloadEventListener: () => void;
   private readonly _offlineEventListener: () => void;
@@ -603,6 +610,7 @@ export class SocketWithoutUpgrade extends Emitter<
 
       // Socket is live - any packet counts
       this.emitReserved("heartbeat");
+      this._lastPacketTime = Date.now();
 
       switch (packet.type) {
         case "open":
@@ -662,13 +670,45 @@ export class SocketWithoutUpgrade extends Emitter<
     const delay = this._pingInterval + this._pingTimeout;
     this._pingTimeoutTime = Date.now() + delay;
     this._pingTimeoutTimer = this.setTimeoutFn(() => {
-      this._onClose("ping timeout");
+      this._checkPingTimeout(delay);
     }, delay);
     if (this.opts.autoUnref) {
       this._pingTimeoutTimer.unref();
     }
   }
 
+  /**
+   * Called when the ping timeout timer fires. Rather than closing the
+   * connection unconditionally, this checks whether any packet has been
+   * received recently enough that the server is clearly still alive, and if
+   * so, extends the wait instead of giving up - mirroring the same check on
+   * the server side (see `Socket#checkPingTimeout` in the `engine.io`
+   * package), which needed the equivalent client-side change to actually take
+   * effect: without it, a client that stopped hearing new pings (because the
+   * server's own next ping was delayed while it waited on a slow pong) would
+   * disconnect on its own regardless of what the server decided.
+   *
+   * @param delay - the ping timeout duration to apply, in milliseconds
+   * @private
+   */
+  private _checkPingTimeout(delay: number) {
+    const elapsedSinceLastPacket = Date.now() - this._lastPacketTime;
+
+    if (elapsedSinceLastPacket < delay) {
+      const remaining = delay - elapsedSinceLastPacket;
+      this._pingTimeoutTime = Date.now() + remaining;
+      this._pingTimeoutTimer = this.setTimeoutFn(() => {
+        this._checkPingTimeout(delay);
+      }, remaining);
+      if (this.opts.autoUnref) {
+        this._pingTimeoutTimer.unref();
+      }
+      return;
+    }
+
+    this._onClose("ping timeout");
+  }
+
   /**
    * Called on `drain` event
    *
@@ -755,6 +795,14 @@ export class SocketWithoutUpgrade extends Emitter<
 
     const hasExpired = Date.now() > this._pingTimeoutTime;
     if (hasExpired) {
+      const delay = this._pingInterval + this._pingTimeout;
+      const elapsedSinceLastPacket = Date.now() - this._lastPacketTime;
+
+      if (elapsedSinceLastPacket < delay) {
+        this._pingTimeoutTime = Date.now() + (delay - elapsedSinceLastPacket);
+        return false;
+      }
+
       debug("throttled timer detected, scheduling connection close");
       this._pingTimeoutTime = 0;
 

packages/engine.io-client/test/socket.js

@@ -293,8 +293,10 @@ describe("Socket", function () {
       socket.on("open", () => {
         expect(socket._hasPingExpired()).to.be(false);
 
-        // simulate a throttled timer
+        // simulate a throttled timer with no recent packet either
         socket._pingTimeoutTime = Date.now() - 1;
+        socket._lastPacketTime =
+          Date.now() - (socket._pingInterval + socket._pingTimeout) - 1;
 
         expect(socket._hasPingExpired()).to.be(true);
 
@@ -308,5 +310,21 @@ describe("Socket", function () {
         done();
       });
     });
+
+    it("does not close if a packet was recently received", (done) => {
+      const socket = new Socket();
+
+      socket.on("open", () => {
+        // simulate a throttled timer
+        socket._pingTimeoutTime = Date.now() - 1;
+        // simulate a recently received packet
+        socket._lastPacketTime = Date.now();
+
+        expect(socket._hasPingExpired()).to.be(false);
+
+        socket.close();
+        done();
+      });
+    });
   });
 });

packages/engine.io/lib/socket.ts

@@ -58,6 +58,13 @@ export class Socket extends EventEmitter {
   private cleanupFn: any[] = [];
   private pingTimeoutTimer;
   private pingIntervalTimer;
+  /**
+   * Timestamp of the last packet received from the client, of any type. Used by
+   * the ping timeout check as a cheap liveness signal: a client that is clearly
+   * still sending data shouldn't be dropped just because its next expected pong
+   * happens to be delayed behind other traffic.
+   */
+  private lastPacketTime: number = Date.now();
 
   /**
    * This is the session identifier that the client will use in the subsequent HTTP requests. It must not be shared with
@@ -157,6 +164,7 @@ export class Socket extends EventEmitter {
     // export packet event
     debug(`received packet ${packet.type}`);
     this.emit("packet", packet);
+    this.lastPacketTime = Date.now();
 
     switch (packet.type) {
       case "ping":
@@ -227,17 +235,44 @@ export class Socket extends EventEmitter {
    */
   private resetPingTimeout() {
     clearTimeout(this.pingTimeoutTimer);
-    this.pingTimeoutTimer = setTimeout(
-      () => {
-        if (this.readyState === "closed") return;
-        this.onClose("ping timeout");
-      },
+    const timeout =
       this.protocol === 3
         ? this.server.opts.pingInterval + this.server.opts.pingTimeout
-        : this.server.opts.pingTimeout,
+        : this.server.opts.pingTimeout;
+    this.pingTimeoutTimer = setTimeout(
+      () => this.checkPingTimeout(timeout),
+      timeout,
     );
   }
 
+  /**
+   * Called when the ping timeout timer fires. Instead of closing
+   * unconditionally, checks whether any packet (not just the expected pong)
+   * was received recently enough to extend the wait instead.
+   *
+   * Unlike a previous, reverted fix, this doesn't touch any timer on the
+   * common per-packet path (only a timestamp), and never reschedules
+   * `pingIntervalTimer`, so the ping schedule itself is unaffected.
+   *
+   * @param timeout - the ping timeout duration to apply, in milliseconds
+   * @private
+   */
+  private checkPingTimeout(timeout: number) {
+    if (this.readyState === "closed") return;
+
+    const elapsedSinceLastPacket = Date.now() - this.lastPacketTime;
+
+    if (elapsedSinceLastPacket < timeout) {
+      this.pingTimeoutTimer = setTimeout(
+        () => this.checkPingTimeout(timeout),
+        timeout - elapsedSinceLastPacket,
+      );
+      return;
+    }
+
+    this.onClose("ping timeout");
+  }
+
   /**
    * Attaches handlers for the given transport.
    *

packages/engine.io/test/server.js

@@ -970,6 +970,101 @@ describe("server", () => {
       });
     });
 
+    it("should not trigger a ping timeout on the server if other packets are received while waiting for the pong", (done) => {
+      // pingInterval/pingTimeout are kept short and distinct so a bug here
+      // reliably shows up as a premature close within the test's own window,
+      // without needing to wait out multiple full cycles.
+      const opts = { allowUpgrades: false, pingInterval: 50, pingTimeout: 30 };
+      const engine = listen(opts, (port) => {
+        const socket = new ClientSocket(`ws://localhost:${port}`);
+        let serverClosed = false;
+        let messageInterval;
+
+        engine.on("connection", (conn) => {
+          conn.on("close", () => {
+            serverClosed = true;
+          });
+        });
+
+        socket.on("open", () => {
+          // This test is scoped to the server's own decision, so the client's
+          // independent ping timeout watchdog (covered separately below) is
+          // neutralized here - otherwise the client would eventually give up
+          // waiting for a next ping of its own accord (since the server never
+          // gets to send one without a pong to resume its own cycle from),
+          // which would mask whether the server's check is actually correct.
+          socket._resetPingTimeout = () => {};
+
+          // Suppress the client's automatic "ping" -> "pong" reply (simulating
+          // a pong that is delayed or lost behind other traffic), while still
+          // dispatching every other packet type as usual.
+          socket[sendPacketMethod] = (
+            (original) => (type, data, options, fn) => {
+              if (type === "pong") return;
+              return original(type, data, options, fn);
+            }
+          )(socket[sendPacketMethod].bind(socket));
+
+          // Keep sending "message" packets well within pingTimeout (30ms) of
+          // each other, so the connection should look alive to the server
+          // even though no pong ever arrives.
+          messageInterval = setInterval(() => socket.send("still here"), 10);
+        });
+
+        setTimeout(() => {
+          clearInterval(messageInterval);
+          expect(serverClosed).to.be(false);
+          socket.close();
+          engine.httpServer.close();
+          done();
+        }, 250); // several multiples of pingInterval + pingTimeout
+      });
+    });
+
+    it("should not trigger a ping timeout on the client if other packets are received while waiting for the next ping", (done) => {
+      // Symmetric counterpart of the test above: here the *server's* ping
+      // schedule is what's stalled (simulating it never getting a pong to
+      // resume its own cycle from), while both ends keep exchanging ordinary
+      // "message" packets. The client should not give up on its own accord
+      // just because no fresh ping arrives.
+      const opts = { allowUpgrades: false, pingInterval: 50, pingTimeout: 30 };
+      const engine = listen(opts, (port) => {
+        const socket = new ClientSocket(`ws://localhost:${port}`);
+        let clientClosed = false;
+        let clientMessageInterval;
+
+        socket.on("open", () => {
+          clientMessageInterval = setInterval(
+            () => socket.send("client alive"),
+            10,
+          );
+        });
+        socket.on("close", () => {
+          clientClosed = true;
+        });
+
+        engine.on("connection", (conn) => {
+          const originalSendPacket = conn.sendPacket.bind(conn);
+          conn.sendPacket = (type, data, options, fn) => {
+            if (type === "ping") return;
+            return originalSendPacket(type, data, options, fn);
+          };
+          const serverMessageInterval = setInterval(() => {
+            if (conn.readyState === "open") conn.send("still here");
+          }, 10);
+          conn.on("close", () => clearInterval(serverMessageInterval));
+        });
+
+        setTimeout(() => {
+          clearInterval(clientMessageInterval);
+          expect(clientClosed).to.be(false);
+          socket.close();
+          engine.httpServer.close();
+          done();
+        }, 250);
+      });
+    });
+
     it("should trigger when server closes a client", (done) => {
       const engine = listen({ allowUpgrades: false }, (port) => {
         const socket = new ClientSocket(`ws://localhost:${port}`);