Skip to content

Commit

Permalink
fix: properly report timeout error when connecting
Browse files Browse the repository at this point in the history
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: socketio/socket.io#4062
  • Loading branch information
darrachequesne committed Jun 20, 2023
1 parent 781d753 commit 5bc94b5
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 12 deletions.
21 changes: 9 additions & 12 deletions lib/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
32 changes: 32 additions & 0 deletions test/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
4 changes: 4 additions & 0 deletions test/support/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down

0 comments on commit 5bc94b5

Please sign in to comment.