From 81d7171c6bb4053c802e3cc4b29a0e42dcf9c065 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Mon, 3 Feb 2020 13:04:58 +0100 Subject: [PATCH] feat: reverse the ping-pong mechanism The ping packets will now be sent by the server, because the timers set in the browsers are not reliable enough. We suspect that a lot of timeout problems came from timers being delayed on the client-side. Breaking change: v3.x clients will not be able to connect anymore (they will send a ping packet and timeout while waiting for a pong packet). Related: https://github.com/socketio/engine.io/issues/312 --- README.md | 4 ++-- lib/socket.js | 55 +++++++++------------------------------------------ 2 files changed, 11 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index ab559f8ca..a0bceef60 100644 --- a/README.md +++ b/README.md @@ -172,9 +172,9 @@ Exposed as `eio` in the browser standalone build. - `upgrade` - Fired upon upgrade success, after the new transport is set - `ping` - - Fired upon _flushing_ a ping packet (ie: actual packet write out) + - Fired upon receiving a ping packet. - `pong` - - Fired upon receiving a pong packet. + - Fired upon _flushing_ a pong packet (ie: actual packet write out) #### Methods diff --git a/lib/socket.js b/lib/socket.js index 02afd606a..561f7b343 100644 --- a/lib/socket.js +++ b/lib/socket.js @@ -116,7 +116,6 @@ class Socket extends Emitter { this.pingTimeout = null; // set on heartbeat - this.pingIntervalTimer = null; this.pingTimeoutTimer = null; this.open(); @@ -415,8 +414,9 @@ class Socket extends Emitter { this.onHandshake(JSON.parse(packet.data)); break; - case "pong": - this.setPing(); + case "ping": + this.resetPingTimeout(); + this.sendPacket("pong"); this.emit("pong"); break; @@ -452,56 +452,19 @@ class Socket extends Emitter { this.onOpen(); // In case open handler closes socket if ("closed" === this.readyState) return; - this.setPing(); - - // Prolong liveness of socket on heartbeat - this.removeListener("heartbeat", this.onHeartbeat); - this.on("heartbeat", this.onHeartbeat); + this.resetPingTimeout(); } /** - * Resets ping timeout. + * Sets and resets ping timeout timer based on server pings. * * @api private */ - onHeartbeat(timeout) { + resetPingTimeout() { clearTimeout(this.pingTimeoutTimer); - const self = this; - self.pingTimeoutTimer = setTimeout(function() { - if ("closed" === self.readyState) return; - self.onClose("ping timeout"); - }, timeout || self.pingInterval + self.pingTimeout); - } - - /** - * Pings server every `this.pingInterval` and expects response - * within `this.pingTimeout` or closes connection. - * - * @api private - */ - setPing() { - const self = this; - clearTimeout(self.pingIntervalTimer); - self.pingIntervalTimer = setTimeout(function() { - debug( - "writing ping packet - expecting pong within %sms", - self.pingTimeout - ); - self.ping(); - self.onHeartbeat(self.pingTimeout); - }, self.pingInterval); - } - - /** - * Sends a ping packet. - * - * @api private - */ - ping() { - const self = this; - this.sendPacket("ping", function() { - self.emit("ping"); - }); + this.pingTimeoutTimer = setTimeout(() => { + this.onClose("ping timeout"); + }, this.pingInterval + this.pingTimeout); } /**