From 0617552956f26c3be9d445dd185ce4e817dcf3f9 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 9 Mar 2026 13:54:08 +0100 Subject: [PATCH 01/14] Fix circular import breaking --dev mode A circular import caused code to attempt to use symbols before they were defined. Due to the --dev mode on the controller running with different chunck boundaries this only appeared in the --dev mode. --- packages/lib/src/external/FactorioVersions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/lib/src/external/FactorioVersions.ts b/packages/lib/src/external/FactorioVersions.ts index 45125433..0fe9b8c6 100644 --- a/packages/lib/src/external/FactorioVersions.ts +++ b/packages/lib/src/external/FactorioVersions.ts @@ -1,5 +1,5 @@ import { Static, Type } from "@sinclair/typebox"; -import { FullVersionSchema, integerFullVersion, isFullVersion } from "../data"; +import { FullVersionSchema, integerFullVersion, isFullVersion } from "../data/version"; const ARCHIVE_URL = "https://factorio.com/download/archive"; From f56605600fb4d29157297ff06d33136060c64b17 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 9 Mar 2026 14:12:40 +0100 Subject: [PATCH 02/14] Fix host bricked on Factorio version check If the FactorioVersionRequest throws the instance and host ends up bricked as the instance is left in an invalid starting state that can't be stopped. --- packages/host/src/Host.ts | 1 + packages/host/src/Instance.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts index 7e778c32..b096a889 100644 --- a/packages/host/src/Host.ts +++ b/packages/host/src/Host.ts @@ -74,6 +74,7 @@ async function discoverInstances(instancesDir: string) { return instances; } +// Requests that wake up the instance is responsible for updating the instance state in case of errors. const instanceStartingMessages = new Set([ lib.InstanceStartRequest.name, lib.InstanceLoadScenarioRequest.name, diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index 952470be..782b26a2 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -637,9 +637,10 @@ end`.replace(/\r?\n/g, " "); } async init(pluginInfos: lib.PluginNodeEnvInfo[]) { + // After changing the status this function is responsible for calling notifyExit if it throws. this.notifyStatus("starting"); - const factorioVersions = await this.sendTo("controller", new lib.FactorioVersionsRequest()); try { + const factorioVersions = await this.sendTo("controller", new lib.FactorioVersionsRequest()); await this._loadStats(); await this.server.checkForUpdates(factorioVersions); await this.server.init(); From 25d98d2ab355e0e17ca7b28c6cc64a19476d5096 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 9 Mar 2026 14:54:19 +0100 Subject: [PATCH 03/14] Fix error starting instance on unlisted versions The FactorioServer.checkForUpdates function would wrongfully assume that the version running would always be listed on Wube's archive page, and throw an error if this was not the case. Ignore version checking logic if no suitable latest version could be found. --- packages/host/src/server.ts | 4 + test/host/server.js | 370 +++++++-------------- test/lib/external/FactorioVersions.test.js | 6 + 3 files changed, 124 insertions(+), 256 deletions(-) diff --git a/packages/host/src/server.ts b/packages/host/src/server.ts index 6152fee6..c51538d1 100644 --- a/packages/host/src/server.ts +++ b/packages/host/src/server.ts @@ -978,6 +978,10 @@ export class FactorioServer extends events.EventEmitter { .sort((v1, v2) => versionOrder(v1.version, v2.version)) [0]; + if (!latestVersion) { + return; + } + const installedVersions = await listFactorioVersions(this._factorioDir); if (!installedVersions.versions.has(latestVersion.version)) { const v = latestVersion.version; diff --git a/test/host/server.js b/test/host/server.js index 28d21054..70f9cff8 100644 --- a/test/host/server.js +++ b/test/host/server.js @@ -342,264 +342,122 @@ describe("host/server", function() { }); }); - describe("full version", function() { - it("should do nothing when there is no newer version", async function() { - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = "0.1.1"; - await server.checkForUpdates([{ - stable: true, - version: "0.1.1", - headlessUrl: "test1", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test2", - }]); + for (const [suite, target] of [ + ["full version", "0.1.1"], + ["partial version", "0.1"], + ["latest version", "latest"], + ]) { + /* eslint-disable no-loop-func */ + describe("full version", function() { + it("should do nothing when there are no versions", async function() { + server._factorioDir = path.join("test", "file", "factorio"); + server._targetVersion = target; + await server.checkForUpdates([]); - assert.equal(fetchCalledWith, null); + assert.equal(fetchCalledWith, null); + }); + it("should do nothing when no version matches", async function() { + server._factorioDir = path.join("test", "file", "factorio"); + server._targetVersion = target; + await server.checkForUpdates([{ + stable: true, + version: "0.2.1", + headlessUrl: "test1", + }, { + stable: false, + version: "0.2.0", + headlessUrl: "test2", + }]); + + assert.equal(fetchCalledWith, null); + }); + it("should do nothing when there is no newer version", async function() { + server._factorioDir = path.join("test", "file", "factorio"); + server._targetVersion = target; + await server.checkForUpdates([{ + stable: true, + version: "0.1.1", + headlessUrl: "test1", + }, { + stable: false, + version: "0.1.0", + headlessUrl: "test2", + }]); + + assert.equal(fetchCalledWith, null); + }); + it("should do nothing for direct installs", async function() { + server._factorioDir = path.join("test", "file", "factorio", "0.1.1"); + server._targetVersion = target === "0.1.1" ? "0.1.5" : target; + await server.checkForUpdates([{ + stable: true, + version: "0.1.5", + headlessUrl: "test1", + }, { + stable: true, + version: "0.1.1", + headlessUrl: "test1", + }, { + stable: false, + version: "0.1.0", + headlessUrl: "test2", + }]); + + assert.equal(fetchCalledWith, null); + }); + it("should do nothing when on windows", async function() { + let logLine = null; + server._logger = { info: line => { logLine = line; } }; + Object.defineProperty(process, "platform", { value: "win32" }); + + server._factorioDir = path.join("test", "file", "factorio"); + server._targetVersion = target === "0.1.1" ? "0.1.5" : target; + await server.checkForUpdates([{ + stable: true, + version: "0.1.5", + headlessUrl: "test1", + }, { + stable: true, + version: "0.1.1", + headlessUrl: "test2", + }, { + stable: false, + version: "0.1.0", + headlessUrl: "test3", + }]); + + assert.equal(fetchCalledWith, null); + assert.ok(logLine !== null); + assert.ok(logLine.endsWith("but must be manually downloaded")); + }); + it("should do attempt to download on linux", async function() { + let logLine = null; + server._logger = { info: line => { logLine = line; } }; + Object.defineProperty(process, "platform", { value: "linux" }); + + server._factorioDir = path.join("test", "file", "factorio"); + server._targetVersion = target === "0.1.1" ? "0.1.5" : target; + await assert.rejects(server.checkForUpdates([{ + stable: true, + version: "0.1.5", + headlessUrl: "test1", + }, { + stable: true, + version: "0.1.1", + headlessUrl: "test2", + }, { + stable: false, + version: "0.1.0", + headlessUrl: "test3", + }]), new Error("Failed to fetch test1: -1 Fetch called")); + + assert.equal(fetchCalledWith, "test1"); + assert.ok(logLine !== null); + assert.ok(logLine.endsWith("starting download...")); + }); }); - it("should do nothing for direct installs", async function() { - server._factorioDir = path.join("test", "file", "factorio", "0.1.1"); - server._targetVersion = "0.1.5"; - await server.checkForUpdates([{ - stable: true, - version: "0.1.5", - headlessUrl: "test1", - }, { - stable: true, - version: "0.1.1", - headlessUrl: "test1", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test2", - }]); - - assert.equal(fetchCalledWith, null); - }); - it("should do nothing when on windows", async function() { - let logLine = null; - server._logger = { info: line => { logLine = line; } }; - Object.defineProperty(process, "platform", { value: "win32" }); - - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = "0.1.5"; - await server.checkForUpdates([{ - stable: true, - version: "0.1.5", - headlessUrl: "test1", - }, { - stable: true, - version: "0.1.1", - headlessUrl: "test2", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test3", - }]); - - assert.equal(fetchCalledWith, null); - assert.ok(logLine !== null); - assert.ok(logLine.endsWith("but must be manually downloaded")); - }); - it("should do attempt to download on linux", async function() { - let logLine = null; - server._logger = { info: line => { logLine = line; } }; - Object.defineProperty(process, "platform", { value: "linux" }); - - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = "0.1.5"; - await assert.rejects(server.checkForUpdates([{ - stable: true, - version: "0.1.5", - headlessUrl: "test1", - }, { - stable: true, - version: "0.1.1", - headlessUrl: "test2", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test3", - }]), new Error("Failed to fetch test1: -1 Fetch called")); - - assert.equal(fetchCalledWith, "test1"); - assert.ok(logLine !== null); - assert.ok(logLine.endsWith("starting download...")); - }); - }); - describe("partial version", function() { - it("should do nothing when there is no newer version", async function() { - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = "0.1"; - await server.checkForUpdates([{ - stable: true, - version: "0.1.1", - headlessUrl: "test1", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test2", - }]); - - assert.equal(fetchCalledWith, null); - }); - it("should do nothing for direct installs", async function() { - server._factorioDir = path.join("test", "file", "factorio", "0.1.1"); - server._targetVersion = "0.1"; - await server.checkForUpdates([{ - stable: true, - version: "0.1.5", - headlessUrl: "test1", - }, { - stable: true, - version: "0.1.1", - headlessUrl: "test1", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test2", - }]); - - assert.equal(fetchCalledWith, null); - }); - it("should do nothing when on windows", async function() { - let logLine = null; - server._logger = { info: line => { logLine = line; } }; - Object.defineProperty(process, "platform", { value: "win32" }); - - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = "0.1"; - await server.checkForUpdates([{ - stable: true, - version: "0.1.5", - headlessUrl: "test1", - }, { - stable: true, - version: "0.1.1", - headlessUrl: "test2", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test3", - }]); - - assert.equal(fetchCalledWith, null); - assert.ok(logLine !== null); - assert.ok(logLine.endsWith("but must be manually downloaded")); - }); - it("should do attempt to download on linux", async function() { - let logLine = null; - server._logger = { info: line => { logLine = line; } }; - Object.defineProperty(process, "platform", { value: "linux" }); - - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = "0.1"; - await assert.rejects(server.checkForUpdates([{ - stable: true, - version: "0.1.5", - headlessUrl: "test1", - }, { - stable: true, - version: "0.1.1", - headlessUrl: "test2", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test3", - }]), new Error("Failed to fetch test1: -1 Fetch called")); - - assert.equal(fetchCalledWith, "test1"); - assert.ok(logLine !== null); - assert.ok(logLine.endsWith("starting download...")); - }); - }); - describe("latest version", function() { - it("should do nothing when there is no newer version", async function() { - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = "latest"; - await server.checkForUpdates([{ - stable: true, - version: "0.1.1", - headlessUrl: "test1", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test2", - }]); - - assert.equal(fetchCalledWith, null); - }); - it("should do nothing for direct installs", async function() { - server._factorioDir = path.join("test", "file", "factorio", "0.1.1"); - server._targetVersion = "latest"; - await server.checkForUpdates([{ - stable: true, - version: "0.1.5", - headlessUrl: "test1", - }, { - stable: true, - version: "0.1.1", - headlessUrl: "test1", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test2", - }]); - - assert.equal(fetchCalledWith, null); - }); - it("should do nothing when on windows", async function() { - let logLine = null; - server._logger = { info: line => { logLine = line; } }; - Object.defineProperty(process, "platform", { value: "win32" }); - - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = "latest"; - await server.checkForUpdates([{ - stable: true, - version: "0.1.5", - headlessUrl: "test1", - }, { - stable: true, - version: "0.1.1", - headlessUrl: "test2", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test3", - }]); - - assert.equal(fetchCalledWith, null); - assert.ok(logLine !== null); - assert.ok(logLine.endsWith("but must be manually downloaded")); - }); - it("should do attempt to download on linux", async function() { - let logLine = null; - server._logger = { info: line => { logLine = line; } }; - Object.defineProperty(process, "platform", { value: "linux" }); - - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = "latest"; - await assert.rejects(server.checkForUpdates([{ - stable: true, - version: "0.1.5", - headlessUrl: "test1", - }, { - stable: true, - version: "0.1.1", - headlessUrl: "test2", - }, { - stable: false, - version: "0.1.0", - headlessUrl: "test3", - }]), new Error("Failed to fetch test1: -1 Fetch called")); - - assert.equal(fetchCalledWith, "test1"); - assert.ok(logLine !== null); - assert.ok(logLine.endsWith("starting download...")); - }); - }); + /* eslint-enable no-loop-func */ + } it("should download a version correctly (live api)", async function() { slowTest(this); if (_platform !== "linux") { diff --git a/test/lib/external/FactorioVersions.test.js b/test/lib/external/FactorioVersions.test.js index 9058e6e0..c2e4f49a 100644 --- a/test/lib/external/FactorioVersions.test.js +++ b/test/lib/external/FactorioVersions.test.js @@ -121,6 +121,12 @@ describe("FactorioVersions", function() { assert.equal(versions.filter(v => v.stable).length, 48); assert.equal(versions.filter(v => !v.stable).length, 57); }); + it("parses versions from empty html file", async function() { + mockFetch(""); + + const versions = await fetchFactorioVersions(); + assert.equal(versions.length, 0); + }); it("parses versions from live api", async function() { slowTest(this); global.fetch = originalFetch; From 2f70db4b7e92ab8427521fd77d72082c7f1f61fa Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 9 Mar 2026 15:16:46 +0100 Subject: [PATCH 04/14] Ignore errors fetching Factorio versions If an error occurs trying to fetch the list of Factorio versions log a warning and then return an empty list instead of preventing the system from operating. --- packages/controller/src/Controller.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index a5671795..6e1f8a78 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -112,7 +112,14 @@ export default class Controller { // Cache for mod portal requests modPortalCache = new Map(); // Cache for factorio versions - factorioVersions = new lib.ValueCache(lib.fetchFactorioVersions); + factorioVersions = new lib.ValueCache(async () => { + try { + return await lib.fetchFactorioVersions(); + } catch (err: any) { + logger.warn(`Failed to retrieve Factorio versions ${err.message}`); + return []; + } + }); static async bootstrap(config: lib.ControllerConfig) { let databaseDirectory = config.get("controller.database_directory"); From 41b9eb1005ee55c274ea4076d0d30123f223243f Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 9 Mar 2026 15:32:43 +0100 Subject: [PATCH 05/14] Avoid redirect in fetchFactorioVersions Speed up fetching of result by avoiding a roundtrip due to path being redirected to a path ending in a /. --- packages/lib/src/external/FactorioVersions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/lib/src/external/FactorioVersions.ts b/packages/lib/src/external/FactorioVersions.ts index 0fe9b8c6..86360b39 100644 --- a/packages/lib/src/external/FactorioVersions.ts +++ b/packages/lib/src/external/FactorioVersions.ts @@ -1,7 +1,7 @@ import { Static, Type } from "@sinclair/typebox"; import { FullVersionSchema, integerFullVersion, isFullVersion } from "../data/version"; -const ARCHIVE_URL = "https://factorio.com/download/archive"; +const ARCHIVE_URL = "https://factorio.com/download/archive/"; /** * Represents the parsed result of a factorio version From 2399fb9d15d8dda5b146c183947e0370c6e9d6c2 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 9 Mar 2026 15:58:35 +0100 Subject: [PATCH 06/14] Fix NpmButton fetching config on every render The useEffect updates list has the major pitfall that passing an object via for example a prop like foo={{ bar: 1 }} to it causes it to to detect it as a change on every re-render due to the newly created object comparing unqual to the previously passed in object. Unpack the object to prevent the identity from changing every time the metrics update on the Host page. --- packages/web_ui/src/components/NpmButton.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/web_ui/src/components/NpmButton.tsx b/packages/web_ui/src/components/NpmButton.tsx index 8696cac1..aa01c7b4 100644 --- a/packages/web_ui/src/components/NpmButton.tsx +++ b/packages/web_ui/src/components/NpmButton.tsx @@ -82,7 +82,11 @@ function NpmPluginInstall({ setApplyAction, form, target }: NpmActionProps) { ; } -export function NpmButton(props: { target: lib.AddressShorthand, canRestart?: boolean, disabled?: boolean }) { +export function NpmButton(props: { + target: "controller" | { hostId: number }, + canRestart?: boolean, + disabled?: boolean +}) { const control = useContext(ControlContext); const account = useAccount(); const [open, setOpen] = useState(false); @@ -93,6 +97,7 @@ export function NpmButton(props: { target: lib.AddressShorthand, canRestart?: bo remoteUpdates: true, pluginUpdates: true, pluginInstall: true, }); + const hostId = props.target === "controller" ? undefined : props.target.hostId; useEffect(() => { if (props.target === "controller") { if (!account.hasPermission("core.controller.get_config")) { @@ -121,7 +126,7 @@ export function NpmButton(props: { target: lib.AddressShorthand, canRestart?: bo }); }).catch(notifyErrorHandler("Failed to fetch host config for remote updates")); } - }, [props.target]); + }, [hostId]); function onValuesChange({ action } : { action?: string }) { if (action) { From b0595a6502af20a8d1ea4abab6c45cfaddf7f8e5 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 9 Mar 2026 16:20:21 +0100 Subject: [PATCH 07/14] Fix identical updatedAtMs values in datastore The assumption with the subscription system is that new updates will always increment the updatedAtMs value to something greater than what has been seen before. This might not happen if the system clock is adjusted, or updates are sent so fast that Date.now() returns the same value. Fix these edge cases so that the timestamp is always incremented. --- packages/lib/src/datastore.ts | 45 ++++++++++++++--- test/controller/User.test.js | 6 +++ test/lib/datastore.js | 91 +++++++++++++++++++++++++++++------ 3 files changed, 121 insertions(+), 21 deletions(-) diff --git a/packages/lib/src/datastore.ts b/packages/lib/src/datastore.ts index 0dae1a57..48b9d15b 100644 --- a/packages/lib/src/datastore.ts +++ b/packages/lib/src/datastore.ts @@ -290,18 +290,50 @@ export class KeyValueDatastore< export class SubscribableDatastore< V extends SubscribableValue, > extends Datastore { + /** Last time an update occurred */ + lastUpdatedAtMs: number = 0; + + private static maxUpdatedAtMs(values: Iterable) { + let max = 0; + for (const value of values) { + // Prevent one NaN, undefined, +Inf or similar from breaking the datastore. + if (Number.isFinite(value.updatedAtMs)) { + max = Math.max(max, value.updatedAtMs); + } + } + return max; + } + + private incrementLastUpdatedMs() { + this.lastUpdatedAtMs = Math.max(Date.now(), this.lastUpdatedAtMs + 1); + } + + constructor( + provider: DatastoreProvider = new MemoryDatastoreProvider(), + data = new Map(), + ) { + super(provider, data); + this.lastUpdatedAtMs = SubscribableDatastore.maxUpdatedAtMs(this.data.values()); + } + + async load() { + await super.load(); + this.lastUpdatedAtMs = SubscribableDatastore.maxUpdatedAtMs(this.data.values()); + } + // Set the value in the datastore, be careful of race conditions if you await any functions before calling set set(value: V) { + this.incrementLastUpdatedMs(); this.data.set(value.id, value); - value.updatedAtMs = Date.now(); + value.updatedAtMs = this.lastUpdatedAtMs; this.touch([value]); } // Set many values at once from an array of key value pairs setMany(values: V[]) { - const nowMs = Date.now(); + this.incrementLastUpdatedMs(); for (const value of values) { - value.updatedAtMs = nowMs; + value.updatedAtMs = this.lastUpdatedAtMs; this.data.set(value.id, value); } this.touch(values); @@ -309,18 +341,19 @@ export class SubscribableDatastore< // Delete a value in the datastore delete(value: V) { + this.incrementLastUpdatedMs(); this.data.delete(value.id); - value.updatedAtMs = Date.now(); + value.updatedAtMs = this.lastUpdatedAtMs; value.isDeleted = true; this.touch([value]); } // Delete many values at once from an array of values deleteMany(values: V[]) { - const nowMs = Date.now(); + this.incrementLastUpdatedMs(); for (const value of values) { value.isDeleted = true; - value.updatedAtMs = nowMs; + value.updatedAtMs = this.lastUpdatedAtMs; this.data.delete(value.id); } this.touch(values); diff --git a/test/controller/User.test.js b/test/controller/User.test.js index 3e5ea4ab..cf605280 100644 --- a/test/controller/User.test.js +++ b/test/controller/User.test.js @@ -37,6 +37,12 @@ describe("controller/User", function () { user.saveRecord(); assert(user.updatedAtMs > prev); }); + it("should always increment updatedAtMs even if called in quick succession", function () { + user.saveRecord(); + let prev = user.updatedAtMs; + user.saveRecord(); + assert(user.updatedAtMs > prev, "updatedAtMs did not increase"); + }); }); describe(".set()", function () { diff --git a/test/lib/datastore.js b/test/lib/datastore.js index 93b53277..ba2cf116 100644 --- a/test/lib/datastore.js +++ b/test/lib/datastore.js @@ -5,13 +5,6 @@ const lib = require("@clusterio/lib"); const fs = require("fs-extra"); const path = require("path"); -async function incrementDateMs() { - const prev = Date.now(); - do { - await lib.wait(1); - } while (prev >= Date.now()); -} - class MockDatastoreProvider extends lib.DatastoreProvider { constructor() { super(); @@ -571,12 +564,31 @@ describe("lib/datastore", function() { eventsRaised = []; datastoreProvider = new MockDatastoreProvider(); datastoreProvider.value = new Map([ - ["foo", {id: "foo"}], ["bar", {id: "bar"}], ["baz", {id: "baz"}], + ["foo", {id: "foo", updatedAtMs: 10}], ["bar", {id: "bar"}], ["baz", {id: "baz"}], ]); datastore = new lib.SubscribableDatastore(datastoreProvider, datastoreProvider.value); datastore.on("update", updates => eventsRaised.push(updates)); }); + function forceFuture() { + const future = {id: "future"}; + for (let i = 1; i < 100; i++) { + datastore.set(future); + } + return future.updatedAtMs; + } + + describe("lastUpdateAtMs", function() { + it("is set to the highest value after constructing a new instance", function() { + assert(datastore.lastUpdatedAtMs === 10, `${datastore.lastUpdatedAtMs} !== 10`); + }); + it("is set to the highest value after loading from a provider", async function() { + const future = Date.now() + 2000; + datastore.provider.value = ["future", {id: "future", updatedAtMs: future}]; + await datastore.load(); + assert(datastore.lastUpdatedAtMs === future, `${datastore.lastUpdatedAtMs} !== ${future}`); + }); + }); describe("set", function() { it("can set a value", function() { const value = { id: "test" }; @@ -598,12 +610,21 @@ describe("lib/datastore", function() { const value = { id: "test" }; datastore.set(value); assert.notEqual(value.updatedAtMs, undefined); - assert(value.updatedAtMs > 0, "updatedAtMs greater than zero"); + assert(value.updatedAtMs > 0, "updatedAtMs not greater than zero"); + }); + it("increments the updatedAtMs property", async function() { + const value = { id: "test" }; + datastore.set(value); const prev = value.updatedAtMs; - await incrementDateMs(); datastore.set(value); assert(value.updatedAtMs > prev, "updatedAtMs must increase"); }); + it("increments the updatedAtMs property past existing values", async function() { + const future = forceFuture(); + const value = { id: "test" }; + datastore.set(value); + assert(value.updatedAtMs > future, "updatedAtMs must increase"); + }); }); describe("setMany", function() { @@ -632,11 +653,22 @@ describe("lib/datastore", function() { assert.notEqual(updates[1].updatedAtMs, undefined); assert(updates[0].updatedAtMs > 0, "updatedAtMs greater than zero"); assert(updates[1].updatedAtMs > 0, "updatedAtMs greater than zero"); - const prev = [updates[0].updatedAtMs, updates[1].updatedAtMs]; - await incrementDateMs(); + }); + it("increments the updatedAtMs property", async function() { + const value = { id: "test1" }; + datastore.set(value); + const prev = value.updatedAtMs; + const updates = [{ id: "test1" }, { id: "test2" }]; datastore.setMany(updates); - assert(updates[0].updatedAtMs > prev[0], "updatedAtMs must increase"); - assert(updates[1].updatedAtMs > prev[1], "updatedAtMs must increase"); + assert(updates[0].updatedAtMs > prev, "updatedAtMs must increase"); + assert(updates[1].updatedAtMs > prev, "updatedAtMs must increase"); + }); + it("increments the updatedAtMs property past existing values", async function() { + const future = forceFuture(); + const updates = [{ id: "test1" }, { id: "test2" }]; + datastore.setMany(updates); + assert(updates[0].updatedAtMs > future, "updatedAtMs must increase"); + assert(updates[1].updatedAtMs > future, "updatedAtMs must increase"); }); }); @@ -667,7 +699,20 @@ describe("lib/datastore", function() { const value = { id: "test" }; datastore.delete(value); assert.notEqual(value.updatedAtMs, undefined); - assert(value.updatedAtMs > 0, "updatedAtMs greater than zero"); + assert(value.updatedAtMs > 0, "updatedAtMs not greater than zero"); + }); + it("increments the updatedAtMs property", function() { + const value = { id: "test" }; + datastore.set(value); + const prev = value.updatedAtMs; + datastore.delete(value); + assert(value.updatedAtMs > prev, "updatedAtMs must increase"); + }); + it("increments the updatedAtMs property past existing values", function() { + const future = forceFuture(); + const value = { id: "test" }; + datastore.delete(value); + assert(value.updatedAtMs > future, "updatedAtMs must increase"); }); it("sets the isDeleted property", function() { const value = { id: "test" }; @@ -704,6 +749,22 @@ describe("lib/datastore", function() { assert(values[0].updatedAtMs > 0, "updatedAtMs greater than zero"); assert(values[1].updatedAtMs > 0, "updatedAtMs greater than zero"); }); + it("increments the updatedAtMs property", function() { + const value = { id: "foo" }; + datastore.set(value); + const prev = value.updatedAtMs; + const values = [{id: "foo", updatedAtMs: prev}, {id: "bar", updatedAtMs: prev}]; + datastore.deleteMany(values); + assert(values[0].updatedAtMs > prev, "updatedAtMs must increase"); + assert(values[1].updatedAtMs > prev, "updatedAtMs must increase"); + }); + it("increments the updatedAtMs property past existing values", function() { + const future = forceFuture(); + const values = [{id: "foo"}, {id: "bar"}]; + datastore.deleteMany(values); + assert(values[0].updatedAtMs > future, "updatedAtMs must increase"); + assert(values[1].updatedAtMs > future, "updatedAtMs must increase"); + }); it("sets the isDeleted property", function() { const values = [{id: "foo"}, {id: "bar"}]; datastore.deleteMany(values); From f6eb55163fa7e823085df4a24bf976a995df0b23 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 9 Mar 2026 19:07:10 +0100 Subject: [PATCH 08/14] Fix metrics broken by comma and equal in labels The logic for turning a key back into the labelles that defined it would not account for the labels values possibly including a comma or equal sign, which in turn broke the logic for handling these metrics. Fix by properly parsing the keys. --- packages/lib/src/prometheus.ts | 21 +++++++++++++++------ test/lib/prometheus.js | 17 +++++++++++++++-- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/packages/lib/src/prometheus.ts b/packages/lib/src/prometheus.ts index 2a55ddd7..becdf0cb 100644 --- a/packages/lib/src/prometheus.ts +++ b/packages/lib/src/prometheus.ts @@ -272,18 +272,27 @@ function labelsToKey(labels: LabelValues, metricLabels: string[]) { function keyToLabels(key: string) { let labels = new Map(); - if (key !== "") { - for (let pair of key.split(",")) { - let [name, value] = pair.split("=", 2); - if (!name || value === undefined) { - continue; + let pos = 0; + let matchLabel = /([^=]+)="(([^\\"]|\\["n\\])+)"/y; + while (pos < key.length) { + if (pos !== 0) { + if (key[pos] !== ",") { + throw Error(`Expected , at pos ${pos} in "${key}"`); } + pos += 1; + } + matchLabel.lastIndex = pos; + const match = matchLabel.exec(key); + if (match) { + let [, name, value] = match; labels.set(name, value - .slice(1, -1) .replace(/\\"/g, "\"") .replace(/\\n/g, "\n") .replace(/\\\\/g, "\\") ); + pos += match[0].length; + } else { + throw new Error(`Failed to parse label at pos ${pos} in "${key}"`); } } diff --git a/test/lib/prometheus.js b/test/lib/prometheus.js index b0f24ca3..ade1b09f 100644 --- a/test/lib/prometheus.js +++ b/test/lib/prometheus.js @@ -651,8 +651,21 @@ describe("lib/prometheus", function() { let gauge = new prometheus.Gauge( "test", "Help", { register: false, labels: ["a", "b"] } ); - gauge.labels('x\\\\\n\"\"\n\\', "2"); - gauge.removeAll({ a: 'x\\\\\n\"\"\n\\' }); + + let labelValue = ""; + for (let i = 0; i < 128; i++) { + const char = String.fromCodePoint(i); + if (!/[1-9B-Zb-z]/.test(char)) { // exclude boring characters + labelValue += char; + } + } + labelValue += "©💣"; + + gauge.labels(labelValue, "2"); + gauge.removeAll({ a: labelValue }); + assert(gauge._values.size === 0, "Matching labels remained"); + gauge.labels("1", labelValue); + gauge.removeAll({ a: "1" }); assert(gauge._values.size === 0, "Matching labels remained"); }); }); From 05ae607a6a4ef3d5cb8183d2b0fb0fe721ce95a1 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 9 Mar 2026 20:09:07 +0100 Subject: [PATCH 09/14] Fix tests for checkForUpdates Fix incorrect suite name and attempting to test no version matches using the "latest" version which always matches in the tests for the FactorioServer.checkForUpdates method. --- test/host/server.js | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/test/host/server.js b/test/host/server.js index 70f9cff8..13de4e75 100644 --- a/test/host/server.js +++ b/test/host/server.js @@ -348,7 +348,7 @@ describe("host/server", function() { ["latest version", "latest"], ]) { /* eslint-disable no-loop-func */ - describe("full version", function() { + describe(suite, function() { it("should do nothing when there are no versions", async function() { server._factorioDir = path.join("test", "file", "factorio"); server._targetVersion = target; @@ -356,21 +356,23 @@ describe("host/server", function() { assert.equal(fetchCalledWith, null); }); - it("should do nothing when no version matches", async function() { - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = target; - await server.checkForUpdates([{ - stable: true, - version: "0.2.1", - headlessUrl: "test1", - }, { - stable: false, - version: "0.2.0", - headlessUrl: "test2", - }]); + if (target !== "latest") { + it("should do nothing when no version matches", async function() { + server._factorioDir = path.join("test", "file", "factorio"); + server._targetVersion = target; + await server.checkForUpdates([{ + stable: true, + version: "0.2.1", + headlessUrl: "test1", + }, { + stable: false, + version: "0.2.0", + headlessUrl: "test2", + }]); - assert.equal(fetchCalledWith, null); - }); + assert.equal(fetchCalledWith, null); + }); + } it("should do nothing when there is no newer version", async function() { server._factorioDir = path.join("test", "file", "factorio"); server._targetVersion = target; From 30f552847ab4a8bef7ffd32b36610bc8de228e80 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 10 Mar 2026 02:13:13 +0100 Subject: [PATCH 10/14] Move FactorioVersionsRequest to BaseConnection No reason to duplicate this in both the instance and host connection if the implementation is identical in both. --- packages/controller/src/BaseConnection.ts | 5 +++++ packages/controller/src/ControlConnection.ts | 5 ----- packages/controller/src/HostConnection.ts | 5 ----- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/controller/src/BaseConnection.ts b/packages/controller/src/BaseConnection.ts index 8efbf14c..07b753f8 100644 --- a/packages/controller/src/BaseConnection.ts +++ b/packages/controller/src/BaseConnection.ts @@ -32,6 +32,7 @@ export default class BaseConnection extends lib.Link { this.handle(lib.ModPackGetRequest, this.handleModPackGetRequest.bind(this)); this.handle(lib.ModPackGetDefaultRequest, this.handleModPackGetDefaultRequest.bind(this)); this.handle(lib.ModDownloadRequest, this.handleModDownloadRequest.bind(this)); + this.handle(lib.FactorioVersionsRequest, this.handleFactorioVersionsRequest.bind(this)); this.handle(lib.SubscriptionRequest, this.handleSubscriptionRequest.bind(this)); this.connector.on("close", () => { @@ -106,4 +107,8 @@ export default class BaseConnection extends lib.Link { async handleSubscriptionRequest(request: lib.SubscriptionRequest, src: lib.Address, dst: lib.Address) { return await this._controller.subscriptions.handleRequest(this, request, src, dst); } + + async handleFactorioVersionsRequest(request: lib.FactorioVersionsRequest) { + return await this._controller.factorioVersions.get(request.maxAgeMs); + } } diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index 673a9277..d1ac875c 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/packages/controller/src/ControlConnection.ts @@ -127,7 +127,6 @@ export default class ControlConnection extends BaseConnection { this.handle(lib.PluginUpdateRequest, this.handlePluginUpdateRequest.bind(this)); this.handle(lib.PluginInstallRequest, this.handlePluginInstallRequest.bind(this)); this.handle(lib.DebugDumpWsRequest, this.handleDebugDumpWsRequest.bind(this)); - this.handle(lib.FactorioVersionsRequest, this.handleFactorioVersionsRequest.bind(this)); } validateIngress(message: lib.MessageRequest | lib.MessageEvent) { @@ -1240,10 +1239,6 @@ export default class ControlConnection extends BaseConnection { this.connector._socket!.clusterio_ignore_dump = true; this._controller.debugEvents.on("message", this.ws_dumper); } - - async handleFactorioVersionsRequest(request: lib.FactorioVersionsRequest) { - return await this._controller.factorioVersions.get(request.maxAgeMs); - } } module.exports = ControlConnection; diff --git a/packages/controller/src/HostConnection.ts b/packages/controller/src/HostConnection.ts index 66601e20..e7132925 100644 --- a/packages/controller/src/HostConnection.ts +++ b/packages/controller/src/HostConnection.ts @@ -81,7 +81,6 @@ export default class HostConnection extends BaseConnection { this.handle(lib.InstanceSaveDetailsUpdatesEvent, this.handleInstanceSaveDetailsUpdatesEvent.bind(this)); this.handle(lib.LogMessageEvent, this.handleLogMessageEvent.bind(this)); this.handle(lib.InstancePlayerUpdateEvent, this.handleInstancePlayerUpdateEvent.bind(this)); - this.handle(lib.FactorioVersionsRequest, this.handleFactorioVersionsRequest.bind(this)); this.snoopEvent(lib.InstanceAdminlistUpdateEvent, this.snoopInstanceAdminlistUpdateEvent.bind(this)); this.snoopEvent(lib.InstanceBanlistUpdateEvent, this.snoopInstanceBanlistUpdateEvent.bind(this)); this.snoopEvent(lib.InstanceWhitelistUpdateEvent, this.snoopInstanceWhitelistUpdateEvent.bind(this)); @@ -359,10 +358,6 @@ export default class HostConnection extends BaseConnection { }); } - async handleFactorioVersionsRequest(request: lib.FactorioVersionsRequest) { - return await this._controller.factorioVersions.get(request.maxAgeMs); - } - async snoopInstanceAdminlistUpdateEvent(event: lib.InstanceAdminlistUpdateEvent, src: lib.Address) { if (src.type !== lib.Address.instance) { return; From 66c066616b37c80e0b087e1da472cb3157fa2691 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 10 Mar 2026 02:14:44 +0100 Subject: [PATCH 11/14] Add test for Controller.factorioVersions failure Check that errors from fetchFactorioVersions is correctly handled in the fetch method provided by the Controller class. --- test/controller/Controller.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/controller/Controller.js b/test/controller/Controller.js index ae45a11b..7b0d4127 100644 --- a/test/controller/Controller.js +++ b/test/controller/Controller.js @@ -252,6 +252,28 @@ describe("controller/src/Controller", function() { assert.equal(controller.config.restartRequired, true); }); }); + + describe(".factorioVersions", function() { + const _fetch = global.fetch; + const fetchCalls = []; + before(function() { + global.fetch = async (url) => { + fetchCalls.push(url); + return { ok: false, status: 503, statusText: "Service Unavailable" }; + }; + }); + it("should handle fetch failure", async function() { + await assert.deepEqual( + await controller.factorioVersions.get(0), + [], + ); + assert(fetchCalls.length === 1, "expected one fetch call"); + assert(fetchCalls[0].indexOf("/download") !== -1, "fetch call to be to a URL with /download in it"); + }); + after(function() { + global.fetch = _fetch; + }); + }); }); describe("finalise", function() { describe(".finaliseHosts()", function() { From 86a648e759ef40b3b167565726a44e697017588e Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 10 Mar 2026 02:17:36 +0100 Subject: [PATCH 12/14] Detail requirements for instance waking requests These need to make sure instance status is correctly handled, otherwise they may cause the instance or host to become inoperable. --- packages/host/src/Host.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts index b096a889..c3f2466e 100644 --- a/packages/host/src/Host.ts +++ b/packages/host/src/Host.ts @@ -74,7 +74,10 @@ async function discoverInstances(instancesDir: string) { return instances; } -// Requests that wake up the instance is responsible for updating the instance state in case of errors. +// Requests that wake up instances are responsible for making sure the +// instance status is correctly updated even if they fail with an error. This +// means they should typically use try catch on anything that can throw +// and call notifyExit upon any errors. const instanceStartingMessages = new Set([ lib.InstanceStartRequest.name, lib.InstanceLoadScenarioRequest.name, From 60eb2389a370341028882d3e6cf18615d29acfdb Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 10 Mar 2026 02:19:01 +0100 Subject: [PATCH 13/14] Test every ASCII character in prometheus test The a and A felt lonely being the only letters included. Add the other letters in the ASCII character set as there was no good reason to leave them out. --- test/lib/prometheus.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/lib/prometheus.js b/test/lib/prometheus.js index ade1b09f..8ef24147 100644 --- a/test/lib/prometheus.js +++ b/test/lib/prometheus.js @@ -653,12 +653,11 @@ describe("lib/prometheus", function() { ); let labelValue = ""; + // Include every ASCII codepoint for (let i = 0; i < 128; i++) { - const char = String.fromCodePoint(i); - if (!/[1-9B-Zb-z]/.test(char)) { // exclude boring characters - labelValue += char; - } + labelValue += String.fromCodePoint(i); } + // Include Unicode codepoints from the basic and supplementary planes. labelValue += "©💣"; gauge.labels(labelValue, "2"); From 76099d2a42b29e1ba42e3e155c0eb0b23b552343 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 10 Mar 2026 02:24:03 +0100 Subject: [PATCH 14/14] Document unreachable code paths in keyToLabels These were added to as a diagnostic in case the logic turns out to be broken. They should not be possible to reach as any value produced by labelsToKeys will not cause the error conditions to occur. --- packages/lib/src/prometheus.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/lib/src/prometheus.ts b/packages/lib/src/prometheus.ts index becdf0cb..84999093 100644 --- a/packages/lib/src/prometheus.ts +++ b/packages/lib/src/prometheus.ts @@ -276,13 +276,16 @@ function keyToLabels(key: string) { let matchLabel = /([^=]+)="(([^\\"]|\\["n\\])+)"/y; while (pos < key.length) { if (pos !== 0) { + /* istanbul ignore if */ if (key[pos] !== ",") { + // Should be unreachable throw Error(`Expected , at pos ${pos} in "${key}"`); } pos += 1; } matchLabel.lastIndex = pos; const match = matchLabel.exec(key); + /* istanbul ignore else */ if (match) { let [, name, value] = match; labels.set(name, value @@ -292,6 +295,7 @@ function keyToLabels(key: string) { ); pos += match[0].length; } else { + // Should be unreachable throw new Error(`Failed to parse label at pos ${pos} in "${key}"`); } }