From 132f8ec918a596eec872aee0c61d4ce63714c400 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Mon, 12 Oct 2020 13:32:28 +0200 Subject: [PATCH] feat: split the events of the Manager and Socket Previously, most of the events emitted by the Manager were also emitted by the Socket instances, but it was somehow misleading for the end users because they don't have the same meaning: - Manager: the state of the low-level connection (with connection and reconnection events) - Socket: the state of the connection to the Namespace (only 'connect', 'disconnect' and 'error') For example, the `reconnect` event: ```js socket.on("reconnect", () => { console.log(socket.connected); // might be false, which is a bit surprising }); ``` Breaking change: the Socket instance will no longer forward the events of its Manager Those events can still be accessed on the Manager instance though: ```js socket.io.on("reconnect", () => { // ... }); ``` --- build/manager.d.ts | 10 +---- build/manager.js | 94 ++++++---------------------------------- lib/manager.ts | 106 +++++++-------------------------------------- test/connection.js | 63 ++++++++------------------- test/socket.js | 6 +-- 5 files changed, 52 insertions(+), 227 deletions(-) diff --git a/build/manager.d.ts b/build/manager.d.ts index e37344573..465697691 100644 --- a/build/manager.d.ts +++ b/build/manager.d.ts @@ -27,12 +27,6 @@ export declare class Manager extends Emitter { * @api public */ constructor(uri: any, opts: any); - /** - * Propagate given event to sockets and emit on `this` - * - * @api private - */ - emitAll(event: string, arg?: any): void; /** * Sets the `reconnection` config. * @@ -87,8 +81,8 @@ export declare class Manager extends Emitter { * @return {Manager} self * @api public */ - open(fn?: any, opts?: any): this; - connect(fn: any, opts: any): this; + open(fn?: any, opts?: any): Manager; + connect(fn: any, opts: any): Manager; /** * Called upon transport open. * diff --git a/build/manager.js b/build/manager.js index 70d1083a2..a3e392470 100644 --- a/build/manager.js +++ b/build/manager.js @@ -76,19 +76,6 @@ class Manager extends component_emitter_1.default { if (this.autoConnect) this.open(); } - /** - * Propagate given event to sockets and emit on `this` - * - * @api private - */ - emitAll(event, arg) { - super.emit(event, arg); - for (let nsp in this.nsps) { - if (has.call(this.nsps, nsp)) { - this.nsps[nsp].emit(event, arg); - } - } - } /** * Sets the `reconnection` config. * @@ -200,11 +187,11 @@ class Manager extends component_emitter_1.default { fn && fn(); }); // emit `connect_error` - const errorSub = on_1.on(socket, "error", function (data) { + const errorSub = on_1.on(socket, "error", (data) => { debug("connect_error"); self.cleanup(); self.readyState = "closed"; - self.emitAll("connect_error", data); + super.emit("connect_error", data); if (fn) { const err = new Error("Connection error"); // err.data = data; @@ -223,12 +210,12 @@ class Manager extends component_emitter_1.default { openSub.destroy(); // prevents a race condition with the 'open' event } // set timer - const timer = setTimeout(function () { + const timer = setTimeout(() => { debug("connect attempt timed out after %d", timeout); openSub.destroy(); socket.close(); socket.emit("error", "timeout"); - self.emitAll("connect_timeout", timeout); + super.emit("connect_timeout", "timeout"); }, timeout); this.subs.push({ destroy: function () { @@ -241,60 +228,7 @@ class Manager extends component_emitter_1.default { return this; } connect(fn, opts) { - debug("readyState %s", this.readyState); - if (~this.readyState.indexOf("open")) - return this; - debug("opening %s", this.uri); - this.engine = engine_io_client_1.default(this.uri, this.opts); - const socket = this.engine; - const self = this; - this.readyState = "opening"; - this.skipReconnect = false; - // emit `open` - const openSub = on_1.on(socket, "open", function () { - self.onopen(); - fn && fn(); - }); - // emit `connect_error` - const errorSub = on_1.on(socket, "error", function (data) { - debug("connect_error"); - self.cleanup(); - self.readyState = "closed"; - self.emitAll("connect_error", data); - if (fn) { - const err = new Error("Connection error"); - // err.data = data; - fn(err); - } - else { - // Only do this if there is no fn to handle the error - self.maybeReconnectOnOpen(); - } - }); - // emit `connect_timeout` - if (false !== this._timeout) { - const timeout = this._timeout; - debug("connect attempt will timeout after %d", timeout); - if (timeout === 0) { - openSub.destroy(); // prevents a race condition with the 'open' event - } - // set timer - const timer = setTimeout(function () { - debug("connect attempt timed out after %d", timeout); - openSub.destroy(); - socket.close(); - socket.emit("error", "timeout"); - self.emitAll("connect_timeout", timeout); - }, timeout); - this.subs.push({ - destroy: function () { - clearTimeout(timer); - }, - }); - } - this.subs.push(openSub); - this.subs.push(errorSub); - return this; + return this.open(fn, opts); } /** * Called upon transport open. @@ -322,7 +256,7 @@ class Manager extends component_emitter_1.default { * @api private */ onping() { - this.emitAll("ping"); + super.emit("ping"); } /** * Called with data. @@ -347,7 +281,7 @@ class Manager extends component_emitter_1.default { */ onerror(err) { debug("error", err); - this.emitAll("error", err); + super.emit("error", err); } /** * Creates a new socket for the given `nsp`. @@ -476,28 +410,28 @@ class Manager extends component_emitter_1.default { if (this.backoff.attempts >= this._reconnectionAttempts) { debug("reconnect failed"); this.backoff.reset(); - this.emitAll("reconnect_failed"); + super.emit("reconnect_failed"); this.reconnecting = false; } else { const delay = this.backoff.duration(); debug("will wait %dms before reconnect attempt", delay); this.reconnecting = true; - const timer = setTimeout(function () { + const timer = setTimeout(() => { if (self.skipReconnect) return; debug("attempting reconnect"); - self.emitAll("reconnect_attempt", self.backoff.attempts); - self.emitAll("reconnecting", self.backoff.attempts); + super.emit("reconnect_attempt", self.backoff.attempts); + super.emit("reconnecting", self.backoff.attempts); // check again for the case socket closed in above events if (self.skipReconnect) return; - self.open(function (err) { + self.open((err) => { if (err) { debug("reconnect attempt error"); self.reconnecting = false; self.reconnect(); - self.emitAll("reconnect_error", err.data); + super.emit("reconnect_error", err.data); } else { debug("reconnect success"); @@ -521,7 +455,7 @@ class Manager extends component_emitter_1.default { const attempt = this.backoff.attempts; this.reconnecting = false; this.backoff.reset(); - this.emitAll("reconnect", attempt); + super.emit("reconnect", attempt); } } exports.Manager = Manager; diff --git a/lib/manager.ts b/lib/manager.ts index 3a42de624..262399745 100644 --- a/lib/manager.ts +++ b/lib/manager.ts @@ -76,20 +76,6 @@ export class Manager extends Emitter { if (this.autoConnect) this.open(); } - /** - * Propagate given event to sockets and emit on `this` - * - * @api private - */ - emitAll(event: string, arg?) { - super.emit(event, arg); - for (let nsp in this.nsps) { - if (has.call(this.nsps, nsp)) { - this.nsps[nsp].emit(event, arg); - } - } - } - /** * Sets the `reconnection` config. * @@ -188,7 +174,7 @@ export class Manager extends Emitter { * @return {Manager} self * @api public */ - open(fn?, opts?) { + open(fn?, opts?): Manager { debug("readyState %s", this.readyState); if (~this.readyState.indexOf("open")) return this; @@ -206,11 +192,11 @@ export class Manager extends Emitter { }); // emit `connect_error` - const errorSub = on(socket, "error", function (data) { + const errorSub = on(socket, "error", (data) => { debug("connect_error"); self.cleanup(); self.readyState = "closed"; - self.emitAll("connect_error", data); + super.emit("connect_error", data); if (fn) { const err = new Error("Connection error"); // err.data = data; @@ -231,12 +217,12 @@ export class Manager extends Emitter { } // set timer - const timer = setTimeout(function () { + const timer = setTimeout(() => { debug("connect attempt timed out after %d", timeout); openSub.destroy(); socket.close(); socket.emit("error", "timeout"); - self.emitAll("connect_timeout", timeout); + super.emit("connect_timeout", "timeout"); }, timeout); this.subs.push({ @@ -252,68 +238,8 @@ export class Manager extends Emitter { return this; } - connect(fn, opts) { - debug("readyState %s", this.readyState); - if (~this.readyState.indexOf("open")) return this; - - debug("opening %s", this.uri); - this.engine = eio(this.uri, this.opts); - const socket = this.engine; - const self = this; - this.readyState = "opening"; - this.skipReconnect = false; - - // emit `open` - const openSub = on(socket, "open", function () { - self.onopen(); - fn && fn(); - }); - - // emit `connect_error` - const errorSub = on(socket, "error", function (data) { - debug("connect_error"); - self.cleanup(); - self.readyState = "closed"; - self.emitAll("connect_error", data); - if (fn) { - const err = new Error("Connection error"); - // err.data = data; - fn(err); - } else { - // Only do this if there is no fn to handle the error - self.maybeReconnectOnOpen(); - } - }); - - // emit `connect_timeout` - if (false !== this._timeout) { - const timeout = this._timeout; - debug("connect attempt will timeout after %d", timeout); - - if (timeout === 0) { - openSub.destroy(); // prevents a race condition with the 'open' event - } - - // set timer - const timer = setTimeout(function () { - debug("connect attempt timed out after %d", timeout); - openSub.destroy(); - socket.close(); - socket.emit("error", "timeout"); - self.emitAll("connect_timeout", timeout); - }, timeout); - - this.subs.push({ - destroy: function () { - clearTimeout(timer); - }, - }); - } - - this.subs.push(openSub); - this.subs.push(errorSub); - - return this; + connect(fn, opts): Manager { + return this.open(fn, opts); } /** @@ -346,7 +272,7 @@ export class Manager extends Emitter { * @api private */ onping() { - this.emitAll("ping"); + super.emit("ping"); } /** @@ -374,7 +300,7 @@ export class Manager extends Emitter { */ onerror(err) { debug("error", err); - this.emitAll("error", err); + super.emit("error", err); } /** @@ -516,29 +442,29 @@ export class Manager extends Emitter { if (this.backoff.attempts >= this._reconnectionAttempts) { debug("reconnect failed"); this.backoff.reset(); - this.emitAll("reconnect_failed"); + super.emit("reconnect_failed"); this.reconnecting = false; } else { const delay = this.backoff.duration(); debug("will wait %dms before reconnect attempt", delay); this.reconnecting = true; - const timer = setTimeout(function () { + const timer = setTimeout(() => { if (self.skipReconnect) return; debug("attempting reconnect"); - self.emitAll("reconnect_attempt", self.backoff.attempts); - self.emitAll("reconnecting", self.backoff.attempts); + super.emit("reconnect_attempt", self.backoff.attempts); + super.emit("reconnecting", self.backoff.attempts); // check again for the case socket closed in above events if (self.skipReconnect) return; - self.open(function (err) { + self.open((err) => { if (err) { debug("reconnect attempt error"); self.reconnecting = false; self.reconnect(); - self.emitAll("reconnect_error", err.data); + super.emit("reconnect_error", err.data); } else { debug("reconnect success"); self.onreconnect(); @@ -563,6 +489,6 @@ export class Manager extends Emitter { const attempt = this.backoff.attempts; this.reconnecting = false; this.backoff.reset(); - this.emitAll("reconnect", attempt); + super.emit("reconnect", attempt); } } diff --git a/test/connection.js b/test/connection.js index f6d29f364..5234a886f 100644 --- a/test/connection.js +++ b/test/connection.js @@ -158,7 +158,7 @@ describe("connection", function () { socket.disconnect(); }) .once("disconnect", () => { - socket.on("reconnect", () => { + socket.io.on("reconnect", () => { socket.disconnect(); done(); }); @@ -177,7 +177,7 @@ describe("connection", function () { reconnectionDelay: 10, }); const socket = manager.socket("/timeout"); - socket.once("reconnect_failed", () => { + manager.once("reconnect_failed", () => { let reconnects = 0; const reconnectCb = () => { reconnects++; @@ -208,10 +208,10 @@ describe("connection", function () { let startTime; let prevDelay = 0; - socket.on("connect_error", () => { + manager.on("connect_error", () => { startTime = new Date().getTime(); }); - socket.on("reconnect_attempt", () => { + manager.on("reconnect_attempt", () => { reconnects++; const currentTime = new Date().getTime(); const delay = currentTime - startTime; @@ -221,7 +221,7 @@ describe("connection", function () { prevDelay = delay; }); - socket.on("reconnect_failed", () => { + manager.on("reconnect_failed", () => { expect(reconnects).to.be(3); expect(increasingDelay).to.be.ok(); socket.close(); @@ -230,27 +230,14 @@ describe("connection", function () { }); }); - it("reconnect event should fire in socket", (done) => { - const socket = io({ forceNew: true }); - - socket.on("reconnect", () => { - socket.disconnect(); - done(); - }); - - setTimeout(() => { - socket.io.engine.close(); - }, 500); - }); - it("should not reconnect when force closed", (done) => { const socket = io("/invalid", { forceNew: true, timeout: 0, reconnectionDelay: 10, }); - socket.on("connect_error", () => { - socket.on("reconnect_attempt", () => { + socket.io.once("connect_error", () => { + socket.io.on("reconnect_attempt", () => { expect().fail(); }); socket.disconnect(); @@ -267,8 +254,8 @@ describe("connection", function () { timeout: 0, reconnectionDelay: 10, }); - socket.once("reconnect_attempt", () => { - socket.on("reconnect_attempt", () => { + socket.io.once("reconnect_attempt", () => { + socket.io.on("reconnect_attempt", () => { expect().fail(); }); socket.disconnect(); @@ -285,8 +272,8 @@ describe("connection", function () { timeout: 0, reconnectionDelay: 10, }); - socket.once("reconnect_attempt", () => { - socket.on("reconnect_attempt", () => { + socket.io.once("reconnect_attempt", () => { + socket.io.on("reconnect_attempt", () => { socket.disconnect(); done(); }); @@ -344,7 +331,7 @@ describe("connection", function () { socket = manager.socket("/timeout"); }); - it("should fire reconnect_* events on socket", (done) => { + it("should fire reconnect_* events on manager", (done) => { const manager = new io.Manager({ reconnection: true, timeout: 0, @@ -359,8 +346,8 @@ describe("connection", function () { expect(attempts).to.be(reconnects); }; - socket.on("reconnect_attempt", reconnectCb); - socket.on("reconnect_failed", () => { + manager.on("reconnect_attempt", reconnectCb); + manager.on("reconnect_failed", () => { expect(reconnects).to.be(2); socket.close(); manager.close(); @@ -368,23 +355,7 @@ describe("connection", function () { }); }); - it("should fire error on socket", (done) => { - const manager = new io.Manager({ reconnection: true }); - const socket = manager.socket("/timeout_socket"); - - socket.on("error", (data) => { - expect(data.code).to.be("test"); - socket.close(); - manager.close(); - done(); - }); - - socket.on("connect", () => { - manager.engine.onPacket({ type: "error", data: "test" }); - }); - }); - - it("should fire reconnecting (on socket) with attempts number when reconnecting twice", (done) => { + it("should fire reconnecting (on manager) with attempts number when reconnecting twice", (done) => { const manager = new io.Manager({ reconnection: true, timeout: 0, @@ -399,8 +370,8 @@ describe("connection", function () { expect(attempts).to.be(reconnects); }; - socket.on("reconnecting", reconnectCb); - socket.on("reconnect_failed", () => { + manager.on("reconnecting", reconnectCb); + manager.on("reconnect_failed", () => { expect(reconnects).to.be(2); socket.close(); manager.close(); diff --git a/test/socket.js b/test/socket.js index 5d523300d..04a734621 100644 --- a/test/socket.js +++ b/test/socket.js @@ -42,7 +42,7 @@ describe("socket", function () { it("doesn't fire a connect_error if we force disconnect in opening state", (done) => { const socket = io({ forceNew: true, timeout: 100 }); socket.disconnect(); - socket.on("connect_error", () => { + socket.io.on("connect_error", () => { throw new Error("Unexpected"); }); setTimeout(() => { @@ -55,11 +55,11 @@ describe("socket", function () { socket.on("connect", () => { const id = socket.id; - socket.on("reconnect_attempt", () => { + socket.io.on("reconnect_attempt", () => { expect(socket.id).to.not.be.ok(); }); - socket.on("reconnect", () => { + socket.io.on("reconnect", () => { expect(socket.id).to.not.eql(id); socket.disconnect(); done();