diff --git a/docs/configuration.md b/docs/configuration.md index f78c609f..aab55e1f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -86,19 +86,20 @@ Defaults to null which means generate a secure secret on startup. ### master.heartbeat_interval Interval in seconds heartbeats are sent out at for WebSocket connections. -If a connection hasn't received a heartbeat in 2 times the heartbeat interval it will be considered stale and closed. +If a WebSocket connection hasn't received a heartbeat in 2 times the heartbeat interval it will be considered stale and closed. A lower value means less time between connections going stale and them being closed. -Defaults to 45. +Defaults to 15. -### master.connector_shutdown_timeout +### master.session_timeout -Timeout in seconds to use while shutting down the master server for link sessions that currently don't have an active connection and haven't been closed down properly. -This happens when the network between a slave and the master server goes down unexpectedly, or the slave goes down without closing the connection properly either do to crashing or the machine it's hosted on going down. -If the timeout expires the session is terminated and data loss may occur, if the slave went down this is unavoidable. +Time in seconds from the connection is closed following a heartbeat timeout until the session is invalidated and data loss occurs. +A higher timeout gives more time for a client to recconect and resume an active session over a connection that was dropped, but also leads to stale connections taking longer to clear out. +In the case of the master or client crashing the data loss is unavoidable. -Defaults to 30. +Value should not be less than the configured `max_reconnect_delay` of the clients and it should be greater than 2 times `master.heartbeat_interval`. +Defaults to 60. ### master.metrics_timeout @@ -197,12 +198,12 @@ This is used by plugins like Server Select to give the correct address to switch Defaults to "localhost". -### slave.reconnect_delay +### slave.max_reconnect_delay -Delay in seconds to wait after connection to the master server is dropped before attempting to reconnect to it. -The actual delay on each reconnect will be a random number between 0 and this configured value to avoid all slaves trying to reconnect at the same time. +Maximum delay in seconds to wait after connection to the master server is dropped before attempting to reconnect to it. +The actual delay on each reconnect will be a random number between 0 and this configured value to avoid all clients trying to reconnect at the same time. -Defaults to 5. +Defaults to 60. ## Instance Configuration @@ -399,8 +400,9 @@ If you have a self signed certificate on the master server you will need to copy Defaults to null meaning use Node.js's default set of trusted certificate authorities. -### control.reconnect_delay +### control.max_reconnect_delay -Duration in seconds to wait before attempting to reconnect with the master server after the connection is dropped. +Max duration in seconds to wait before attempting to reconnect with the master server after the connection is dropped. +The actual delay on each reconnect will be a random number between 0 and this configured value to avoid all clients trying to reconnect at the same time. -Defaults to 2. +Defaults to 60. diff --git a/docs/devs/protocol.md b/docs/devs/protocol.md index 39625378..d45957fe 100644 --- a/docs/devs/protocol.md +++ b/docs/devs/protocol.md @@ -99,7 +99,7 @@ The following messages are the only valid messages that can be sent during the h Sent by the master server when the WebSocket connection has been opened. - `version: string` - The version of the master, e.g. "2.0.0". -- `plugins: Object<string, string>` - +- `plugins: Object` - Object mapping plugin names to plugin version for plugins that are loaded on the master server. #### `register_slave` @@ -111,7 +111,7 @@ Slave handshake for establishing a new connection session. - `version: string` - The protocol version of the slave, e.g. "2.0.0". - `name: string` - Name of the slave. - `id: integer` - ID of the slave. -- `plugins: Object<string, string>` - +- `plugins: Object` - Object mapping plugin names to plugin version for plugins that are available on the slave. #### `register_control` diff --git a/packages/lib/factorio/export.js b/packages/lib/factorio/export.js index 2e71dd08..173f5cb3 100644 --- a/packages/lib/factorio/export.js +++ b/packages/lib/factorio/export.js @@ -2,10 +2,10 @@ "use strict"; const fs = require("fs-extra"); const path = require("path"); -const ini = require("ini"); const Jimp = require("jimp"); const JSZip = require("jszip"); +const libIni = require("../ini"); const libBuildMod = require("../build_mod"); /** @@ -312,7 +312,7 @@ async function exportLocale(server, modVersions, modOrder, languageCode) { } let baseLocaleFilePath = server.dataPath("base", "locale", languageCode, "base.cfg"); - mergeLocale(ini.parse(await fs.readFile(baseLocaleFilePath, "utf8"))); + mergeLocale(libIni.parse(await fs.readFile(baseLocaleFilePath, "utf8"))); for (let mod of modOrder) { if (["core", "base", "export"].includes(mod)) { @@ -330,7 +330,7 @@ async function exportLocale(server, modVersions, modOrder, languageCode) { } for (let file of zip.file(new RegExp(`locale\\/${languageCode}\\/.*\\.cfg`))) { let content = await file.async("nodebuffer"); - mergeLocale(ini.parse(content.toString("utf8"))); + mergeLocale(libIni.parse(content.toString("utf8"))); } } diff --git a/packages/lib/factorio/server.js b/packages/lib/factorio/server.js index 5b08447a..ec2d4766 100644 --- a/packages/lib/factorio/server.js +++ b/packages/lib/factorio/server.js @@ -7,7 +7,7 @@ const events = require("events"); const util = require("util"); const crypto = require("crypto"); -const ini = require("ini"); +const libIni = require("../ini"); const rconClient = require("rcon-client"); const libErrors = require("../errors"); const { logger } = require("../logging"); @@ -1167,7 +1167,7 @@ class FactorioServer extends events.EventEmitter { } async _writeConfigIni() { - let content = ini.encode({ + let content = libIni.stringify({ path: { "read-data": this.dataPath(), "write-data": this.writePath(), diff --git a/packages/lib/ini.js b/packages/lib/ini.js new file mode 100644 index 00000000..413aeb92 --- /dev/null +++ b/packages/lib/ini.js @@ -0,0 +1,94 @@ +/** + * Factorio Compatible INI parser + * + * @module lib/ini + * @author Hornwitser + */ + +"use strict"; + +/** + * Parse INI file into object of sections + * @param {string} input - content of INI file to parse. + * @returns {object>} + * sections parsed from the ini file. + */ +function parse(input) { + const sections = {}; + let currentSection = sections; + const lines = input.split(/\r\n|\n/); + for (let number = 1; number <= lines.length; number++) { + let line = lines[number - 1].replace(/^[\t ]+/, ""); + if (!line || line[0] === "#" || line[0] === ";") { + continue; + } + + // Read section header + if (line[0] === "[") { + line = line.replace(/[\t ]+$/, ""); + if (line.slice(-1) !== "]") { + throw new Error(`Unterminated section header on line ${number}`); + } + const name = line.slice(1, -1); + if (Object.prototype.hasOwnProperty.call(sections, name)) { + throw new Error(`Duplicated section [${name}] on line ${number}`); + } + currentSection = {}; + sections[name] = currentSection; + continue; + } + + // Read key value + const equalIndex = line.indexOf("="); + if (equalIndex === -1) { + throw new Error(`Missing value for key ${line} on line ${number}`); + } + const key = line.slice(0, equalIndex); + if (Object.prototype.hasOwnProperty.call(currentSection, key)) { + throw new Error(`Duplicated key ${key} on line ${number}`); + } + const value = line.slice(equalIndex + 1); + currentSection[key] = value; + } + + return sections; +} + +/** + * Serialize an object of section definitions into an INI file + * @param {object>} sections - + * Sections to encode into ini format + * @returns {string} sections serialized into INI format + */ +function stringify(sections) { + let serialized = []; + let first = true; + + for (let [key, value] of Object.entries(sections)) { + if (typeof value === "string") { + serialized.push(`${key}=${value}`); + first = false; + } + } + + for (let [section, content] of Object.entries(sections)) { + if (typeof content === "object") { + if (!first) { + serialized.push(""); + } + first = false; + serialized.push(`[${section}]`); + for (let [key, value] of Object.entries(content)) { + serialized.push(`${key}=${value}`); + } + } + } + + if (serialized.length) { serialized.push(""); } + return serialized.join("\n"); +} + +module.exports = { + parse, + stringify, +}; diff --git a/packages/lib/link/connectors.js b/packages/lib/link/connectors.js index e2aacdf7..dc7bc6a3 100644 --- a/packages/lib/link/connectors.js +++ b/packages/lib/link/connectors.js @@ -73,7 +73,12 @@ class WebSocketBaseConnector extends events.EventEmitter { this._check("connected"); if (Date.now() - this._lastHeartbeat > 2000 * this._heartbeatInterval) { logger.verbose("Connector | closing after heartbeat timed out"); - this._socket.close(1008, "Heartbeat timeout"); + // eslint-disable-next-line node/no-process-env + if (process.env.APP_ENV === "browser") { + this._socket.close(4008, "Heartbeat timeout"); + } else { + this._socket.terminate(); + } } else { this._socket.send(JSON.stringify({ diff --git a/packages/lib/package.json b/packages/lib/package.json index 63e9d6a3..f7b78901 100644 --- a/packages/lib/package.json +++ b/packages/lib/package.json @@ -15,7 +15,6 @@ "ajv": "^6.12.6", "chalk": "^4.1.0", "fs-extra": "^8.1.0", - "ini": "^1.3.5", "isomorphic-ws": "^4.0.1", "jimp": "^0.12.1", "jsonwebtoken": "^8.2.1", diff --git a/packages/lib/plugin.js b/packages/lib/plugin.js index 834d2c96..fa438da5 100644 --- a/packages/lib/plugin.js +++ b/packages/lib/plugin.js @@ -60,6 +60,9 @@ class BaseInstancePlugin { * `error`, `warn`, `audit`, `info` and `verbose`. */ this.logger = instance.logger.child({ plugin: this.info.name }); + + this._pendingRconMessages = []; + this._sendingRconMessages = false; } /** @@ -207,9 +210,72 @@ class BaseInstancePlugin { */ async onPlayerEvent(event) { } - async sendRcon(message, expectEmpty) { + /** + * Send RCON message to instance + * + * Send a message or command to the server which can potentially be + * executed out of order in relation to other RCON commands. + * + * This should not be called before onStart or after onStop. A simple + * way to achieve this is to check that the instance status is running + * before sending commands. Note that the instance status is set to + * running some time after `onStart` and some time before `onStop` is + * invoked. + * + * @param {string} message - message to send to server over RCON. + * @param {boolean=} [expectEmpty=false] - + * if true throw if the response is not empty. Useful for detecting + * errors that might have been sent in response. + * @returns {Promise} response from server. + */ + async sendRcon(message, expectEmpty=false) { return await this.instance.sendRcon(message, expectEmpty, this.info.name); } + + /** + * Send serially ordered RCON message to instance + * + * Send a message or command to the server which will not be executed + * out of order in relation to other RCON commands sent with this + * method. The ordering applies per plugin, and two plugins sending + * commands with this method may execute out of order in relation to + * each other. + * + * This should not be called before onStart or after onStop. A simple + * way to achieve this is to check that the instance status is running + * before sending commands. Note that the instance status is set to + * running some time after `onStart` and some time before `onStop` is + * invoked. + * + * @param {string} message - message to send to server over RCON. + * @param {boolean=} [expectEmpty=false] - + * if true throw if the response is not empty. Useful for detecting + * errors that might have been sent in response. + * @returns {Promise} response from server. + */ + async sendOrderedRcon(message, expectEmpty=false) { + let promise = new Promise((resolve, reject) => { + this._pendingRconMessages.push({resolve, reject, message, expectEmpty}); + }); + if (!this._sendingRconMessages) { + this._sendPendingRconMessages(); + } + return await promise; + } + + async _sendPendingRconMessages() { + this._sendingRconMessages = true; + while (this._pendingRconMessages.length) { + let task = this._pendingRconMessages.shift(); + try { + let result = await this.sendRcon(task.message, task.expectEmpty); + task.resolve(result); + } catch (err) { + task.reject(err); + } + } + this._sendingRconMessages = false; + } } /** diff --git a/packages/master/package.json b/packages/master/package.json index 76a2fb6d..435983a7 100644 --- a/packages/master/package.json +++ b/packages/master/package.json @@ -26,7 +26,6 @@ "fs-extra": "^8.1.0", "jsonwebtoken": "^8.2.1", "jszip": "^3.2.2", - "node-factorio-api": "^0.3.8", "set-blocking": "^2.0.0", "winston": "^3.3.3", "ws": "^7.2.3", diff --git a/packages/master/src/BaseConnection.js b/packages/master/src/BaseConnection.js index 44c086e6..e7bca6fa 100644 --- a/packages/master/src/BaseConnection.js +++ b/packages/master/src/BaseConnection.js @@ -64,6 +64,7 @@ class BaseConnection extends libLink.Link { } async disconnect(code, reason) { + this.connector.setClosing(); try { await libLink.messages.prepareDisconnect.send(this); } catch (err) { diff --git a/packages/master/src/WsServer.js b/packages/master/src/WsServer.js index 9c383976..1e1de286 100644 --- a/packages/master/src/WsServer.js +++ b/packages/master/src/WsServer.js @@ -136,7 +136,7 @@ class WsServer { let timeoutId = setTimeout(() => { logger.verbose(`WsServer | closing ${req.socket.remoteAddress} after timing out on handshake`); wsRejectedConnectionsCounter.inc(); - socket.close(1008, "Handshake timeout"); + socket.terminate(); this.pendingSockets.delete(socket); }, 30*1000); @@ -266,7 +266,6 @@ ${err.stack}` let connection = this.slaveConnections.get(data.id); if (connection) { logger.verbose(`WsServer | disconnecting existing connection for slave ${data.id}`); - connection.connector.setTimeout(15); // Slave connection is likely stalled await connection.disconnect(1008, "Registered from another connection"); } diff --git a/packages/master/src/WsServerConnector.js b/packages/master/src/WsServerConnector.js index bd58003d..1db846cb 100644 --- a/packages/master/src/WsServerConnector.js +++ b/packages/master/src/WsServerConnector.js @@ -73,19 +73,10 @@ class WsServerConnector extends libLink.WebSocketBaseConnector { continue(socket, lastSeq) { // It's possible the previous connection hasn't closed yet due to a - // stale connection. If this is the case the connector will have to - // wait for the previous socket to close down before it can continue - // from a new socket, or there will be overlapping events from both. + // stale connection. Terminate it if so. if (this._state === "connected") { - this._socket.close(4003, "Session Hijacked"); - - // Correctly waiting for the connector to close and then - // handling all the edge cases like multiple continues happening - // in parallel or connections going stale is far too difficult. - // Kill the connection here and let the client reconnect later - // when the connector is not held up by an active connection. - socket.close(1013, "Session Busy"); - socket.terminate(); + this._socket.terminate(); + this._socket.once("close", () => this.continue(socket, lastSeq)); return; } diff --git a/packages/master/src/routes.js b/packages/master/src/routes.js index 0310d001..749b2b55 100644 --- a/packages/master/src/routes.js +++ b/packages/master/src/routes.js @@ -8,6 +8,7 @@ const jwt = require("jsonwebtoken"); const path = require("path"); const util = require("util"); +const libErrors = require("@clusterio/lib/errors"); const libHelpers = require("@clusterio/lib/helpers"); const libLink = require("@clusterio/lib/link"); const libPlugin = require("@clusterio/lib/plugin"); @@ -47,7 +48,18 @@ async function getMetrics(req, res, next) { let requests = []; let timeout = req.app.locals.master.config.get("master.metrics_timeout") * 1000; for (let slaveConnection of req.app.locals.master.wsServer.slaveConnections.values()) { - requests.push(libHelpers.timeout(libLink.messages.getMetrics.send(slaveConnection), timeout, null)); + if (!slaveConnection.connected) { + continue; + } + requests.push(libHelpers.timeout( + libLink.messages.getMetrics.send(slaveConnection).catch(err => { + if (!(err instanceof libErrors.SessionLost)) { + logger.error(`Unexpected error gathering metrics from slave:\n${err.stack}`); + } + return null; + }), + timeout, null + )); } for await (let result of await libPrometheus.defaultRegistry.collect()) { diff --git a/packages/web_ui/src/components/InstancesPage.jsx b/packages/web_ui/src/components/InstancesPage.jsx index e7020a05..85996fe2 100644 --- a/packages/web_ui/src/components/InstancesPage.jsx +++ b/packages/web_ui/src/components/InstancesPage.jsx @@ -11,6 +11,8 @@ import { useInstanceList } from "../model/instance"; import { notifyErrorHandler } from "../util/notify"; import { useSlaveList } from "../model/slave"; +const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; + function CreateInstanceButton(props) { let control = useContext(ControlContext); @@ -64,6 +66,14 @@ export default function InstancesPage() { let [slaveList] = useSlaveList(); let [instanceList] = useInstanceList(); + function slaveName(slaveId) { + let slave = slaveList.find(s => s.id === slaveId); + if (slave) { + return slave.name; + } + return String(slaveId); + } + return strcmp(a["name"], b["name"]), }, { title: "Assigned Slave", key: "assigned_slave", - render: instance => { - let slave = slaveList.find(s => s.id === instance.assigned_slave); - if (slave) { - return slave.name; - } - return instance.assigned_slave; - }, + render: instance => slaveName(instance["assigned_slave"]), + sorter: (a, b) => strcmp(slaveName(a["assigned_slave"]), slaveName(b["assigned_slave"])), }, { title: "Status", dataIndex: "status", + sorter: (a, b) => strcmp(a["status"], b["status"]), }, ]} dataSource={instanceList} diff --git a/packages/web_ui/src/components/PluginsPage.jsx b/packages/web_ui/src/components/PluginsPage.jsx index e93483bc..3c3c5a46 100644 --- a/packages/web_ui/src/components/PluginsPage.jsx +++ b/packages/web_ui/src/components/PluginsPage.jsx @@ -6,6 +6,8 @@ import notify from "../util/notify"; import ControlContext from "./ControlContext"; import PageLayout from "./PageLayout"; +const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; + export default function PluginsPage() { let plugins = useContext(ControlContext).plugins; @@ -47,6 +49,8 @@ export default function PluginsPage() { title: "Name", key: "name", render: plugin => (plugin.info ? plugin.info.title : plugin.meta.name), + defaultSortOrder: "ascend", + sorter: (a, b) => strcmp(a.info ? a.info.title : a.meta.name, b.info ? b.info.title : b.meta.name), }, { title: "Version", @@ -60,11 +64,13 @@ export default function PluginsPage() { } return plugin.package.version; }, + sorter: (a, b) => strcmp(a.meta.version, b.meta.version), }, { title: "Loaded", dataIndex: ["meta", "loaded"], render: loaded => (loaded ? "Yes" : null), + sorter: (a, b) => a.loaded - b.loaded, responsive: ["sm"], }, ]} diff --git a/packages/web_ui/src/components/RolesPage.jsx b/packages/web_ui/src/components/RolesPage.jsx index 388a48f1..fc1ba59a 100644 --- a/packages/web_ui/src/components/RolesPage.jsx +++ b/packages/web_ui/src/components/RolesPage.jsx @@ -9,6 +9,8 @@ import ControlContext from "./ControlContext"; import { notifyErrorHandler } from "../util/notify"; import PageLayout from "./PageLayout"; +const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; + function CreateRoleButton() { let control = useContext(ControlContext); @@ -81,6 +83,7 @@ export default function RolesPage() { { title: "Name", dataIndex: "name", + sorter: (a, b) => strcmp(a["name"], b["name"]), }, { title: "Description", diff --git a/packages/web_ui/src/components/SlavesPage.jsx b/packages/web_ui/src/components/SlavesPage.jsx index 15a83f3b..f94001a8 100644 --- a/packages/web_ui/src/components/SlavesPage.jsx +++ b/packages/web_ui/src/components/SlavesPage.jsx @@ -9,6 +9,8 @@ import PageLayout from "./PageLayout"; import { useSlaveList } from "../model/slave"; import { notifyErrorHandler } from "../util/notify"; +const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; + function GenerateSlaveTokenButton(props) { let control = useContext(ControlContext); @@ -86,19 +88,25 @@ export default function SlavesPage() { { title: "Name", dataIndex: "name", + defaultSortOrder: "ascend", + sorter: (a, b) => strcmp(a["name"], b["name"]), }, { title: "Agent", dataIndex: "agent", + sorter: (a, b) => strcmp(a["agent"], b["agent"]), + responsive: ["lg"], }, { title: "Version", dataIndex: "version", + sorter: (a, b) => strcmp(a["version"], b["version"]), }, { title: "Connected", key: "connected", render: slave => slave["connected"] && "Yes", + sorter: (a, b) => a["connected"] - b["connected"], }, ]} dataSource={slaveList} diff --git a/packages/web_ui/src/components/UsersPage.jsx b/packages/web_ui/src/components/UsersPage.jsx index c8f98376..296fd1ce 100644 --- a/packages/web_ui/src/components/UsersPage.jsx +++ b/packages/web_ui/src/components/UsersPage.jsx @@ -9,6 +9,8 @@ import ControlContext from "./ControlContext"; import { notifyErrorHandler } from "../util/notify"; import PageLayout from "./PageLayout"; +const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; + function CreateUserButton() { let control = useContext(ControlContext); @@ -80,6 +82,8 @@ export default function UsersPage() { { title: "Name", dataIndex: "name", + defaultSortOrder: "ascend", + sorter: (a, b) => strcmp(a["name"], b["name"]), }, { title: "Roles", @@ -90,18 +94,21 @@ export default function UsersPage() { title: "Admin", key: "admin", render: user => user["is_admin"] && "yes", + sorter: (a, b) => a["is_admin"] - b["is_admin"], responsive: ["lg"], }, { title: "Whitelisted", key: "whitelisted", render: user => user["is_whitelisted"] && "yes", + sorter: (a, b) => a["is_whitelisted"] - b["is_whitelisted"], responsive: ["lg"], }, { title: "Banned", key: "banned", render: user => user["is_banned"] && "yes", + sorter: (a, b) => a["is_banned"] - b["is_banned"], responsive: ["lg"], }, ]} diff --git a/plugins/inventory_sync/module/upload_inventory.lua b/plugins/inventory_sync/module/upload_inventory.lua index d1c4cf23..5ab6051e 100644 --- a/plugins/inventory_sync/module/upload_inventory.lua +++ b/plugins/inventory_sync/module/upload_inventory.lua @@ -3,6 +3,18 @@ local serialize = require("modules/inventory_sync/serialize") local save_crafts = require("modules/inventory_sync/save_crafts") local function upload_inventory(player, player_record) + -- Editor mode is not supported. + if player.controller_type == defines.controllers.editor then + player.toggle_map_editor() + end + + if player.controller_type == defines.controllers.editor then + log("ERROR: Inventory sync failed to upload, unable to switch " .. player.name .. " out of editor mode") + player.print("ERROR: Inventory sync failed to upload, unable to switch out of editor mode") + global.inventory_sync.active_uploads[player.name] = nil + return + end + local serialized_player = serialize.serialize_player(player) serialized_player.generation = player_record.generation diff --git a/plugins/research_sync/instance.js b/plugins/research_sync/instance.js index 0da6c858..3f58bf2c 100644 --- a/plugins/research_sync/instance.js +++ b/plugins/research_sync/instance.js @@ -19,6 +19,8 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin { this.instance.server.on("ipc-research_sync:finished", (tech) => { this.researchFinished(tech).catch(err => this.unexpectedError(err)); }); + + this.syncStarted = false; } async researchContribution(tech) { @@ -26,8 +28,11 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin { } async progressEventHandler(message) { + if (!this.syncStarted || !["starting", "running"].includes(this.instance.status)) { + return; + } let techsJson = libLuaTools.escapeString(JSON.stringify(message.data.technologies)); - await this.sendRcon(`/sc research_sync.update_progress("${techsJson}")`, true); + await this.sendOrderedRcon(`/sc research_sync.update_progress("${techsJson}")`, true); } async researchFinished(tech) { @@ -35,14 +40,17 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin { } async finishedEventHandler(message) { + if (!this.syncStarted || !["starting", "running"].includes(this.instance.status)) { + return; + } let { name, level, researched } = message.data; - await this.sendRcon( + await this.sendOrderedRcon( `/sc research_sync.research_technology("${libLuaTools.escapeString(name)}", ${level})`, true ); } async onStart() { - let dumpJson = await this.sendRcon("/sc research_sync.dump_technologies()"); + let dumpJson = await this.sendOrderedRcon("/sc research_sync.dump_technologies()"); let techsToSend = []; let instanceTechs = new Map(); for (let tech of JSON.parse(dumpJson)) { @@ -56,6 +64,7 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin { } let response = await this.info.messages.syncTechnologies.send(this.instance, { technologies: techsToSend }); + this.syncStarted = true; let techsToSync = []; for (let masterTech of response.technologies) { let [name, level, progress, researched] = masterTech; @@ -72,7 +81,7 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin { if (techsToSync.length) { let syncJson = libLuaTools.escapeString(JSON.stringify(techsToSync)); - await this.sendRcon(`/sc research_sync.sync_technologies("${syncJson}")`, true); + await this.sendOrderedRcon(`/sc research_sync.sync_technologies("${syncJson}")`, true); } } } diff --git a/plugins/statistics_exporter/instance.js b/plugins/statistics_exporter/instance.js index db40dcbd..7d98f5eb 100644 --- a/plugins/statistics_exporter/instance.js +++ b/plugins/statistics_exporter/instance.js @@ -77,6 +77,10 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin { } async onMetrics() { + if (this.instance.status !== "running") { + return; + } + // Wait configured timeout for the metrics to be collected. It may // take a long time for the command to go through if the command // stream is overloaded. Should the timeout be exceeded the diff --git a/plugins/statistics_exporter/test/plugin.js b/plugins/statistics_exporter/test/plugin.js index 50a28c88..56a16b5d 100644 --- a/plugins/statistics_exporter/test/plugin.js +++ b/plugins/statistics_exporter/test/plugin.js @@ -17,7 +17,14 @@ describe("statistics_exporter plugin", function() { }); describe(".onMetrics()", function() { + it("should not send commands before running", async function() { + instancePlugin.instance.server.rconCommands.length = 0; + instancePlugin.instance.status = "starting"; + await instancePlugin.onMetrics(); + assert(instancePlugin.instance.server.rconCommands.length === 0, "commands were run"); + }); it("should record statistics", async function() { + instancePlugin.instance.status = "running"; instancePlugin.instance.server.rconCommandResults.set( "/sc statistics_exporter.export()", JSON.stringify({ diff --git a/test/lib/ini.js b/test/lib/ini.js new file mode 100644 index 00000000..846e614c --- /dev/null +++ b/test/lib/ini.js @@ -0,0 +1,93 @@ +"use strict"; +const assert = require("assert").strict; +const libIni = require("@clusterio/lib/ini"); + +describe("lib/ini", function() { + describe("parse()", function() { + it("should ignore comments", function() { + assert.deepEqual( + libIni.parse("# A comment\n; Another comment\nfoo=bar\n"), + { foo: "bar" }, + ); + }); + it("should ignore certain whitespace", function() { + assert.deepEqual( + libIni.parse(" # A comment \n\n [spam] \t \n \tfoo=bar\n"), + { spam: { foo: "bar" }} + ); + }); + it("should preserve certain whitespace", function() { + assert.deepEqual( + libIni.parse("[\t spam\t ]\nfoo =\t bar \n"), + { "\t spam\t ": { "foo ": "\t bar " }} + ); + }); + it("should not treat characters specially", function() { + assert.deepEqual( + libIni.parse("[#;spam]\nfoo#;=#;bar=#;'\""), + { "#;spam": { "foo#;": "#;bar=#;'\"" }} + ); + }); + it("should handle windows line endings", function() { + assert.deepEqual( + libIni.parse("[spam]\r\nfoo=bar\r\n"), + { "spam": { "foo": "bar" }} + ); + }); + it("should throw on incomplete section", function() { + assert.throws( + () => libIni.parse("[spam"), + new Error("Unterminated section header on line 1") + ); + }); + it("should throw on missing value", function() { + assert.throws( + () => libIni.parse("key"), + new Error("Missing value for key key on line 1") + ); + }); + it("should throw on duplicated section", function() { + assert.throws( + () => libIni.parse("[spam]\n[spam]\n"), + new Error("Duplicated section [spam] on line 2") + ); + }); + it("should throw on duplicated keys", function() { + assert.throws( + () => libIni.parse("key=value\nkey=value\n"), + new Error("Duplicated key key on line 2") + ); + }); + }); + describe("stringify()", function() { + it("should serialize to reference output", function() { + let cases = [ + [{}, ""], + [{ foo: "bar" }, "foo=bar\n"], + [{ foo: { spam: "one", bar: "two" }}, "[foo]\nspam=one\nbar=two\n"], + [ + { start: "alpha", foo: { spam: "one", bar: "two" }, end: "beta" }, + "start=alpha\nend=beta\n\n[foo]\nspam=one\nbar=two\n", + ], + [{ "trail ": "\t tabs \t" }, "trail =\t tabs \t\n"], + [{ "#;spam": { "foo#;": "#;bar=#;'\"" }}, "[#;spam]\nfoo#;=#;bar=#;'\"\n"], + ]; + for (let [input, expected] of cases) { + assert.equal(libIni.stringify(input), expected); + } + }); + }); + it("should round trip object", function() { + for (let expected of [ + {}, + { simple: "value" }, + { section: { foo: "false", bar: "text" }}, + { mixed: "root", section: { foo: "1", bar: "2" }, end: "value" }, + { first: { foo: "1", bar: "2" }, second: { spam: "true" }}, + { '"#;[] ': ' "#;[] ', ' #;"[ ': { '"#;[] ': ' "#;[ ' }}, + ]) { + let actual = libIni.parse(libIni.stringify(expected)); + assert.deepEqual(actual, expected); + } + }); +}); diff --git a/test/lib/plugin.js b/test/lib/plugin.js index 6ea1d9ac..55159f4f 100644 --- a/test/lib/plugin.js +++ b/test/lib/plugin.js @@ -10,7 +10,7 @@ describe("lib/plugin", function() { describe("class BaseInstancePlugin", function() { let instancePlugin; it("should be constructible", async function() { - instancePlugin = new libPlugin.BaseInstancePlugin({}, { logger: new mock.MockLogger() }, {}); + instancePlugin = new libPlugin.BaseInstancePlugin({}, new mock.MockInstance(), {}); await instancePlugin.init(); }); it("should define defaults for hooks", async function() { @@ -22,6 +22,36 @@ describe("lib/plugin", function() { instancePlugin.onMasterConnectionEvent("connect"); await instancePlugin.onPrepareMasterDisconnect(); }); + describe("sendRcon", function() { + it("should send commands out of order", async function() { + instancePlugin.instance.server.rconCommandResults.set("a", { time: 100, response: "a" }); + instancePlugin.instance.server.rconCommandResults.set("b", { time: 50, response: "b" }); + + let a = instancePlugin.sendRcon("a"); + let b = instancePlugin.sendRcon("b"); + let result = await Promise.race([a, b]); + assert.equal(result, "b"); + }); + it("should propagate errors", async function() { + instancePlugin.instance.server.rconCommandResults.set("a", new Error("ref")); + await assert.rejects(instancePlugin.sendRcon("a"), new Error("ref")); + }); + }); + describe("sendOrderedRcon", function() { + it("should send commands in order", async function() { + instancePlugin.instance.server.rconCommandResults.set("a", { time: 100, response: "a" }); + instancePlugin.instance.server.rconCommandResults.set("b", { time: 50, response: "b" }); + + let a = instancePlugin.sendOrderedRcon("a"); + let b = instancePlugin.sendOrderedRcon("b"); + let result = await Promise.race([a, b]); + assert.equal(result, "a"); + }); + it("should propagate errors", async function() { + instancePlugin.instance.server.rconCommandResults.set("a", new Error("ref")); + await assert.rejects(instancePlugin.sendOrderedRcon("a"), new Error("ref")); + }); + }); }); describe("class BaseMasterPlugin", function() { diff --git a/test/mock.js b/test/mock.js index 0787c403..c66b3d0b 100644 --- a/test/mock.js +++ b/test/mock.js @@ -3,6 +3,7 @@ const events = require("events"); const http = require("http"); const express = require("express"); +const libHelpers = require("@clusterio/lib/helpers"); const libLink = require("@clusterio/lib/link"); const libPlugin = require("@clusterio/lib/plugin"); const libUsers = require("@clusterio/lib/users"); @@ -105,7 +106,20 @@ class MockServer extends events.EventEmitter { async sendRcon(command) { this.rconCommands.push(command); - return this.rconCommandResults.get(command) || ""; + let result = this.rconCommandResults.get(command); + if (!result) { + result = { response: "" }; + } + if (result instanceof Error || typeof result === "string") { + result = { response: result }; + } + if (result.time) { + await libHelpers.wait(result.time); + } + if (result.response instanceof Error) { + throw result.response; + } + return result.response || ""; } } @@ -116,6 +130,7 @@ class MockInstance extends libLink.Link { this.server = new MockServer(); this.name = "test"; this.id = 7357; + this.status = "running"; this.mockConfigEntries = new Map([ ["instance.id", 7357], ["factorio.enable_save_patching", true],