From 024d7b427ac1962711d77b9b38c5d2f257bb6cb0 Mon Sep 17 00:00:00 2001 From: Bastiaan <17990055+bbassie@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:14:31 +0100 Subject: [PATCH] Rename safeOutputFile to atomicOverwriteFile (#1167) "safe" suggested the data being written survives a crash, it only guarantees the old file is either kept or fully replaced. The new name says what it does. No alias, plugin API breaks are allowed in minor versions and the 2.1 migration guide lists the rename. Co-authored-by: Claude Opus 5.5 Co-authored-by: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> --- docs/devs/plugin-migration-to-2.1.md | 6 ++++++ packages/controller/controller.ts | 2 +- packages/controller/src/Controller.ts | 2 +- packages/controller/src/UserManager.ts | 4 ++-- packages/controller/src/routes.ts | 2 +- packages/create/create.js | 22 +++++++++++----------- packages/host/src/Instance.ts | 10 +++++----- packages/lib/src/ModStore.ts | 4 ++-- packages/lib/src/config/classes.ts | 4 ++-- packages/lib/src/database.ts | 4 ++-- packages/lib/src/datastore.ts | 10 +++++----- packages/lib/src/file_ops.ts | 4 ++-- packages/lib/src/load_plugin_list.ts | 2 +- packages/lib/src/logging_utils.ts | 2 +- packages/lib/src/shared_commands.ts | 4 ++-- plugins/player_sync/controller.ts | 2 +- plugins/research_sync/controller.ts | 4 ++-- plugins/subspace_storage/controller.ts | 2 +- test/lib/file_ops.js | 14 +++++++------- 19 files changed, 55 insertions(+), 49 deletions(-) diff --git a/docs/devs/plugin-migration-to-2.1.md b/docs/devs/plugin-migration-to-2.1.md index 3a6a440f..56b96d6a 100644 --- a/docs/devs/plugin-migration-to-2.1.md +++ b/docs/devs/plugin-migration-to-2.1.md @@ -61,6 +61,12 @@ The `container` property of `WebPluginContext` is now the plugin's ES module nam `lib.PluginWebpackEnvInfo` is renamed to `lib.PluginWebEnvInfo`. +## Renamed lib functions + +`lib.safeOutputFile` is renamed to `lib.atomicOverwriteFile`. +It takes the same arguments. + + ## Host pairing Host tokens are no longer generated by users. diff --git a/packages/controller/controller.ts b/packages/controller/controller.ts index e2cdb06a..29334ed4 100644 --- a/packages/controller/controller.ts +++ b/packages/controller/controller.ts @@ -209,7 +209,7 @@ async function handleBootstrapCommand( console.log(content); } else { logger.info(`Writing ${args.output}`); - await lib.safeOutputFile(args.output, content, { mode: 0o600 }); + await lib.atomicOverwriteFile(args.output, content, { mode: 0o600 }); } } } diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index c5c838f8..e51d78fa 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -138,7 +138,7 @@ export default class Controller { const cachePath = path.join(this.config.get("controller.database_directory"), "latest-releases.json"); try { const releases = await lib.fetchLatestReleases(); - await lib.safeOutputFile(cachePath, JSON.stringify(releases, null, "\t")); + await lib.atomicOverwriteFile(cachePath, JSON.stringify(releases, null, "\t")); return releases; } catch (err: any) { logger.warn(`Failed to retrieve Factorio releases ${err.message}`); diff --git a/packages/controller/src/UserManager.ts b/packages/controller/src/UserManager.ts index 9ef56d7c..4e19bc28 100644 --- a/packages/controller/src/UserManager.ts +++ b/packages/controller/src/UserManager.ts @@ -40,8 +40,8 @@ export default class UserManager { } // We intentionally write the roles first because if it fails then the old user file will still contain them - await lib.safeOutputFile(rolesFilePath, JSON.stringify(combinedJson.roles)); - await lib.safeOutputFile(usersFilePath, JSON.stringify(combinedJson.users)); + await lib.atomicOverwriteFile(rolesFilePath, JSON.stringify(combinedJson.roles)); + await lib.atomicOverwriteFile(usersFilePath, JSON.stringify(combinedJson.users)); } catch (err: any) { if (err.code !== "ENOENT") { throw err; diff --git a/packages/controller/src/routes.ts b/packages/controller/src/routes.ts index 0499d22a..785283be 100644 --- a/packages/controller/src/routes.ts +++ b/packages/controller/src/routes.ts @@ -384,7 +384,7 @@ async function uploadExport(req: Request, res: Response) { let { name, ext } = path.posix.parse(filePath); let hash = await lib.hashStream(await zip.openStream(file)); assets[name] = `${name}.${hash}${ext}`; - await lib.safeOutputFile(path.join("static", `${name}.${hash}${ext}`), content); + await lib.atomicOverwriteFile(path.join("static", `${name}.${hash}${ext}`), content); } modPack.exportManifest = new lib.ExportManifest(assets, new Date().toISOString()); diff --git a/packages/create/create.js b/packages/create/create.js index 48b81ac5..d5d91505 100755 --- a/packages/create/create.js +++ b/packages/create/create.js @@ -104,7 +104,7 @@ async function pathExists(filePath) { return true; } -async function safeOutputFile(file, data, options={}) { +async function atomicOverwriteFile(file, data, options={}) { let { dir, name, ext } = path.parse(file); let temporary = path.join(dir, `${name}.tmp${ext}`); if (dir) { @@ -257,7 +257,7 @@ async function migrateRename(args) { async function migrateConfig(source, destination) { if (await pathExists(source) && !await pathExists(destination)) { - await safeOutputFile(destination, JSON.stringify( + await atomicOverwriteFile(destination, JSON.stringify( renameConfig(JSON.parse(await fs.readFile(source))), null, "\t" ), { mode: 0o600 }); logger.info(`Migrated ${source} to ${destination}`); @@ -269,7 +269,7 @@ async function migrateRename(args) { let instancesFile = path.join("database", "instances.json"); if (await pathExists(instancesFile)) { - await safeOutputFile(instancesFile, JSON.stringify( + await atomicOverwriteFile(instancesFile, JSON.stringify( JSON.parse(await fs.readFile(instancesFile)).map(renameConfig), null, "\t" )); logger.info(`Migrated ${instancesFile}`); @@ -281,7 +281,7 @@ async function migrateRename(args) { for (let role of users["roles"]) { role["permissions"] = role["permissions"].map(rename); } - await safeOutputFile(usersFile, JSON.stringify(users, null, "\t")); + await atomicOverwriteFile(usersFile, JSON.stringify(users, null, "\t")); logger.info(`Migrated ${usersFile}`); } @@ -309,7 +309,7 @@ async function migrateRename(args) { lines.push(inputLine); } } - safeOutputFile(outputFile, Buffer.concat(lines)); + atomicOverwriteFile(outputFile, Buffer.concat(lines)); } async function migrateLogsDir(inputLogs, outputLogs) { @@ -434,7 +434,7 @@ async function groupIdToName(gid) { async function writeScripts(mode, selfSignedCert) { if (["standalone", "controller"].includes(mode)) { if (process.platform === "win32") { - await safeOutputFile( + await atomicOverwriteFile( "run-controller.cmd", `\ @echo off @@ -447,7 +447,7 @@ goto restart ` ); } else { - await safeOutputFile( + await atomicOverwriteFile( "run-controller.sh", `\ #!/bin/bash @@ -459,7 +459,7 @@ done `, { mode: 0o755 }, ); - await safeOutputFile( + await atomicOverwriteFile( "systemd/clusteriocontroller.service", `\ [Unit] @@ -486,7 +486,7 @@ WantedBy=multi-user.target if (["standalone", "host"].includes(mode)) { if (process.platform === "win32") { let extraCaCertsLine = selfSignedCert ? `set "NODE_EXTRA_CA_CERTS=${selfSignedCert}"\n` : ""; - await safeOutputFile( + await atomicOverwriteFile( "run-host.cmd", `\ @echo off @@ -500,7 +500,7 @@ goto restart ); } else { let extraCaCertsLine = selfSignedCert ? `export "NODE_EXTRA_CA_CERTS=${selfSignedCert}"\n` : ""; - await safeOutputFile( + await atomicOverwriteFile( "run-host.sh", `\ #!/bin/bash @@ -513,7 +513,7 @@ done { mode: 0o755 }, ); extraCaCertsLine = selfSignedCert ? `Environment=NODE_EXTRA_CA_CERTS=${selfSignedCert}\n` : ""; - await safeOutputFile( + await atomicOverwriteFile( "systemd/clusteriohost.service", `\ [Unit] diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index 6cee6a7b..a6a6c02c 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -693,7 +693,7 @@ end`.replace(/\r?\n/g, " "); players: [...this.playerStats], player_autosave_slot: this._playerAutosaveSlot, }, null, "\t"); - await lib.safeOutputFile(this.path("instance-stats.json"), content); + await lib.atomicOverwriteFile(this.path("instance-stats.json"), content); } /** @@ -870,7 +870,7 @@ end`.replace(/\r?\n/g, " "); "_comment_warning": warning, ...await this.resolveServerSettings(this.config.get("factorio.settings"), includeCredentials), }; - await lib.safeOutputFile( + await lib.atomicOverwriteFile( this.server.writePath("server-settings.json"), JSON.stringify(serverSettings, null, "\t") ); @@ -996,7 +996,7 @@ end`.replace(/\r?\n/g, " "); if (this.config.get("factorio.sync_adminlist") !== "disabled") { this.logger.verbose("Writing server-adminlist.json"); - await lib.safeOutputFile( + await lib.atomicOverwriteFile( this.server.writePath("server-adminlist.json"), JSON.stringify([...this._host.adminlist], null, "\t") ); @@ -1004,7 +1004,7 @@ end`.replace(/\r?\n/g, " "); if (this.config.get("factorio.sync_banlist") !== "disabled") { this.logger.verbose("Writing server-banlist.json"); - await lib.safeOutputFile( + await lib.atomicOverwriteFile( this.server.writePath("server-banlist.json"), JSON.stringify([...this._host.banlist].map( ([username, reason]) => ({ username, reason }) @@ -1014,7 +1014,7 @@ end`.replace(/\r?\n/g, " "); if (this.config.get("factorio.sync_whitelist") !== "disabled") { this.logger.verbose("Writing server-whitelist.json"); - await lib.safeOutputFile( + await lib.atomicOverwriteFile( this.server.writePath("server-whitelist.json"), JSON.stringify([...this._host.whitelist], null, "\t") ); diff --git a/packages/lib/src/ModStore.ts b/packages/lib/src/ModStore.ts index 999157f3..3efb7a5b 100644 --- a/packages/lib/src/ModStore.ts +++ b/packages/lib/src/ModStore.ts @@ -3,7 +3,7 @@ import path from "path"; import events from "events"; import { Static } from "@sinclair/typebox"; import { logger } from "./logging.js"; -import { downloadFile, safeOutputFile } from "./file_ops.js"; +import { downloadFile, atomicOverwriteFile } from "./file_ops.js"; import { ModPortalReleaseSchema, ModPortalDetailsSchema, ModNameVersionPair } from "./data/messages_mod.js"; import { ModInfo, ModPack, MajorMinorVersion, ModVersionEquality, SourceVersion } from "./data/index.js"; @@ -148,7 +148,7 @@ export default class ModStore extends events.EventEmitter { if (files.size) { logger.info(`Loaded info for ${files.size} mods`); const content: ModInfoCache = [...files.values()].map(mod => mod.toJSON()); - await safeOutputFile(cacheFilename, JSON.stringify(content)); + await atomicOverwriteFile(cacheFilename, JSON.stringify(content)); } return new this(modsDirectory, files); diff --git a/packages/lib/src/config/classes.ts b/packages/lib/src/config/classes.ts index 19a7dbe0..84367ad6 100644 --- a/packages/lib/src/config/classes.ts +++ b/packages/lib/src/config/classes.ts @@ -7,7 +7,7 @@ import isDeepStrictEqual from "#is_deep_strict_equal"; import { basicType } from "../helpers.js"; import * as libSchema from "../schema.js"; import { StringEnum } from "../data/composites.js"; -import { safeOutputFile } from "../file_ops.js"; +import { atomicOverwriteFile } from "../file_ops.js"; import * as validators from "./validators.js"; import { InvalidAccessError, InvalidFieldError, InvalidValueError } from "../errors.js"; import type { SyncHook } from "../SyncHook.js"; @@ -475,7 +475,7 @@ export class Config< if (!this.filepath) { throw new Error("Cannot save config which has no filepath"); } - await safeOutputFile(this.filepath, JSON.stringify(this, null, "\t"), { mode: 0o600 }); + await atomicOverwriteFile(this.filepath, JSON.stringify(this, null, "\t"), { mode: 0o600 }); this._dirty = false; } diff --git a/packages/lib/src/database.ts b/packages/lib/src/database.ts index 1e94d298..2b833e5f 100644 --- a/packages/lib/src/database.ts +++ b/packages/lib/src/database.ts @@ -79,7 +79,7 @@ export async function loadJsonAsMap(filePath: string): Promise) { let obj = mapToObject(map); - await libFileOps.safeOutputFile(filePath, JSON.stringify(obj, null, "\t")); + await libFileOps.atomicOverwriteFile(filePath, JSON.stringify(obj, null, "\t")); } /** @@ -139,7 +139,7 @@ export async function loadJsonArrayAsMap(filePath: string): Promise) { - await libFileOps.safeOutputFile(filePath, JSON.stringify([...map.values()], null, "\t")); + await libFileOps.atomicOverwriteFile(filePath, JSON.stringify([...map.values()], null, "\t")); } diff --git a/packages/lib/src/datastore.ts b/packages/lib/src/datastore.ts index 1db3518e..2b39d94b 100644 --- a/packages/lib/src/datastore.ts +++ b/packages/lib/src/datastore.ts @@ -2,7 +2,7 @@ import path from "path"; import fs from "node:fs/promises"; import { EventEmitter } from "events"; -import { safeOutputFile } from "./file_ops.js"; +import { atomicOverwriteFile } from "./file_ops.js"; import { SubscribableValue } from "./subscriptions.js"; import { logger } from "./logging.js"; import { ControllerConfig } from "./config/index.js"; @@ -56,7 +56,7 @@ export class JsonDatastoreProvider< // Save the data to the json file async save(data: Map) { logger.verbose(`Saving ${this.filePath}`); - await safeOutputFile(this.filePath, JSON.stringify(Object.fromEntries(data), null, "\t")); + await atomicOverwriteFile(this.filePath, JSON.stringify(Object.fromEntries(data), null, "\t")); } // Load the data from the json file @@ -91,7 +91,7 @@ export class JsonDatastoreProvider< logger.warn( `Duplicates detected in ${this.filePath}, a backup was written to: ${backupPath}` ); - await safeOutputFile(backupPath, JSON.stringify(rawJson, null, "\t")); + await atomicOverwriteFile(backupPath, JSON.stringify(rawJson, null, "\t")); } return result; @@ -115,7 +115,7 @@ export class JsonIdDatastoreProvider< // Save the data to the json file async save(data: Map) { logger.verbose(`Saving ${this.filePath}`); - await safeOutputFile(this.filePath, JSON.stringify([...data.values()], null, "\t")); + await atomicOverwriteFile(this.filePath, JSON.stringify([...data.values()], null, "\t")); } // Load the data from the json file @@ -150,7 +150,7 @@ export class JsonIdDatastoreProvider< logger.warn( `Duplicates detected in ${this.filePath}, a backup was written to: ${backupPath}` ); - await safeOutputFile(backupPath, JSON.stringify(rawJson, null, "\t")); + await atomicOverwriteFile(backupPath, JSON.stringify(rawJson, null, "\t")); } return result; diff --git a/packages/lib/src/file_ops.ts b/packages/lib/src/file_ops.ts index de8b30c4..16701808 100644 --- a/packages/lib/src/file_ops.ts +++ b/packages/lib/src/file_ops.ts @@ -89,7 +89,7 @@ export async function getTempFile(prefix = "tmp.", suffix = "", tmpdir = "./") { } /** - * Safely overwrite file data + * Atomically overwrite file data * * Writes the passed data to a temporary file and rename it over the target * file. If the operation fails because the parent directory does not exist, it @@ -105,7 +105,7 @@ export async function getTempFile(prefix = "tmp.", suffix = "", tmpdir = "./") { * @param data - Content to write. * @param options - see fs.writeFile, `flag` and `flush` may not be used. */ -export async function safeOutputFile( +export async function atomicOverwriteFile( file: string, data: string | Buffer, options?: Omit< diff --git a/packages/lib/src/load_plugin_list.ts b/packages/lib/src/load_plugin_list.ts index d6838206..65f1e2fd 100644 --- a/packages/lib/src/load_plugin_list.ts +++ b/packages/lib/src/load_plugin_list.ts @@ -328,7 +328,7 @@ export async function loadPluginList( hasChanges = await findNpmPlugins(pluginList, skippedNpmPlugins) || hasChanges; if (hasChanges) { - await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); + await libFileOps.atomicOverwriteFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); } return pluginList; diff --git a/packages/lib/src/logging_utils.ts b/packages/lib/src/logging_utils.ts index 2a02f57d..ab235182 100644 --- a/packages/lib/src/logging_utils.ts +++ b/packages/lib/src/logging_utils.ts @@ -312,7 +312,7 @@ export class LogIndex { * was loaded/created from. */ async save() { - await libFileOps.safeOutputFile( + await libFileOps.atomicOverwriteFile( path.join(this.logDirectory, "index.json"), JSON.stringify(this.serialize(), null, "\t") ); diff --git a/packages/lib/src/shared_commands.ts b/packages/lib/src/shared_commands.ts index 7767ce8e..155be8f3 100644 --- a/packages/lib/src/shared_commands.ts +++ b/packages/lib/src/shared_commands.ts @@ -199,7 +199,7 @@ export async function handlePluginCommand( } pluginList.set(pluginInfo.name, pluginPath); - await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); + await libFileOps.atomicOverwriteFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); print(`Added ${pluginInfo.name}`); } else if (command === "remove") { @@ -209,7 +209,7 @@ export async function handlePluginCommand( return; } - await libFileOps.safeOutputFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); + await libFileOps.atomicOverwriteFile(pluginListPath, JSON.stringify([...pluginList], null, "\t")); print(`Removed ${args.name}`); } else if (command === "list") { diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index 0df18cb1..a94800cc 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -63,7 +63,7 @@ async function saveDatabase( let file = path.resolve(controllerConfig.get("controller.database_directory"), "player_sync.json"); logger.verbose(`writing ${file}`); let content = JSON.stringify([...playerDatastore, ...tombstones]); - await lib.safeOutputFile(file, content); + await lib.atomicOverwriteFile(file, content); } export default async function(context: ControllerPluginContext) { diff --git a/plugins/research_sync/controller.ts b/plugins/research_sync/controller.ts index 3f29a3a3..a4e02a47 100644 --- a/plugins/research_sync/controller.ts +++ b/plugins/research_sync/controller.ts @@ -49,7 +49,7 @@ async function saveTechnologies( ) { let filePath = path.join(controllerConfig.get("controller.database_directory"), "technologies.json"); logger.verbose(`writing ${filePath}`); - await lib.safeOutputFile(filePath, JSON.stringify([...technologies.entries()], null, "\t")); + await lib.atomicOverwriteFile(filePath, JSON.stringify([...technologies.entries()], null, "\t")); } const MAX_QUEUE = 7; @@ -71,7 +71,7 @@ async function loadQueue(controllerConfig: lib.ControllerConfig, logger: lib.Log async function saveQueue(controllerConfig: lib.ControllerConfig, queue: string[], logger: lib.Logger) { let filePath = path.join(controllerConfig.get("controller.database_directory"), "research_queue.json"); logger.verbose(`writing ${filePath}`); - await lib.safeOutputFile(filePath, JSON.stringify(queue, null, "\t")); + await lib.atomicOverwriteFile(filePath, JSON.stringify(queue, null, "\t")); } export default async function(context: ControllerPluginContext) { diff --git a/plugins/subspace_storage/controller.ts b/plugins/subspace_storage/controller.ts index cad2ec52..8f28b801 100644 --- a/plugins/subspace_storage/controller.ts +++ b/plugins/subspace_storage/controller.ts @@ -64,7 +64,7 @@ async function saveDatabase( let file = path.resolve(controllerConfig.get("controller.database_directory"), "items.json"); logger.verbose(`writing ${file}`); let content = JSON.stringify(items.serialize()); - await lib.safeOutputFile(file, content); + await lib.atomicOverwriteFile(file, content); } else if (items) { logger.error(`Item database too large, not saving (${items.size})`); } diff --git a/test/lib/file_ops.js b/test/lib/file_ops.js index 3e8dd21f..a44e069f 100644 --- a/test/lib/file_ops.js +++ b/test/lib/file_ops.js @@ -51,7 +51,7 @@ describe("lib/file_ops", function() { }); }); - describe("safeOutputFile()", function() { + describe("atomicOverwriteFile()", function() { async function assertNoTemporary(target) { let { dir, name } = path.parse(target); let entries = (await fs.readdir(dir)).filter( @@ -61,14 +61,14 @@ describe("lib/file_ops", function() { } it("should write new target file", async function() { let target = path.join(baseDir, "safe", "simple.txt"); - await lib.safeOutputFile(target, "a text file", "utf8"); + await lib.atomicOverwriteFile(target, "a text file", "utf8"); await assertNoTemporary(target); assert.equal(await fs.readFile(target, "utf8"), "a text file"); }); it("should overwrite existing target file", async function() { let target = path.join(baseDir, "safe", "exists.txt"); await fs.writeFile(target, "previous", "utf8"); - await lib.safeOutputFile(target, "current", "utf8"); + await lib.atomicOverwriteFile(target, "current", "utf8"); await assertNoTemporary(target); assert.equal(await fs.readFile(target, "utf8"), "current"); }); @@ -77,7 +77,7 @@ describe("lib/file_ops", function() { this.skip(); } let target = path.join(baseDir, "safe", "mode.txt"); - await lib.safeOutputFile(target, "private", { mode: 0o600 }); + await lib.atomicOverwriteFile(target, "private", { mode: 0o600 }); assert.equal((await fs.stat(target)).mode & 0o777, 0o600); assert.equal(await fs.readFile(target, "utf8"), "private"); }); @@ -85,7 +85,7 @@ describe("lib/file_ops", function() { let target = path.join(baseDir, "safe", "concurrent.txt"); let writes = []; for (let i = 0; i < 20; i++) { - writes.push(lib.safeOutputFile(target, `write ${i}`, "utf8")); + writes.push(lib.atomicOverwriteFile(target, `write ${i}`, "utf8")); } // Ignore errors from concurrently renaming onto the same file. await Promise.allSettled(writes); @@ -95,13 +95,13 @@ describe("lib/file_ops", function() { it("should remove the temporary file when the rename fails", async function() { let target = path.join(baseDir, "safe", "rename-fails"); await fs.mkdir(target, { recursive: true }); - await assert.rejects(lib.safeOutputFile(target, "data", "utf8")); + await assert.rejects(lib.atomicOverwriteFile(target, "data", "utf8")); await assertNoTemporary(target); }); it("should handle creating file in current working directory", async function() { let target = "temporary-file-made-to-test-cwd.txt"; try { - await lib.safeOutputFile(target, "a text file", "utf8"); + await lib.atomicOverwriteFile(target, "a text file", "utf8"); } finally { try { await fs.unlink(target);