Skip to content

Commit

Permalink
feat: reverse the ping-pong mechanism
Browse files Browse the repository at this point in the history
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: socketio/engine.io#312
  • Loading branch information
darrachequesne committed Feb 4, 2020
1 parent 581ceff commit 81d7171
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 48 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
55 changes: 9 additions & 46 deletions lib/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ class Socket extends Emitter {
this.pingTimeout = null;

// set on heartbeat
this.pingIntervalTimer = null;
this.pingTimeoutTimer = null;

this.open();
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}

/**
Expand Down

0 comments on commit 81d7171

Please sign in to comment.