From 8adcfbfde50679095ec2abe376650cf2b6814325 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Wed, 18 Sep 2024 11:11:22 +0200 Subject: [PATCH] fix(sio-client): do not send a packet on an expired connection (#5134) When a laptop is suspended or a phone is locked, the timer that is used to check the liveness of the connection is paused and is not able to detect that the heartbeat has failed. Previously, emitting a message after resuming the page would lose the message. The status of the timer will now be checked before sending the message, so that it gets buffered and sent upon reconnection. Note: we could also have used the Page Visibility API or a custom setTimeout() method based on setInterval(), but this would not be as reliable as the current solution. Reference: https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API Related: https://github.com/socketio/socket.io/issues/5135 --- packages/engine.io-client/lib/socket.ts | 35 +++++++++++++++++++++++- packages/engine.io-client/test/socket.js | 26 ++++++++++++++++++ packages/socket.io-client/lib/socket.ts | 11 +++----- packages/socket.io-client/test/socket.ts | 29 ++++++++++++++++++++ 4 files changed, 93 insertions(+), 8 deletions(-) diff --git a/packages/engine.io-client/lib/socket.ts b/packages/engine.io-client/lib/socket.ts index d663524ba4..dec52864d5 100644 --- a/packages/engine.io-client/lib/socket.ts +++ b/packages/engine.io-client/lib/socket.ts @@ -10,6 +10,7 @@ import { CookieJar, createCookieJar, defaultBinaryType, + nextTick, } from "./globals.node.js"; import debugModule from "debug"; // debug() @@ -320,6 +321,11 @@ export class SocketWithoutUpgrade extends Emitter< private _pingTimeout: number = -1; private _maxPayload?: number = -1; private _pingTimeoutTimer: NodeJS.Timer; + /** + * The expiration timestamp of the {@link _pingTimeoutTimer} object is tracked, in case the timer is throttled and the + * 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; private clearTimeoutFn: typeof clearTimeout; private readonly _beforeunloadEventListener: () => void; private readonly _offlineEventListener: () => void; @@ -630,9 +636,11 @@ export class SocketWithoutUpgrade extends Emitter< */ private _resetPingTimeout() { this.clearTimeoutFn(this._pingTimeoutTimer); + const delay = this._pingInterval + this._pingTimeout; + this._pingTimeoutTime = Date.now() + delay; this._pingTimeoutTimer = this.setTimeoutFn(() => { this._onClose("ping timeout"); - }, this._pingInterval + this._pingTimeout); + }, delay); if (this.opts.autoUnref) { this._pingTimeoutTimer.unref(); } @@ -710,6 +718,31 @@ export class SocketWithoutUpgrade extends Emitter< return this.writeBuffer; } + /** + * Checks whether the heartbeat timer has expired but the socket has not yet been notified. + * + * Note: this method is private for now because it does not really fit the WebSocket API, but if we put it in the + * `write()` method then the message would not be buffered by the Socket.IO client. + * + * @return {boolean} + * @private + */ + /* private */ _hasPingExpired() { + if (!this._pingTimeoutTime) return true; + + const hasExpired = Date.now() > this._pingTimeoutTime; + if (hasExpired) { + debug("throttled timer detected, scheduling connection close"); + this._pingTimeoutTime = 0; + + nextTick(() => { + this._onClose("ping timeout"); + }, this.setTimeoutFn); + } + + return hasExpired; + } + /** * Sends a message. * diff --git a/packages/engine.io-client/test/socket.js b/packages/engine.io-client/test/socket.js index a7037f6e94..9e87e36170 100644 --- a/packages/engine.io-client/test/socket.js +++ b/packages/engine.io-client/test/socket.js @@ -270,4 +270,30 @@ describe("Socket", function () { }); }); }); + + describe("throttled timer", () => { + it("checks the state of the timer", (done) => { + const socket = new Socket(); + + expect(socket._hasPingExpired()).to.be(false); + + socket.on("open", () => { + expect(socket._hasPingExpired()).to.be(false); + + // simulate a throttled timer + socket._pingTimeoutTime = Date.now() - 1; + + expect(socket._hasPingExpired()).to.be(true); + + // subsequent calls should not trigger more 'close' events + expect(socket._hasPingExpired()).to.be(true); + expect(socket._hasPingExpired()).to.be(true); + }); + + socket.on("close", (reason) => { + expect(reason).to.eql("ping timeout"); + done(); + }); + }); + }); }); diff --git a/packages/socket.io-client/lib/socket.ts b/packages/socket.io-client/lib/socket.ts index 1f4255d66c..973bafb32f 100644 --- a/packages/socket.io-client/lib/socket.ts +++ b/packages/socket.io-client/lib/socket.ts @@ -440,16 +440,13 @@ export class Socket< packet.id = id; } - const isTransportWritable = - this.io.engine && - this.io.engine.transport && - this.io.engine.transport.writable; + const isTransportWritable = this.io.engine?.transport?.writable; + const isConnected = this.connected && !this.io.engine?._hasPingExpired(); - const discardPacket = - this.flags.volatile && (!isTransportWritable || !this.connected); + const discardPacket = this.flags.volatile && !isTransportWritable; if (discardPacket) { debug("discard packet as the transport is not currently writable"); - } else if (this.connected) { + } else if (isConnected) { this.notifyOutgoingListeners(packet); this.packet(packet); } else { diff --git a/packages/socket.io-client/test/socket.ts b/packages/socket.io-client/test/socket.ts index 2f067b1ea9..9374453850 100644 --- a/packages/socket.io-client/test/socket.ts +++ b/packages/socket.io-client/test/socket.ts @@ -787,4 +787,33 @@ describe("socket", () => { }); }); }); + + describe("throttled timer", () => { + it("should buffer the event and send it upon reconnection", () => { + return wrap((done) => { + let hasReconnected = false; + + const socket = io(BASE_URL, { + forceNew: true, + reconnectionDelay: 10, + }); + + socket.once("connect", () => { + // @ts-expect-error simulate a throttled timer + socket.io.engine._pingTimeoutTime = Date.now() - 1; + + socket.emit("echo", "123", (value) => { + expect(hasReconnected).to.be(true); + expect(value).to.eql("123"); + + success(done, socket); + }); + }); + + socket.io.once("reconnect", () => { + hasReconnected = true; + }); + }); + }); + }); });