From 073ddf142153a56c753a804ef08ce9deb8eb8b6a Mon Sep 17 00:00:00 2001 From: bbassie Date: Sun, 13 Sep 2026 01:52:22 +0000 Subject: [PATCH 1/4] Fix stop hanging if Factorio exits before RCON connects stop() waits for the rcon-ready event when RCON has not connected yet, but nothing emits it once the process is gone, so the promise never settles and the stop request stays pending until the session closes. Wait for exit as well and drop the other listener when either fires. Repro: send InstanceExportDataRequest to a stopped instance, then InstanceStopRequest while the status is exporting_data. Instance.stop() sees the server as running and calls server.stop(), then exportData kills the process before RCON is up. The export finishes and the instance reports stopped, but the stop request never gets a response. Co-Authored-By: Claude Opus 5 (1M context) --- packages/host/src/server.ts | 14 ++++++++- test/host/server_stop.js | 62 +++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 test/host/server_stop.js diff --git a/packages/host/src/server.ts b/packages/host/src/server.ts index 32d65b52..e117c4de 100644 --- a/packages/host/src/server.ts +++ b/packages/host/src/server.ts @@ -1405,7 +1405,19 @@ export class FactorioServer extends events.EventEmitter { // complete before the RCON connection can be used if (!this._rconReady) { // Not using events.once here to avoid throwing on error events. - await new Promise(resolve => this.once("rcon-ready", resolve)); + // The process may also exit before RCON connects. + await new Promise(resolve => { + const onRconReady = () => { + this.off("exit", onExit); + resolve(); + }; + const onExit = () => { + this.off("rcon-ready", onRconReady); + resolve(); + }; + this.once("rcon-ready", onRconReady); + this.once("exit", onExit); + }); } // The Factorio server may have decided to get ahead of us and diff --git a/test/host/server_stop.js b/test/host/server_stop.js new file mode 100644 index 00000000..abff051c --- /dev/null +++ b/test/host/server_stop.js @@ -0,0 +1,62 @@ +import assert from "node:assert/strict"; +import events from "node:events"; +import path from "node:path"; + +import { FactorioServer } from "@clusterio/host/dist/node/src/server.js"; + + +// Stand-in for the Factorio child process, only exit and kill are used here. +class FakeProcess extends events.EventEmitter { + killed = false; + kill() { + this.killed = true; + } +} + +function createStartedServer() { + const server = new FactorioServer(path.join("test", "file", "factorio"), path.join("temp", "test", "server"), {}); + server._state = "running"; + server._rconReady = false; + server._server = new FakeProcess(); + server._watchExit(); + return server; +} + +// Fail instead of hanging if stop() never resolves. +async function waitForStop(stopped) { + let timeoutId; + const timeout = new Promise((resolve, reject) => { + timeoutId = setTimeout(() => reject(new Error("stop() did not resolve")), 500); + }); + try { + await Promise.race([stopped, timeout]); + } finally { + clearTimeout(timeoutId); + } +} + +describe("host/src/server", function() { + describe("FactorioServer.stop()", function() { + it("returns if the process exits before RCON is ready", async function() { + const server = createStartedServer(); + const stopped = server.stop(); + server._server.emit("exit", 1, null); + await waitForStop(stopped); + assert.equal(server._state, "init"); + assert.equal(server.listenerCount("rcon-ready"), 0); + }); + + it("stops the server if RCON becomes ready", async function() { + const server = createStartedServer(); + const stopped = server.stop(); + server._rconReady = true; + server.emit("rcon-ready"); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(server.listenerCount("exit"), 0); + const fakeProcess = server._server; + assert.equal(fakeProcess.killed, true); + fakeProcess.emit("exit", 0, null); + await waitForStop(stopped); + }); + }); +}); From 49e8d6ecc513d66784ae3389b0284bf66e91d243 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sun, 13 Sep 2026 13:49:58 +0000 Subject: [PATCH 2/4] Fix sendRcon hanging if Factorio exits before RCON connects sendRcon waits for the rcon-ready event when RCON has not connected yet, which is the same root cause as the stop() hang. events.once rejects on error events, but _watchExit skips emitting error while the state is stopping, and kill() sets that state before sending SIGKILL. exportData does exactly that, so an RCON call racing the export never settles and the request stays pending until the session closes. Wait for exit as well, which leaves the existing RCON client check to reject the call with "RCON connection lost" instead. Both call sites now share a helper as the paired wait is identical. Co-Authored-By: Claude Opus 5 (1M context) --- packages/host/src/server.ts | 34 +++++++++++++++++++--------------- test/host/server_stop.js | 27 +++++++++++++++++++++------ 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/packages/host/src/server.ts b/packages/host/src/server.ts index e117c4de..1fc01afe 100644 --- a/packages/host/src/server.ts +++ b/packages/host/src/server.ts @@ -1318,6 +1318,23 @@ export class FactorioServer extends events.EventEmitter { } + // Not using events.once here to avoid throwing on error events, which are + // not emitted when the process exits while stopping. + async _waitForRconReadyOrExit() { + await new Promise(resolve => { + const onRconReady = () => { + this.off("exit", onExit); + resolve(); + }; + const onExit = () => { + this.off("rcon-ready", onRconReady); + resolve(); + }; + this.once("rcon-ready", onRconReady); + this.once("exit", onExit); + }); + } + /** * Send message over RCON * @@ -1333,7 +1350,7 @@ export class FactorioServer extends events.EventEmitter { async sendRcon(message: string, expectEmpty?: boolean) { this._check(["running", "stopping"]); if (!this._rconReady) { - await events.once(this, "rcon-ready"); + await this._waitForRconReadyOrExit(); } if (!this._rconClient) { throw new Error("RCON connection lost"); @@ -1404,20 +1421,7 @@ export class FactorioServer extends events.EventEmitter { // If RCON is not yet fully connected that operation needs to // complete before the RCON connection can be used if (!this._rconReady) { - // Not using events.once here to avoid throwing on error events. - // The process may also exit before RCON connects. - await new Promise(resolve => { - const onRconReady = () => { - this.off("exit", onExit); - resolve(); - }; - const onExit = () => { - this.off("rcon-ready", onRconReady); - resolve(); - }; - this.once("rcon-ready", onRconReady); - this.once("exit", onExit); - }); + await this._waitForRconReadyOrExit(); } // The Factorio server may have decided to get ahead of us and diff --git a/test/host/server_stop.js b/test/host/server_stop.js index abff051c..c98aff21 100644 --- a/test/host/server_stop.js +++ b/test/host/server_stop.js @@ -22,14 +22,14 @@ function createStartedServer() { return server; } -// Fail instead of hanging if stop() never resolves. -async function waitForStop(stopped) { +// Fail instead of hanging if the call never settles. +async function withTimeout(promise, message) { let timeoutId; const timeout = new Promise((resolve, reject) => { - timeoutId = setTimeout(() => reject(new Error("stop() did not resolve")), 500); + timeoutId = setTimeout(() => reject(new Error(message)), 500); }); try { - await Promise.race([stopped, timeout]); + return await Promise.race([promise, timeout]); } finally { clearTimeout(timeoutId); } @@ -41,7 +41,7 @@ describe("host/src/server", function() { const server = createStartedServer(); const stopped = server.stop(); server._server.emit("exit", 1, null); - await waitForStop(stopped); + await withTimeout(stopped, "stop() did not resolve"); assert.equal(server._state, "init"); assert.equal(server.listenerCount("rcon-ready"), 0); }); @@ -56,7 +56,22 @@ describe("host/src/server", function() { const fakeProcess = server._server; assert.equal(fakeProcess.killed, true); fakeProcess.emit("exit", 0, null); - await waitForStop(stopped); + await withTimeout(stopped, "stop() did not resolve"); + }); + }); + + describe("FactorioServer.sendRcon()", function() { + it("throws if the process exits before RCON is ready", async function() { + const server = createStartedServer(); + const sent = server.sendRcon("/version"); + // kill() sets the state before killing, which suppresses the error event. + server._state = "stopping"; + server._server.emit("exit", null, "SIGKILL"); + await assert.rejects( + withTimeout(sent, "sendRcon() did not settle"), + new Error("RCON connection lost"), + ); + assert.equal(server.listenerCount("rcon-ready"), 0); }); }); }); From 229bbf8eb8c2e8286436634e8bdc04d9d9bd51fe Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sun, 13 Sep 2026 17:09:51 +0000 Subject: [PATCH 3/4] Report why the server exited from sendRcon Waiting for exit alongside rcon-ready made sendRcon reject with the flat "RCON connection lost" for every case, including the unexpected exits that used to reject with the EnvironmentError saying why the server went away, as events.once rejects on error events. Have the paired wait capture the error emitted with the exit and hand it back, so sendRcon throws that when there is one and falls through to the old message when there is not, which is the kill path where no error is emitted. stop() ignores the error as its job is done once the process is gone. Co-Authored-By: Claude Opus 5 (1M context) --- packages/host/src/server.ts | 32 ++++++++++++++++++++++++-------- test/host/server_stop.js | 32 +++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/packages/host/src/server.ts b/packages/host/src/server.ts index 1fc01afe..b7e426be 100644 --- a/packages/host/src/server.ts +++ b/packages/host/src/server.ts @@ -1318,18 +1318,29 @@ export class FactorioServer extends events.EventEmitter { } - // Not using events.once here to avoid throwing on error events, which are - // not emitted when the process exits while stopping. + // Not using events.once here to avoid throwing on error events, as they are + // not emitted when the process exits while stopping. Returns the error + // emitted with the exit, if there was one, for the caller to report. async _waitForRconReadyOrExit() { - await new Promise(resolve => { - const onRconReady = () => { + return await new Promise(resolve => { + let exitError: Error | undefined; + const cleanup = () => { + this.off("error", onError); + this.off("rcon-ready", onRconReady); this.off("exit", onExit); - resolve(); + }; + const onError = (err: Error) => { + exitError = err; + }; + const onRconReady = () => { + cleanup(); + resolve(undefined); }; const onExit = () => { - this.off("rcon-ready", onRconReady); - resolve(); + cleanup(); + resolve(exitError); }; + this.on("error", onError); this.once("rcon-ready", onRconReady); this.once("exit", onExit); }); @@ -1350,7 +1361,11 @@ export class FactorioServer extends events.EventEmitter { async sendRcon(message: string, expectEmpty?: boolean) { this._check(["running", "stopping"]); if (!this._rconReady) { - await this._waitForRconReadyOrExit(); + // Prefer the reason the server gave for exiting over the flat message below. + const exitError = await this._waitForRconReadyOrExit(); + if (exitError) { + throw exitError; + } } if (!this._rconClient) { throw new Error("RCON connection lost"); @@ -1421,6 +1436,7 @@ export class FactorioServer extends events.EventEmitter { // If RCON is not yet fully connected that operation needs to // complete before the RCON connection can be used if (!this._rconReady) { + // An exit error is not reported here as stopping is done once the process is gone. await this._waitForRconReadyOrExit(); } diff --git a/test/host/server_stop.js b/test/host/server_stop.js index c98aff21..26f52936 100644 --- a/test/host/server_stop.js +++ b/test/host/server_stop.js @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import events from "node:events"; import path from "node:path"; +import * as lib from "@clusterio/lib"; import { FactorioServer } from "@clusterio/host/dist/node/src/server.js"; @@ -19,6 +20,8 @@ function createStartedServer() { server._rconReady = false; server._server = new FakeProcess(); server._watchExit(); + // Instance keeps an error listener on the server for as long as it runs. + server.on("error", () => {}); return server; } @@ -44,6 +47,18 @@ describe("host/src/server", function() { await withTimeout(stopped, "stop() did not resolve"); assert.equal(server._state, "init"); assert.equal(server.listenerCount("rcon-ready"), 0); + assert.equal(server.listenerCount("error"), 1); + }); + + it("returns if the process fails before RCON is ready", async function() { + const server = createStartedServer(); + const stopped = server.stop(); + // Failing to spawn emits an error event followed by exit. + server._server.emit("error", Object.assign(new Error("spawn failed"), { code: "EACCES" })); + await withTimeout(stopped, "stop() did not resolve"); + assert.equal(server._state, "init"); + assert.equal(server.listenerCount("rcon-ready"), 0); + assert.equal(server.listenerCount("error"), 1); }); it("stops the server if RCON becomes ready", async function() { @@ -61,7 +76,7 @@ describe("host/src/server", function() { }); describe("FactorioServer.sendRcon()", function() { - it("throws if the process exits before RCON is ready", async function() { + it("throws if the process is killed before RCON is ready", async function() { const server = createStartedServer(); const sent = server.sendRcon("/version"); // kill() sets the state before killing, which suppresses the error event. @@ -72,6 +87,21 @@ describe("host/src/server", function() { new Error("RCON connection lost"), ); assert.equal(server.listenerCount("rcon-ready"), 0); + assert.equal(server.listenerCount("error"), 1); + }); + + it("throws the exit error if the process exits before RCON is ready", async function() { + const server = createStartedServer(); + const sent = server.sendRcon("/version"); + // An unexpected exit emits why the server went away before the exit event. + server._server.emit("exit", 1, null); + await assert.rejects( + withTimeout(sent, "sendRcon() did not settle"), + new lib.EnvironmentError("Factorio server unexpectedly shut down with code 1"), + ); + assert.equal(server.listenerCount("rcon-ready"), 0); + assert.equal(server.listenerCount("exit"), 0); + assert.equal(server.listenerCount("error"), 1); }); }); }); From 2a967e84c556cc7746e8d4a0810d3c4fe031873a Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sun, 13 Sep 2026 17:29:41 +0000 Subject: [PATCH 4/4] Reject the RCON wait instead of resolving with an error _waitForRconReadyOrExit resolved with an Error to signal the process exited before RCON came up, which is an odd shape for a promise. It now resolves when RCON is ready and rejects otherwise, with the error emitted with the exit, or "RCON connection lost" when there was none. Observable behaviour is unchanged. sendRcon rejects with the same messages as before and stop ignores the rejection, as stopping is done once the process is gone. Co-Authored-By: Claude Opus 5 (1M context) --- packages/host/src/server.ts | 19 ++++++++----------- test/host/server_stop.js | 13 +++++++++++++ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/packages/host/src/server.ts b/packages/host/src/server.ts index b7e426be..0057446d 100644 --- a/packages/host/src/server.ts +++ b/packages/host/src/server.ts @@ -1318,11 +1318,11 @@ export class FactorioServer extends events.EventEmitter { } - // Not using events.once here to avoid throwing on error events, as they are - // not emitted when the process exits while stopping. Returns the error + // Not using events.once here as it rejects on error events, which are not + // emitted when the process exits while stopping. Rejects with the error // emitted with the exit, if there was one, for the caller to report. async _waitForRconReadyOrExit() { - return await new Promise(resolve => { + return await new Promise((resolve, reject) => { let exitError: Error | undefined; const cleanup = () => { this.off("error", onError); @@ -1334,11 +1334,11 @@ export class FactorioServer extends events.EventEmitter { }; const onRconReady = () => { cleanup(); - resolve(undefined); + resolve(); }; const onExit = () => { cleanup(); - resolve(exitError); + reject(exitError ?? new Error("RCON connection lost")); }; this.on("error", onError); this.once("rcon-ready", onRconReady); @@ -1361,11 +1361,8 @@ export class FactorioServer extends events.EventEmitter { async sendRcon(message: string, expectEmpty?: boolean) { this._check(["running", "stopping"]); if (!this._rconReady) { - // Prefer the reason the server gave for exiting over the flat message below. - const exitError = await this._waitForRconReadyOrExit(); - if (exitError) { - throw exitError; - } + // Rejects with the reason the server gave for exiting, if there was one. + await this._waitForRconReadyOrExit(); } if (!this._rconClient) { throw new Error("RCON connection lost"); @@ -1437,7 +1434,7 @@ export class FactorioServer extends events.EventEmitter { // complete before the RCON connection can be used if (!this._rconReady) { // An exit error is not reported here as stopping is done once the process is gone. - await this._waitForRconReadyOrExit(); + await this._waitForRconReadyOrExit().catch(() => {}); } // The Factorio server may have decided to get ahead of us and diff --git a/test/host/server_stop.js b/test/host/server_stop.js index 26f52936..8cae2ac4 100644 --- a/test/host/server_stop.js +++ b/test/host/server_stop.js @@ -61,6 +61,19 @@ describe("host/src/server", function() { assert.equal(server.listenerCount("error"), 1); }); + it("does not reject if the process exits with an error before RCON is ready", async function() { + const server = createStartedServer(); + const stopped = server.stop(); + // The wait rejects with this error, stopping succeeds regardless. + server.emit("error", new lib.EnvironmentError("Factorio server was killed")); + server._server.emit("exit", null, "SIGKILL"); + await assert.doesNotReject(withTimeout(stopped, "stop() did not resolve")); + assert.equal(server._state, "init"); + assert.equal(server.listenerCount("rcon-ready"), 0); + assert.equal(server.listenerCount("exit"), 0); + assert.equal(server.listenerCount("error"), 1); + }); + it("stops the server if RCON becomes ready", async function() { const server = createStartedServer(); const stopped = server.stop();