From 5bc94b56bc1788bab16d9d514d2c8abf3b1d8f87 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Tue, 20 Jun 2023 18:48:11 +0200 Subject: [PATCH] fix: properly report timeout error when connecting In some specific cases (Node.js client with WebSocket only), the reason attached to the "connect_error" event was "websocket error" instead of "timeout". Related: https://github.com/socketio/socket.io/issues/4062 --- lib/manager.ts | 21 +++++++++------------ test/socket.ts | 32 ++++++++++++++++++++++++++++++++ test/support/server.ts | 4 ++++ 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/lib/manager.ts b/lib/manager.ts index fe5c3fa3..8371824b 100644 --- a/lib/manager.ts +++ b/lib/manager.ts @@ -328,35 +328,32 @@ export class Manager< fn && fn(); }); - // emit `error` - const errorSub = on(socket, "error", (err) => { + const onError = (err) => { debug("error"); - self.cleanup(); - self._readyState = "closed"; + this.cleanup(); + this._readyState = "closed"; this.emitReserved("error", err); if (fn) { fn(err); } else { // Only do this if there is no fn to handle the error - self.maybeReconnectOnOpen(); + this.maybeReconnectOnOpen(); } - }); + }; + + // emit `error` + const errorSub = on(socket, "error", onError); if (false !== this._timeout) { const timeout = this._timeout; debug("connect attempt will timeout after %d", timeout); - if (timeout === 0) { - openSubDestroy(); // prevents a race condition with the 'open' event - } - // set timer const timer = this.setTimeoutFn(() => { debug("connect attempt timed out after %d", timeout); openSubDestroy(); + onError(new Error("timeout")); socket.close(); - // @ts-ignore - socket.emit("error", new Error("timeout")); }, timeout); if (this.opts.autoUnref) { diff --git a/test/socket.ts b/test/socket.ts index d427048d..e8bf70b4 100644 --- a/test/socket.ts +++ b/test/socket.ts @@ -70,6 +70,38 @@ describe("socket", () => { }); }); + it("fire a connect_error event on open timeout (polling)", () => { + return wrap((done) => { + const socket = io(BASE_URL, { + forceNew: true, + transports: ["polling"], + timeout: 0, + }); + + socket.on("connect_error", (err) => { + expect(err.message).to.eql("timeout"); + socket.disconnect(); + done(); + }); + }); + }); + + it("fire a connect_error event on open timeout (websocket)", () => { + return wrap((done) => { + const socket = io(BASE_URL, { + forceNew: true, + transports: ["websocket"], + timeout: 0, + }); + + socket.on("connect_error", (err) => { + expect(err.message).to.eql("timeout"); + socket.disconnect(); + done(); + }); + }); + }); + it("doesn't fire a connect_error event when the connection is already established", () => { return wrap((done) => { const socket = io(BASE_URL, { forceNew: true }); diff --git a/test/support/server.ts b/test/support/server.ts index 14b2cfa2..c582a5df 100644 --- a/test/support/server.ts +++ b/test/support/server.ts @@ -5,6 +5,10 @@ export function createServer() { const server = new Server(3210, { pingInterval: 2000, connectionStateRecovery: {}, + allowRequest: (req, callback) => { + // add a fixed delay to test the connection timeout on the client side + setTimeout(() => callback(null, true), 10); + }, }); server.of("/foo").on("connection", (socket) => {