Merge pull request #1064 from ILLISIS/fix-ws-error-before-handshake

Handle WebSocket errors before the handshake completes

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) => {

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");
 		});

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
+				}
+			});
+		});
 	});
 });