Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Check if missed heartbeat synchronously before sending message #5134

Closed
wants to merge 8 commits into from
42 changes: 39 additions & 3 deletions packages/engine.io-client/lib/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
CookieJar,
createCookieJar,
defaultBinaryType,
nextTick,
} from "./globals.node.js";
import debugModule from "debug"; // debug()

Expand Down Expand Up @@ -318,6 +319,8 @@ export class SocketWithoutUpgrade extends Emitter<
private _pingTimeout: number = -1;
private _maxPayload?: number = -1;
private _pingTimeoutTimer: NodeJS.Timer;
private _pingTimeoutTime: number | null = null;
private _scheduledDisconnectFromIsResponsive = false;
private clearTimeoutFn: typeof clearTimeout;
private readonly _beforeunloadEventListener: () => void;
private readonly _offlineEventListener: () => void;
Expand Down Expand Up @@ -628,9 +631,13 @@ export class SocketWithoutUpgrade extends Emitter<
*/
private _resetPingTimeout() {
this.clearTimeoutFn(this._pingTimeoutTimer);
if (this._pingInterval <= 0 || this._pingTimeout <= 0) return;

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();
}
Expand Down Expand Up @@ -708,6 +715,32 @@ export class SocketWithoutUpgrade extends Emitter<
return this.writeBuffer;
}

/**
* Returns `true` if the connection is responding to heartbeats.
*
* If heartbeats are disabled this will always return `true`.
*
* @return {boolean}
*/
public isResponsive() {
darrachequesne marked this conversation as resolved.
Show resolved Hide resolved
if (this.readyState === "closed") return false;
if (this._pingTimeoutTime === null) return true;

const responsive = Date.now() < this._pingTimeoutTime;
if (!responsive && !this._scheduledDisconnectFromIsResponsive) {
debug(
"`isResponsive` detected missed ping so scheduling connection close"
);
this._scheduledDisconnectFromIsResponsive = true;

nextTick(() => {
this._onClose("ping timeout");
}, this.setTimeoutFn);
}

return responsive;
}

/**
* Sends a message.
*
Expand All @@ -717,6 +750,9 @@ export class SocketWithoutUpgrade extends Emitter<
* @return {Socket} for chaining.
*/
public write(msg: RawData, options?: WriteOptions, fn?: () => void) {
// this will schedule a disconnection on next tick if heartbeat missed
this.isResponsive();
darrachequesne marked this conversation as resolved.
Show resolved Hide resolved

this._sendPacket("message", msg, options, fn);
return this;
}
Expand All @@ -730,8 +766,7 @@ export class SocketWithoutUpgrade extends Emitter<
* @return {Socket} for chaining.
*/
public send(msg: RawData, options?: WriteOptions, fn?: () => void) {
this._sendPacket("message", msg, options, fn);
return this;
return this.write(msg, options, fn);
}

/**
Expand Down Expand Up @@ -858,6 +893,7 @@ export class SocketWithoutUpgrade extends Emitter<

// clear timers
this.clearTimeoutFn(this._pingTimeoutTimer);
this._pingTimeoutTime = null;

// stop event from firing again for transport
this.transport.removeAllListeners("close");
Expand Down
11 changes: 11 additions & 0 deletions packages/socket.io-client/lib/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,17 @@ export class Manager<
this._close();
}

/**
* Returns `true` if the connection is responding to heartbeats.
*
* If heartbeats are disabled this will always return `true`.
*
* @return {boolean}
*/
isResponsive() {
return this.engine.isResponsive ? this.engine.isResponsive() : true;
}

/**
* Writes a packet.
*
Expand Down
5 changes: 4 additions & 1 deletion packages/socket.io-client/lib/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,10 @@ export class Socket<
this.flags.volatile && (!isTransportWritable || !this.connected);
if (discardPacket) {
debug("discard packet as the transport is not currently writable");
} else if (this.connected) {
} else if (
this.connected &&
(this.flags.volatile || this.io.isResponsive())
darrachequesne marked this conversation as resolved.
Show resolved Hide resolved
) {
this.notifyOutgoingListeners(packet);
this.packet(packet);
} else {
Expand Down
Loading