From 16666c3ee0c3eeab33b3c9aef41f51a75e414848 Mon Sep 17 00:00:00 2001 From: ILLISIS Date: Sun, 11 Oct 2026 12:53:13 +1100 Subject: [PATCH 1/2] Handle WebSocket errors before the handshake completes WsServer attached no error listener to a new socket. WsServerConnector adds one, but only after the handshake. ws emits an error for an invalid frame, for example a close frame with the reserved status code 1006, and an error event with no listener throws. That stopped the controller. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01F7obMEi4L4PzUQ8w8q9Uyz --- packages/controller/src/WsServer.ts | 7 +++++++ test/controller/WsServer.js | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/packages/controller/src/WsServer.ts b/packages/controller/src/WsServer.ts index db582ec1..87d3cd72 100644 --- a/packages/controller/src/WsServer.ts +++ b/packages/controller/src/WsServer.ts @@ -116,6 +116,13 @@ export default class WsServer { handleConnection(socket: lib.WebSocketClusterio, req: IncomingMessage) { logger.verbose(`WsServer | new connection from ${this.remoteAddr(req)}`); + // ws emits errors like invalid frames on the socket, and an error with + // no listener crashes the controller. The connector adds its own + // listener, but only once the handshake has completed. + socket.on("error", (err: Error) => { + logger.verbose(`WsServer | error on connection from ${this.remoteAddr(req)}: ${err.message}`); + }); + // Track statistics wsConnectionsCounter.inc(); socket.on("message", (message: WebSocket.RawData) => { diff --git a/test/controller/WsServer.js b/test/controller/WsServer.js index 3b97fe40..c366b1ab 100644 --- a/test/controller/WsServer.js +++ b/test/controller/WsServer.js @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import events from "node:events"; import { Controller, WsServer } from "@clusterio/controller"; import { ControllerConfig } from "@clusterio/lib"; @@ -68,5 +69,23 @@ describe("controller/src/WsServer", function() { assert.equal(ipv6, "2000::1"); }); }); + describe(".handleConnection()", function() { + it("should handle socket errors before the handshake completes", function() { + // A ws socket emits "error" for e.g. a close frame with the reserved code 1006 + const socket = new events.EventEmitter(); + socket.send = () => {}; + socket.close = () => {}; + socket.terminate = () => {}; + const req = { socket: { remoteAddress: "1.2.3.4", remoteFamily: "IPv4" }, headers: {} }; + wsServer.handleConnection(socket, req); + try { + assert.doesNotThrow(() => { + socket.emit("error", new RangeError("Invalid WebSocket frame: invalid status code 1006")); + }); + } finally { + socket.emit("message", "not json"); // ends the handshake and its timeout + } + }); + }); }); }); From f581c70ccd0cd85d36e5e349e8e2beab217bccab Mon Sep 17 00:00:00 2001 From: ILLISIS Date: Tue, 13 Oct 2026 03:19:55 +1100 Subject: [PATCH 2/2] Log a socket error once, from WsServer WsServer.handleConnection now listens for errors on every socket for its whole life, and every socket a WsServerConnector holds comes from there, new sessions and resumed ones alike. The connector's own listener only logged the same error a second time, so it is removed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01F7obMEi4L4PzUQ8w8q9Uyz --- packages/controller/src/WsServerConnector.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/controller/src/WsServerConnector.ts b/packages/controller/src/WsServerConnector.ts index 3b256d1a..3793a651 100644 --- a/packages/controller/src/WsServerConnector.ts +++ b/packages/controller/src/WsServerConnector.ts @@ -144,11 +144,6 @@ export default class WsServerConnector extends lib.WebSocketBaseConnector { } }); - this._socket.on("error", (err: any) => { - // It's assumed that close is always called by ws - logger.verbose("Connector | Error:", err); - }); - this._socket.on("open", () => { logger.verbose("Connector | Open"); });