From 99c2cb8421361487ed7c876edd8670bb69a5c5b5 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Thu, 18 Nov 2021 11:39:14 +0100 Subject: [PATCH] fix: fix `socket.disconnect().connect()` usage Previously, calling `socket.disconnect().connect()` could, if the connection was upgraded to WebSocket, result in "disconnect" being emitted twice, and an engine being leaked. Here's what happened: > socket.disconnect() - calls `socket.destroy()` so the socket doesn't listen to the manager events anymore - then calls `manager._close()` which closes the underlying engine but not the manager itself (it waits for the "close" event of the engine) > socket.connect() - calls `socket.subEvents()` so the socket does listen to the manager events - calls `manager.open()` which creates a new engine And then the first engine emits a "close" event, which is forwarded to the socket, hence the second "disconnect" event. Related: https://github.com/socketio/socket.io-client/issues/1014 --- lib/manager.ts | 10 ++-------- test/socket.ts | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/manager.ts b/lib/manager.ts index e42378bf..09bf5efd 100644 --- a/lib/manager.ts +++ b/lib/manager.ts @@ -520,13 +520,7 @@ export class Manager< debug("disconnect"); this.skipReconnect = true; this._reconnecting = false; - if ("opening" === this._readyState) { - // `onclose` will not fire because - // an open event never happened - this.cleanup(); - } - this.backoff.reset(); - this._readyState = "closed"; + this.onclose("forced close"); if (this.engine) this.engine.close(); } @@ -545,7 +539,7 @@ export class Manager< * @private */ private onclose(reason: string): void { - debug("onclose"); + debug("closed due to %s", reason); this.cleanup(); this.backoff.reset(); diff --git a/test/socket.ts b/test/socket.ts index 47a65a7f..0c1f0031 100644 --- a/test/socket.ts +++ b/test/socket.ts @@ -231,6 +231,25 @@ describe("socket", function () { }, 100); }); + it("should properly disconnect then reconnect", (done) => { + const socket = io("/", { forceNew: true, transports: ["websocket"] }); + + let count = 0; + + socket.once("connect", () => { + socket.disconnect().connect(); + }); + + socket.on("disconnect", () => { + count++; + }); + + setTimeout(() => { + expect(count).to.eql(1); + success(done, socket); + }, 200); + }); + it("should throw on reserved event", () => { const socket = io("/no", { forceNew: true });