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

Author: darrachequesne

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)

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);

package-lock.json

@@ -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,15 +2396,15 @@ 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
       ) {
         eioc("ws://localhost:%d".s(port));
         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();
           });
         });