Skip to content

Commit

Permalink
fix: do not reset the ping timer after upgrade
Browse files Browse the repository at this point in the history
There was two issues with this behavior:

- v3 clients (with allowEIO3: true) were also receiving a "ping" after
a successful upgrade, which is incorrect (in v3, it's the client that
sends the "ping", and the server answers with a "pong")

- the ping timer is not reset after upgrade on the client-side, so an
upgrade which took longer than the `pingTimeout` duration could lead to
a "ping timeout" error on the client-side

I think the latter issue is present since the initial implementation.

Related: socketio/socket.io-client-swift#1309 (comment)
  • Loading branch information
darrachequesne committed Feb 2, 2021
1 parent e5b307c commit ff2b8ab
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
1 change: 0 additions & 1 deletion lib/socket.js
Expand Up @@ -237,7 +237,6 @@ class Socket extends EventEmitter {
self.clearTransport();
self.setTransport(transport);
self.emit("upgrade", transport);
self.schedulePing();
self.flush();
if (self.readyState === "closing") {
transport.close(function() {
Expand Down
18 changes: 18 additions & 0 deletions test/server.js
Expand Up @@ -1282,6 +1282,24 @@ describe("server", () => {
});
});

it("should not timeout after an upgrade", done => {
const opts = { pingInterval: 200, pingTimeout: 20 };
const engine = listen(opts, port => {
const socket = new eioc.Socket("ws://localhost:%d".s(port));
socket.on("open", () => {
setTimeout(() => {
socket.removeListener("close");
engine.close();
socket.close();
done();
}, 500);
});
socket.on("close", () => {
done(new Error("should not happen"));
});
});
});

it("should not crash when messing with Object prototype", done => {
Object.prototype.foo = "bar"; // eslint-disable-line no-extend-native
const engine = listen({ allowUpgrades: true }, port => {
Expand Down

0 comments on commit ff2b8ab

Please sign in to comment.