mirror of
https://github.com/clusterio/clusterio.git
synced 2026-11-11 23:04:52 +09:00
Log errors from instance config change handlers
The fieldChanged handler in the Instance constructor ran updateFactorioSettings and updateFactorioWhitelist with only a finally attached. A rejection from either went unhandled, and the host exits on unhandled rejections, taking every instance on it down. Two ways to hit it against a real host: - Start an instance and immediately toggle factorio.enable_whitelist. updateFactorioWhitelist calls sendRcon while the server state is still "new" and it throws "Expected state running,stopping but state is new". - Set factorio.settings tags to [1] on a running instance. applyAsConfig called tag.replace outside its try block and threw "tag.replace is not a function". Catch and log both on the instance logger before invoking the hook, and convert tags with String() so non-string entries no longer throw. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
f4aa894fe7
commit
18aed41889
2 files changed
+58
-4
No files matched your search
@@ -61,7 +61,7 @@ function applyAsConfig(name: string) {
|
||||
// Replace spaces with non-break spaces and delimit by spaces.
|
||||
// This does change the defined tags, but there doesn't seem to
|
||||
// be a way to include a space into a tag from the console.
|
||||
value = value.map(tag => tag.replace(/ /g, "\u00a0")).join(" ");
|
||||
value = value.map(tag => String(tag).replace(/ /g, "\u00a0")).join(" ");
|
||||
}
|
||||
try {
|
||||
await instance.sendRcon(`/config set ${name} ${value}`);
|
||||
@@ -199,9 +199,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).finally(hook);
|
||||
this.updateFactorioSettings(curr as any, prev as any).catch(err => {
|
||||
this.logger.error(`Error updating server settings:\n${err.stack}`);
|
||||
}).finally(hook);
|
||||
} else if (field === "factorio.enable_whitelist") {
|
||||
this.updateFactorioWhitelist(curr as any).finally(hook);
|
||||
this.updateFactorioWhitelist(curr as any).catch(err => {
|
||||
this.logger.error(`Error updating whitelist:\n${err.stack}`);
|
||||
}).finally(hook);
|
||||
} else {
|
||||
if (field === "factorio.max_concurrent_commands") {
|
||||
this.server.maxConcurrentCommands = curr as number;
|
||||
|
||||
+51
-1
@@ -4,7 +4,7 @@ import path from "node:path";
|
||||
import * as lib from "@clusterio/lib";
|
||||
import { PlayerStats, wait } from "@clusterio/lib";
|
||||
import Instance from "@clusterio/host/dist/node/src/Instance.js";
|
||||
import { MockConnector, MockServer } from "../mock.js";
|
||||
import { MockConnector, MockLogger, MockServer } from "../mock.js";
|
||||
|
||||
const addr = lib.Address.fromShorthand;
|
||||
|
||||
@@ -38,6 +38,56 @@ describe("class Instance", function() {
|
||||
});
|
||||
});
|
||||
|
||||
describe("config fieldChanged", function() {
|
||||
let errors;
|
||||
let rejections;
|
||||
let hookInvoked;
|
||||
function onRejection(err) { rejections.push(err); }
|
||||
beforeEach(function() {
|
||||
errors = [];
|
||||
rejections = [];
|
||||
instance.logger = new MockLogger();
|
||||
instance.logger.error = message => errors.push(message);
|
||||
instance.server.exampleSettings = async () => ({});
|
||||
hookInvoked = new Promise(resolve => {
|
||||
instance.hooks.instanceConfigFieldChanged.attach("test", field => resolve(field));
|
||||
});
|
||||
process.on("unhandledRejection", onRejection);
|
||||
});
|
||||
afterEach(function() {
|
||||
process.off("unhandledRejection", onRejection);
|
||||
});
|
||||
|
||||
it("should log whitelist update errors and invoke the hook", async function() {
|
||||
instance.server.sendRcon = async () => { throw new Error("Expected state running,stopping"); };
|
||||
instance.config.set("factorio.enable_whitelist", true);
|
||||
assert.equal(await hookInvoked, "factorio.enable_whitelist");
|
||||
await wait(10);
|
||||
assert.deepEqual(rejections, []);
|
||||
assert.equal(errors.length, 1);
|
||||
assert.match(errors[0], /^Error updating whitelist:\nError: Expected state running,stopping/);
|
||||
});
|
||||
|
||||
it("should log server settings update errors and invoke the hook", async function() {
|
||||
instance.server.exampleSettings = async () => { throw new Error("no example settings"); };
|
||||
instance.config.set("factorio.settings", { name: "bar" });
|
||||
assert.equal(await hookInvoked, "factorio.settings");
|
||||
await wait(10);
|
||||
assert.deepEqual(rejections, []);
|
||||
assert.equal(errors.length, 1);
|
||||
assert.match(errors[0], /^Error updating server settings:\nError: no example settings/);
|
||||
});
|
||||
|
||||
it("should apply tags with non-string elements", async function() {
|
||||
instance.config.set("factorio.settings", { tags: [1, "a b"] });
|
||||
await hookInvoked;
|
||||
await wait(10);
|
||||
assert.deepEqual(rejections, []);
|
||||
assert.deepEqual(errors, []);
|
||||
assert.deepEqual(instance.server.rconCommands, ["/config set tags 1 a b"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe(".checkModPackVersion()", function() {
|
||||
function modPack(factorioVersion) {
|
||||
return lib.ModPack.fromJSON({ name: "pack", factorio_version: factorioVersion });
|
||||
|
||||
Reference in new issue
Block a user