From 0afbb69308da0a3e9913127ce53bd9cbe23d806c Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Tue, 29 Oct 2024 16:50:05 +0000 Subject: [PATCH] Use sync file writes and catch errors to reset state. (#677) By using sync file writes it prevents interrupts between the state change and the server process starting. Also adding error catching to reset the state following an error writing to the files. Fixes: #676 --- CHANGELOG.md | 1 + packages/host/src/server.ts | 177 +++++++++++++++++++++--------------- 2 files changed, 105 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bad60a8..95c8d2ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Many thanks to the following for contributing to this release: - Added `host.factorio_username` and `host.factorio_token` config to set Factorio credentials used on a given host. - Fixed 2.0 version extraction from linux headless downloaded during installation of clusterio. [#671](https://github.com/clusterio/clusterio/pull/671) - Updated display name and description of `controller.external_address` to avoid confusion. [#674](https://github.com/clusterio/clusterio/pull/674) +- Fixed invalid transient state during server start. [#676](https://github.com/clusterio/clusterio/issues/676) - Fixed host user sync to instances after reconnecting to the controller. [#678](https://github.com/clusterio/clusterio/pull/678) ### Breaking Changes diff --git a/packages/host/src/server.ts b/packages/host/src/server.ts index 626dc644..1658232f 100644 --- a/packages/host/src/server.ts +++ b/packages/host/src/server.ts @@ -896,28 +896,36 @@ export class FactorioServer extends events.EventEmitter { this._check(["init"]); this._state = "create"; - await this._writeConfigIni(); - await this._writeMapSettings(mapGenSettings, mapSettings); - this._server = child_process.spawn( - this.binaryPath(), - [ - "--config", this.writePath("config.ini"), - "--create", this.writePath("saves", name), - ...(seed !== undefined ? ["--map-gen-seed", String(seed)] : []), - ...(mapGenSettings !== undefined - ? ["--map-gen-settings", this.writePath("map-gen-settings.json")] : [] - ), - ...(mapSettings !== undefined ? ["--map-settings", this.writePath("map-settings.json")] : []), - ...(this.verboseLogging ? ["--verbose"] : []), - ], - { - detached: true, - stdio: "pipe", - } - ); - - this._attachStdio(); - this._server.once("exit", () => this.emit("exit")); + try { + // Everything between _state and once exit must be sync and call _resetState on error + // If this this thread yields then the server will be left in an invalid transient state + /* eslint-disable node/no-sync */ + this._writeConfigIniSync(); + this._writeMapSettingsSync(mapGenSettings, mapSettings); + this._server = child_process.spawn( + this.binaryPath(), + [ + "--config", this.writePath("config.ini"), + "--create", this.writePath("saves", name), + ...(seed !== undefined ? ["--map-gen-seed", String(seed)] : []), + ...(mapGenSettings !== undefined + ? ["--map-gen-settings", this.writePath("map-gen-settings.json")] : [] + ), + ...(mapSettings !== undefined ? ["--map-settings", this.writePath("map-settings.json")] : []), + ...(this.verboseLogging ? ["--verbose"] : []), + ], + { + detached: true, + stdio: "pipe", + } + ); + this._attachStdio(); + this._server.once("exit", () => this.emit("exit")); + /* eslint-enable node/no-sync */ + } catch (err: any) { + this._resetState(); + throw new lib.EnvironmentError(`Unexpected error:\n${err.stack}`); + } try { let [code, signal] = await events.once(this._server, "exit"); @@ -949,27 +957,36 @@ export class FactorioServer extends events.EventEmitter { this._check(["init"]); this._state = "running"; - await this._writeConfigIni(); - this._server = child_process.spawn( - this.binaryPath(), - [ - "--config", this.writePath("config.ini"), - "--start-server", this.writePath("saves", save), - "--port", String(this.gamePort), - "--rcon-port", String(this.rconPort), - "--rcon-password", this.rconPassword, - ...(this.enableWhitelist ? ["--use-server-whitelist"] : []), - ...(this.enableAuthserverBans ? ["--use-authserver-bans"] : []), - ...(this.verboseLogging ? ["--verbose"] : []), - ], - { - detached: true, - stdio: "pipe", - } - ); + try { + // Everything between _state and _watchExist must be sync and call _resetState on error + // If this this thread yields then the server will be left in an invalid transient state + /* eslint-disable node/no-sync */ + this._writeConfigIniSync(); + this._server = child_process.spawn( + this.binaryPath(), + [ + "--config", this.writePath("config.ini"), + "--start-server", this.writePath("saves", save), + "--port", String(this.gamePort), + "--rcon-port", String(this.rconPort), + "--rcon-password", this.rconPassword, + ...(this.enableWhitelist ? ["--use-server-whitelist"] : []), + ...(this.enableAuthserverBans ? ["--use-authserver-bans"] : []), + ...(this.verboseLogging ? ["--verbose"] : []), + ], + { + detached: true, + stdio: "pipe", + } + ); + this._attachStdio(); + this._watchExit(); + /* eslint-enable node/no-sync */ + } catch (err: any) { + this.emit("error", new lib.EnvironmentError(`Unexpected error:\n${err.stack}`)); + this._resetState(); + } - this._attachStdio(); - this._watchExit(); await this._waitForReady(); } @@ -990,33 +1007,42 @@ export class FactorioServer extends events.EventEmitter { this._check(["init"]); this._state = "running"; - await this._writeConfigIni(); - await this._writeMapSettings(mapGenSettings, mapSettings); - this._server = child_process.spawn( - this.binaryPath(), - [ - "--config", this.writePath("config.ini"), - "--start-server-load-scenario", scenario, - ...(seed !== undefined ? ["--map-gen-seed", String(seed)] : []), - ...(mapGenSettings !== undefined - ? ["--map-gen-settings", this.writePath("map-gen-settings.json")] : [] - ), - ...(mapSettings !== undefined ? ["--map-settings", this.writePath("map-settings.json")] : []), - "--port", String(this.gamePort), - "--rcon-port", String(this.rconPort), - "--rcon-password", this.rconPassword, - ...(this.enableWhitelist ? ["--use-server-whitelist"] : []), - ...(this.enableAuthserverBans ? ["--use-authserver-bans"] : []), - ...(this.verboseLogging ? ["--verbose"] : []), - ], - { - detached: true, - stdio: "pipe", - } - ); + try { + // Everything between _state and _watchExist must be sync and call _resetState on error + // If this this thread yields then the server will be left in an invalid transient state + /* eslint-disable node/no-sync */ + this._writeConfigIniSync(); + this._writeMapSettingsSync(mapGenSettings, mapSettings); + this._server = child_process.spawn( + this.binaryPath(), + [ + "--config", this.writePath("config.ini"), + "--start-server-load-scenario", scenario, + ...(seed !== undefined ? ["--map-gen-seed", String(seed)] : []), + ...(mapGenSettings !== undefined + ? ["--map-gen-settings", this.writePath("map-gen-settings.json")] : [] + ), + ...(mapSettings !== undefined ? ["--map-settings", this.writePath("map-settings.json")] : []), + "--port", String(this.gamePort), + "--rcon-port", String(this.rconPort), + "--rcon-password", this.rconPassword, + ...(this.enableWhitelist ? ["--use-server-whitelist"] : []), + ...(this.enableAuthserverBans ? ["--use-authserver-bans"] : []), + ...(this.verboseLogging ? ["--verbose"] : []), + ], + { + detached: true, + stdio: "pipe", + } + ); + this._attachStdio(); + this._watchExit(); + /* eslint-enable node/no-sync */ + } catch (err: any) { + this.emit("error", new lib.EnvironmentError(`Unexpected error:\n${err.stack}`)); + this._resetState(); + } - this._attachStdio(); - this._watchExit(); await this._waitForReady(); } @@ -1247,24 +1273,29 @@ export class FactorioServer extends events.EventEmitter { return JSON.parse(await fs.readFile(this.dataPath("server-settings.example.json"), "utf-8")); } - async _writeConfigIni() { + _writeConfigIniSync() { let content = lib.stringify({ path: { "read-data": this.dataPath(), "write-data": this.writePath(), }, }); - await lib.safeOutputFile(this.writePath("config.ini"), content); + // Must be sync to allow process spawn without awaiting + // eslint-disable-next-line node/no-sync + fs.writeFileSync(this.writePath("config.ini"), content); } - async _writeMapSettings(mapGenSettings?: object, mapSettings?: object) { + _writeMapSettingsSync(mapGenSettings?: object, mapSettings?: object) { + // Must be sync to allow process spawn without awaiting if (mapGenSettings) { - await lib.safeOutputFile( + // eslint-disable-next-line node/no-sync + fs.writeFileSync( this.writePath("map-gen-settings.json"), JSON.stringify(mapGenSettings, null, "\t") ); } if (mapSettings) { - await lib.safeOutputFile( + // eslint-disable-next-line node/no-sync + fs.writeFileSync( this.writePath("map-settings.json"), JSON.stringify(mapSettings, null, "\t") ); }