From 31ff87593f231b86dc47ec5761936439ebd53c20 Mon Sep 17 00:00:00 2001
From: Damien Arrachequesne <damien.arrachequesne@gmail.com>
Date: Mon, 3 Feb 2020 12:52:46 +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         |  2 +-
 lib/socket.js     | 42 +++++++++++++++++++++++++++++++++---------
 package-lock.json |  2 +-
 test/server.js    |  8 ++++----
 4 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/README.md b/README.md
index 7c632064a..073ececa1 100644
--- a/README.md
+++ b/README.md
@@ -321,7 +321,7 @@ A representation of a client. _Inherits from EventEmitter_.
       - `type`: packet type
       - `data`: packet data (if type is message)
 - `packetCreate`
-    - Called before a socket sends a packet (`message`, `pong`)
+    - Called before a socket sends a packet (`message`, `ping`)
     - **Arguments**
       - `type`: packet type
       - `data`: packet data (if type is message)
diff --git a/lib/socket.js b/lib/socket.js
index 97de8b6df..4618b2b80 100644
--- a/lib/socket.js
+++ b/lib/socket.js
@@ -30,6 +30,7 @@ class Socket extends EventEmitter {
     this.checkIntervalTimer = null;
     this.upgradeTimeoutTimer = null;
     this.pingTimeoutTimer = null;
+    this.pingIntervalTimer = null;
 
     this.setTransport(transport);
     this.onOpen();
@@ -60,7 +61,7 @@ class Socket extends EventEmitter {
     }
 
     this.emit("open");
-    this.setPingTimeout();
+    this.schedulePing();
   }
 
   /**
@@ -77,12 +78,12 @@ class Socket extends EventEmitter {
 
       // Reset ping timeout on any packet, incoming data is a good sign of
       // other side's liveness
-      this.setPingTimeout();
+      this.resetPingTimeout(this.server.pingInterval + this.server.pingTimeout);
 
       switch (packet.type) {
-        case "ping":
-          debug("got ping");
-          this.sendPacket("pong");
+        case "pong":
+          debug("got pong");
+          this.schedulePing();
           this.emit("heartbeat");
           break;
 
@@ -112,15 +113,34 @@ class Socket extends EventEmitter {
   }
 
   /**
-   * Sets and resets ping timeout timer based on client pings.
+   * Pings client every `this.pingInterval` and expects response
+   * within `this.pingTimeout` or closes connection.
    *
    * @api private
    */
-  setPingTimeout() {
+  schedulePing() {
+    clearTimeout(this.pingIntervalTimer);
+    this.pingIntervalTimer = setTimeout(() => {
+      debug(
+        "writing ping packet - expecting pong within %sms",
+        this.server.pingTimeout
+      );
+      this.sendPacket("ping");
+      this.resetPingTimeout(this.server.pingTimeout);
+    }, this.server.pingInterval);
+  }
+
+  /**
+   * Resets ping timeout.
+   *
+   * @api private
+   */
+  resetPingTimeout(timeout) {
     clearTimeout(this.pingTimeoutTimer);
     this.pingTimeoutTimer = setTimeout(() => {
+      if (this.readyState === "closed") return;
       this.onClose("ping timeout");
-    }, this.server.pingInterval + this.server.pingTimeout);
+    }, timeout);
   }
 
   /**
@@ -191,7 +211,7 @@ class Socket extends EventEmitter {
         self.clearTransport();
         self.setTransport(transport);
         self.emit("upgrade", transport);
-        self.setPingTimeout();
+        self.schedulePing();
         self.flush();
         if (self.readyState === "closing") {
           transport.close(function() {
@@ -283,7 +303,11 @@ class Socket extends EventEmitter {
   onClose(reason, description) {
     if ("closed" !== this.readyState) {
       this.readyState = "closed";
+
+      // clear timers
+      clearTimeout(this.pingIntervalTimer);
       clearTimeout(this.pingTimeoutTimer);
+
       clearInterval(this.checkIntervalTimer);
       this.checkIntervalTimer = null;
       clearTimeout(this.upgradeTimeoutTimer);
diff --git a/package-lock.json b/package-lock.json
index 1e8b4cf5c..19b8c3b3d 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -512,7 +512,7 @@
       }
     },
     "engine.io-client": {
-      "version": "git+https://github.com/socketio/engine.io-client.git#eb88094a0e85b2bb0114eb0055c60cf12cf298af",
+      "version": "git+https://github.com/socketio/engine.io-client.git#81d7171c6bb4053c802e3cc4b29a0e42dcf9c065",
       "from": "git+https://github.com/socketio/engine.io-client.git#v4",
       "dev": true,
       "requires": {
diff --git a/test/server.js b/test/server.js
index 8d66b30ff..ef778b502 100644
--- a/test/server.js
+++ b/test/server.js
@@ -1011,8 +1011,8 @@ describe("server", function() {
 
           engine.on("connection", function(conn) {
             conn.once("heartbeat", function() {
+              socket.onPacket = function() {};
               setTimeout(function() {
-                socket.onPacket = function() {};
                 expect(clientCloseReason).to.be(null);
               }, 150);
               setTimeout(function() {
@@ -2396,7 +2396,7 @@ describe("server", function() {
       });
     });
 
-    it("should emit when receives ping", function(done) {
+    it("should emit when receives pong", function(done) {
       var engine = listen({ allowUpgrades: false, pingInterval: 4 }, function(
         port
       ) {
@@ -2404,7 +2404,7 @@ describe("server", function() {
         engine.on("connection", function(conn) {
           conn.on("packet", function(packet) {
             conn.close();
-            expect(packet.type).to.be("ping");
+            expect(packet.type).to.be("pong");
             done();
           });
         });
@@ -2435,7 +2435,7 @@ describe("server", function() {
         engine.on("connection", function(conn) {
           conn.on("packetCreate", function(packet) {
             conn.close();
-            expect(packet.type).to.be("pong");
+            expect(packet.type).to.be("ping");
             done();
           });
         });