Skip to content

Commit 8adcfbf

Browse files
tjenkinsondarrachequesne
authored andcommitted
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: #5135
1 parent 7a23dde commit 8adcfbf

File tree

4 files changed

+93
-8
lines changed

4 files changed

+93
-8
lines changed

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
CookieJar,
1111
createCookieJar,
1212
defaultBinaryType,
13+
nextTick,
1314
} from "./globals.node.js";
1415
import debugModule from "debug"; // debug()
1516

@@ -320,6 +321,11 @@ export class SocketWithoutUpgrade extends Emitter<
320321
private _pingTimeout: number = -1;
321322
private _maxPayload?: number = -1;
322323
private _pingTimeoutTimer: NodeJS.Timer;
324+
/**
325+
* The expiration timestamp of the {@link _pingTimeoutTimer} object is tracked, in case the timer is throttled and the
326+
* callback is not fired on time. This can happen for example when a laptop is suspended or when a phone is locked.
327+
*/
328+
private _pingTimeoutTime = Infinity;
323329
private clearTimeoutFn: typeof clearTimeout;
324330
private readonly _beforeunloadEventListener: () => void;
325331
private readonly _offlineEventListener: () => void;
@@ -630,9 +636,11 @@ export class SocketWithoutUpgrade extends Emitter<
630636
*/
631637
private _resetPingTimeout() {
632638
this.clearTimeoutFn(this._pingTimeoutTimer);
639+
const delay = this._pingInterval + this._pingTimeout;
640+
this._pingTimeoutTime = Date.now() + delay;
633641
this._pingTimeoutTimer = this.setTimeoutFn(() => {
634642
this._onClose("ping timeout");
635-
}, this._pingInterval + this._pingTimeout);
643+
}, delay);
636644
if (this.opts.autoUnref) {
637645
this._pingTimeoutTimer.unref();
638646
}
@@ -710,6 +718,31 @@ export class SocketWithoutUpgrade extends Emitter<
710718
return this.writeBuffer;
711719
}
712720

721+
/**
722+
* Checks whether the heartbeat timer has expired but the socket has not yet been notified.
723+
*
724+
* Note: this method is private for now because it does not really fit the WebSocket API, but if we put it in the
725+
* `write()` method then the message would not be buffered by the Socket.IO client.
726+
*
727+
* @return {boolean}
728+
* @private
729+
*/
730+
/* private */ _hasPingExpired() {
731+
if (!this._pingTimeoutTime) return true;
732+
733+
const hasExpired = Date.now() > this._pingTimeoutTime;
734+
if (hasExpired) {
735+
debug("throttled timer detected, scheduling connection close");
736+
this._pingTimeoutTime = 0;
737+
738+
nextTick(() => {
739+
this._onClose("ping timeout");
740+
}, this.setTimeoutFn);
741+
}
742+
743+
return hasExpired;
744+
}
745+
713746
/**
714747
* Sends a message.
715748
*

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,4 +270,30 @@ describe("Socket", function () {
270270
});
271271
});
272272
});
273+
274+
describe("throttled timer", () => {
275+
it("checks the state of the timer", (done) => {
276+
const socket = new Socket();
277+
278+
expect(socket._hasPingExpired()).to.be(false);
279+
280+
socket.on("open", () => {
281+
expect(socket._hasPingExpired()).to.be(false);
282+
283+
// simulate a throttled timer
284+
socket._pingTimeoutTime = Date.now() - 1;
285+
286+
expect(socket._hasPingExpired()).to.be(true);
287+
288+
// subsequent calls should not trigger more 'close' events
289+
expect(socket._hasPingExpired()).to.be(true);
290+
expect(socket._hasPingExpired()).to.be(true);
291+
});
292+
293+
socket.on("close", (reason) => {
294+
expect(reason).to.eql("ping timeout");
295+
done();
296+
});
297+
});
298+
});
273299
});

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -440,16 +440,13 @@ export class Socket<
440440
packet.id = id;
441441
}
442442

443-
const isTransportWritable =
444-
this.io.engine &&
445-
this.io.engine.transport &&
446-
this.io.engine.transport.writable;
443+
const isTransportWritable = this.io.engine?.transport?.writable;
444+
const isConnected = this.connected && !this.io.engine?._hasPingExpired();
447445

448-
const discardPacket =
449-
this.flags.volatile && (!isTransportWritable || !this.connected);
446+
const discardPacket = this.flags.volatile && !isTransportWritable;
450447
if (discardPacket) {
451448
debug("discard packet as the transport is not currently writable");
452-
} else if (this.connected) {
449+
} else if (isConnected) {
453450
this.notifyOutgoingListeners(packet);
454451
this.packet(packet);
455452
} else {

packages/socket.io-client/test/socket.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,4 +787,33 @@ describe("socket", () => {
787787
});
788788
});
789789
});
790+
791+
describe("throttled timer", () => {
792+
it("should buffer the event and send it upon reconnection", () => {
793+
return wrap((done) => {
794+
let hasReconnected = false;
795+
796+
const socket = io(BASE_URL, {
797+
forceNew: true,
798+
reconnectionDelay: 10,
799+
});
800+
801+
socket.once("connect", () => {
802+
// @ts-expect-error simulate a throttled timer
803+
socket.io.engine._pingTimeoutTime = Date.now() - 1;
804+
805+
socket.emit("echo", "123", (value) => {
806+
expect(hasReconnected).to.be(true);
807+
expect(value).to.eql("123");
808+
809+
success(done, socket);
810+
});
811+
});
812+
813+
socket.io.once("reconnect", () => {
814+
hasReconnected = true;
815+
});
816+
});
817+
});
818+
});
790819
});

0 commit comments

Comments
 (0)