mirror of
https://github.com/clusterio/clusterio.git
synced 2026-11-12 15:04:52 +09:00
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
073ddf1421
commit
49e8d6ecc5
2 files changed
+40
-21
No files matched your search
+19
-15
@@ -1318,6 +1318,23 @@ export class FactorioServer extends events.EventEmitter<FactorioServerEvents> {
|
||||
}
|
||||
|
||||
|
||||
// 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<void>(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<FactorioServerEvents> {
|
||||
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<FactorioServerEvents> {
|
||||
// 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<void>(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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user