diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index f0a59d0e..64a03bba 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -22,6 +22,11 @@ const scriptCommands = [ "/silent-command", "/sc", ]; +// Statuses the instance passes through on its way to running or stopped. +const transitoryStatuses: ReadonlySet = new Set([ + "starting", "stopping", "creating_save", "exporting_data", +]); + const instanceRconCommandDuration = new lib.Histogram( "clusterio_instance_rcon_command_duration_seconds", "Histogram of the RCON command duration from request to response.", @@ -170,6 +175,7 @@ export default class Instance extends lib.Link { _hadPlayersOnline = false; _playerAutosaveSlot = 1; _expectedUserUpdates: { action: string, name: string, reason: string }[] = []; + _settleWaiters: (() => void)[] = []; constructor( host: Host, @@ -199,13 +205,13 @@ export default class Instance extends lib.Link { if (field === "factorio.shutdown_timeout") { this.server.shutdownTimeoutMs = curr as number * 1000; } else if (field === "factorio.settings") { - this.updateFactorioSettings(curr as any, prev as any).catch(err => { - this.logger.error(`Error updating server settings:\n${err.stack}`); - }).finally(hook); + this._applyWhenRunning( + "server settings", () => this.updateFactorioSettings(curr as any, prev as any) + ); + hook(); } else if (field === "factorio.enable_whitelist") { - this.updateFactorioWhitelist(curr as any).catch(err => { - this.logger.error(`Error updating whitelist:\n${err.stack}`); - }).finally(hook); + this._applyWhenRunning("whitelist", () => this.updateFactorioWhitelist(curr as any)); + hook(); } else { if (field === "factorio.max_concurrent_commands") { this.server.maxConcurrentCommands = curr as number; @@ -556,6 +562,13 @@ end`.replace(/\r?\n/g, " "); notifyStatus(status: Instance["_status"]) { this._status = status; + if (!transitoryStatuses.has(status)) { + const waiters = this._settleWaiters; + this._settleWaiters = []; + for (const resolve of waiters) { + resolve(); + } + } this.sendTo( "controller", new lib.InstanceStatusChangedEvent( @@ -577,6 +590,26 @@ end`.replace(/\r?\n/g, " "); return this._status; } + /** Wait for a transitory status to settle and return the status settled on */ + async _waitForSettledStatus() { + while (transitoryStatuses.has(this._status)) { + await new Promise(resolve => this._settleWaiters.push(resolve)); + } + return this._status; + } + + /** Apply work to the running server, waiting out transitory statuses and skipping it if it stopped */ + async _applyWhenRunning(description: string, fn: () => Promise) { + if (await this._waitForSettledStatus() !== "running") { + return; + } + try { + await fn(); + } catch (err: any) { + this.logger.error(`Error updating ${description}:\n${err.stack}`); + } + } + notifyExit() { this._loadedSave = null; this.notifyStatus("stopped"); @@ -1134,6 +1167,10 @@ end`.replace(/\r?\n/g, " "); } async handleInstanceAdminlistUpdateEvent(request: lib.InstanceAdminlistUpdateEvent) { + if (await this._waitForSettledStatus() !== "running") { + return; + } + const { name, admin } = request; const sync = this.config.get("factorio.sync_adminlist"); if (sync === "disabled") { @@ -1147,6 +1184,10 @@ end`.replace(/\r?\n/g, " "); } async handleInstanceBanlistUpdateEvent(request: lib.InstanceBanlistUpdateEvent) { + if (await this._waitForSettledStatus() !== "running") { + return; + } + const { name, banned, reason } = request; const sync = this.config.get("factorio.sync_banlist"); if (sync === "disabled") { @@ -1160,6 +1201,10 @@ end`.replace(/\r?\n/g, " "); } async handleInstanceWhitelistUpdateEvent(request: lib.InstanceWhitelistUpdateEvent) { + if (await this._waitForSettledStatus() !== "running") { + return; + } + const { name, whitelisted } = request; const sync = this.config.get("factorio.sync_whitelist"); if (this.config.get("factorio.sync_whitelist") === "disabled") { diff --git a/test/host/Instance.js b/test/host/Instance.js index de05589a..6712493d 100644 --- a/test/host/Instance.js +++ b/test/host/Instance.js @@ -49,6 +49,7 @@ describe("class Instance", function() { instance.logger = new MockLogger(); instance.logger.error = message => errors.push(message); instance.server.exampleSettings = async () => ({}); + instance.notifyStatus("running"); hookInvoked = new Promise(resolve => { instance.hooks.instanceConfigFieldChanged.attach("test", field => resolve(field)); }); @@ -86,6 +87,81 @@ describe("class Instance", function() { assert.deepEqual(errors, []); assert.deepEqual(instance.server.rconCommands, ["/config set tags 1 a b"]); }); + + it("should defer changes made while starting until the instance is running", async function() { + instance.notifyStatus("starting"); + instance.config.set("factorio.settings", { tags: ["a"] }); + await wait(10); + assert.deepEqual(instance.server.rconCommands, [], "command sent while starting"); + instance.notifyStatus("running"); + await wait(10); + assert.deepEqual(instance.server.rconCommands, ["/config set tags a"]); + assert.deepEqual(errors, []); + assert.deepEqual(rejections, []); + }); + + it("should drop changes made while starting if the instance stops", async function() { + instance.notifyStatus("starting"); + instance.config.set("factorio.settings", { tags: ["a"] }); + await wait(10); + instance.notifyStatus("stopped"); + await wait(10); + assert.deepEqual(instance.server.rconCommands, []); + assert.deepEqual(errors, []); + assert.deepEqual(rejections, []); + }); + + it("should do nothing for changes made while stopped", async function() { + instance.notifyStatus("stopped"); + instance.config.set("factorio.settings", { tags: ["a"] }); + await wait(10); + assert.deepEqual(instance.server.rconCommands, []); + assert.deepEqual(errors, []); + assert.deepEqual(rejections, []); + }); + + it("should invoke the hook while starting without waiting for the instance", async function() { + instance.notifyStatus("starting"); + instance.config.set("factorio.settings", { tags: ["a"] }); + assert.equal(await hookInvoked, "factorio.settings"); + assert.deepEqual(instance.server.rconCommands, [], "command sent while starting"); + instance.notifyStatus("stopped"); + await wait(10); + }); + }); + + describe("list update events", function() { + let errors; + beforeEach(function() { + errors = []; + instance.logger = new MockLogger(); + instance.logger.error = message => errors.push(message); + }); + + it("should apply an update received while starting once running", async function() { + instance.notifyStatus("starting"); + let handled = instance.handleInstanceAdminlistUpdateEvent( + new lib.InstanceAdminlistUpdateEvent("player", true) + ); + await wait(10); + assert.deepEqual(instance.server.rconCommands, [], "command sent while starting"); + instance.notifyStatus("running"); + await handled; + assert.deepEqual(instance.server.rconCommands, ["/promote player"]); + assert.deepEqual(errors, []); + }); + + it("should drop an update received while starting if the instance stops", async function() { + instance.notifyStatus("starting"); + let handled = instance.handleInstanceAdminlistUpdateEvent( + new lib.InstanceAdminlistUpdateEvent("player", true) + ); + await wait(10); + instance.notifyStatus("stopped"); + await handled; + assert.deepEqual(instance.server.rconCommands, []); + assert.deepEqual(errors, []); + }); }); describe(".checkModPackVersion()", function() {