From a4efe3f11f0c4a7ece0d32a63af325c39a7e86ee Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:33:12 +0200 Subject: [PATCH 1/4] refactor(utils): split config check into helpers and orchestrator --- js/utils.js | 100 +++++++++++++++++++------------ tests/unit/classes/utils_spec.js | 16 +++-- 2 files changed, 70 insertions(+), 46 deletions(-) diff --git a/js/utils.js b/js/utils.js index be0aeb7ef8..8fd82d888d 100644 --- a/js/utils.js +++ b/js/utils.js @@ -184,22 +184,14 @@ const loadConfig = () => { }; /** - * Checks the config file using eslint. - * @param {object} configObject the configuration object + * Runs the ESLint checks configured for the config file. + * @param {string} configFileName - The filename shown in lint messages. + * @param {string} configFileContent - The config file content to lint. + * @returns {Array} ESLint messages for linting problems. */ -const checkConfigFile = (configObject) => { - let configObj = configObject; - if (!configObj) configObj = loadConfig(); - const configFileName = configObj.configFilename; - - // Validate syntax of the configuration file. - Log.info(`Checking config file ${configFileName} ...`); - - // I'm not sure if all ever is utf-8 - const configFile = configObj.configContentFull; - - const errors = linter.verify( - configFile, +function lintConfigFile (configFileName, configFileContent) { + return linter.verify( + configFileContent, { languageOptions: { ecmaVersion: "latest", @@ -215,20 +207,22 @@ const checkConfigFile = (configObject) => { }, configFileName ); +} - if (errors.length === 0) { - Log.info(styleText("green", "Your configuration file doesn't contain syntax errors :)")); - validateModulePositions(configObj.fullConf); - } else { - let errorMessage = "Your configuration file contains syntax errors :("; +/** + * Formats ESLint messages for the config syntax error output. + * @param {Array} errors - ESLint messages returned by `linter.verify`. + * @returns {string} A user-facing error message. + */ +function formatConfigSyntaxErrors (errors) { + let errorMessage = "Your configuration file contains syntax errors :("; - for (const error of errors) { - errorMessage += `\nLine ${error.line} column ${error.column}: ${error.message}`; - } - Log.error(errorMessage); - throw new ConfigError(""); + for (const error of errors) { + errorMessage += `\nLine ${error.line} column ${error.column}: ${error.message}`; } -}; + + return errorMessage; +} /** * Validates the modules array in the config object. @@ -237,38 +231,44 @@ const checkConfigFile = (configObject) => { * - every entry has a `module` property of type string * - every entry's `position` (if set) is a known region from index.html * - * Unknown positions produce a warning; structural errors are fatal. + * Unknown positions produce a warning; structural errors throw an error so the + * outer validation flow can decide how to report and terminate. * @param {object} data - The full config object to validate. + * @throws {Error} When the modules structure is invalid. */ -const validateModulePositions = (data) => { +function validateModulePositions (data) { Log.info("Checking modules structure configuration ..."); const positionList = getModulePositions(); // `modules` always exists (defaults.js provides a default array), but guard against it being overridden with a non-array value if (data.modules !== undefined && !Array.isArray(data.modules)) { - Log.error("This module configuration contains errors:\nmodules must be an array"); - throw new ConfigError(""); + const errorMessage = "This module configuration contains errors:\nmodules must be an array"; + Log.error(errorMessage); + throw new ConfigError(errorMessage); } // Validate each module entry for (const [index, mod] of (data.modules ?? []).entries()) { // Each module entry must be an object so we can safely inspect its fields if (mod === null || typeof mod !== "object" || Array.isArray(mod)) { - Log.error(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule entry must be an object`); - throw new ConfigError(""); + const errorMessage = `This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule entry must be an object`; + Log.error(errorMessage); + throw new ConfigError(errorMessage); } // `module` (the module name) is required and must be a string if (typeof mod.module !== "string") { - Log.error(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule: must be a string`); - throw new ConfigError(""); + const errorMessage = `This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule: must be a string`; + Log.error(errorMessage); + throw new ConfigError(errorMessage); } // `position` is optional, but must be a string when provided if (mod.position !== undefined && typeof mod.position !== "string") { - Log.error(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nposition: must be a string`); - throw new ConfigError(""); + const errorMessage = `This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nposition: must be a string`; + Log.error(errorMessage); + throw new ConfigError(errorMessage); } // `position` is optional, but when set it must match a known region @@ -279,6 +279,32 @@ const validateModulePositions = (data) => { } Log.info(styleText("green", "Your modules structure configuration doesn't contain errors :)")); -}; +} + +/** + * Checks the config file by orchestrating syntax and structure validation. + * + * Syntax errors and invalid module structures are fatal. Unknown module + * positions remain warnings only. + * @param {object} [configObject] - The loaded config object. When omitted, the config is loaded first. + */ +function checkConfigFile (configObject) { + let configObj = configObject; + if (!configObj) configObj = loadConfig(); + const configFileName = configObj.configFilename; + + // Validate syntax of the configuration file first. + Log.info(`Checking config file ${configFileName} ...`); + + const errors = lintConfigFile(configFileName, configObj.configContentFull); + + if (errors.length === 0) { + Log.info(styleText("green", "Your configuration file doesn't contain syntax errors :)")); + validateModulePositions(configObj.fullConf); + } else { + Log.error(formatConfigSyntaxErrors(errors)); + throw new ConfigError(""); + } +} module.exports = { loadConfig, getModulePositions, moduleHasValidPosition, getAvailableModulePositions, checkConfigFile, ConfigError }; diff --git a/tests/unit/classes/utils_spec.js b/tests/unit/classes/utils_spec.js index ee90e880fe..1c293b188d 100644 --- a/tests/unit/classes/utils_spec.js +++ b/tests/unit/classes/utils_spec.js @@ -13,14 +13,11 @@ const runCheck = (modules) => { checkConfigFile(createConfigObject(modules)); }; -const expectExitForModules = (modules) => { - vi.spyOn(process, "exit").mockImplementation(() => { - throw new ConfigError(""); - }); - +const expectConfigErrorForModules = (modules) => { expect(() => { runCheck(modules); }).toThrow(ConfigError); + expect(process.exit).not.toHaveBeenCalled(); }; describe("utils", () => { @@ -41,6 +38,7 @@ describe("utils", () => { vi.spyOn(Log, "info").mockImplementation(() => {}); vi.spyOn(Log, "warn").mockImplementation(() => {}); vi.spyOn(Log, "error").mockImplementation(() => {}); + vi.spyOn(process, "exit").mockImplementation(() => {}); }); afterEach(() => { @@ -57,13 +55,13 @@ describe("utils", () => { expect(Log.error).not.toHaveBeenCalled(); }); - it("exits when modules is not an array", () => { - expectExitForModules("not-an-array"); + it("throws when modules is not an array", () => { + expectConfigErrorForModules("not-an-array"); expect(Log.error).toHaveBeenCalledWith("This module configuration contains errors:\nmodules must be an array"); }); - it("exits when module field is missing or not a string", () => { - expectExitForModules([{ module: 123, position: "top_bar" }]); + it("throws when module field is missing or not a string", () => { + expectConfigErrorForModules([{ module: 123, position: "top_bar" }]); expect(Log.error).toHaveBeenCalled(); expect(Log.error.mock.calls[0][0]).toContain("module: must be a string"); }); From 0cc6157df71133b21e212b8efbb22cf690dee82c Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Sat, 29 Aug 2026 21:35:20 +0200 Subject: [PATCH 2/4] refactor(utils): document top-level helper functions --- js/utils.js | 72 +++++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/js/utils.js b/js/utils.js index 8fd82d888d..a3f5cffff6 100644 --- a/js/utils.js +++ b/js/utils.js @@ -21,23 +21,38 @@ class ConfigError extends Error { } } +/** + * Executes CommonJS source code from a string and returns its exports. + * @param {string} src - JavaScript source code. + * @returns {object} The exported module value. + */ const requireFromString = (src) => { const m = new module.constructor(); m._compile(src, ""); return m.exports; }; -// return all available module positions +/** + * Returns all discovered module positions. + * @returns {Array} Known module positions. + */ const getAvailableModulePositions = () => { return modulePositions; }; -// return if position is on modulePositions Array (true/false) +/** + * Checks whether the provided module position exists. + * @param {string} position - Candidate module position. + * @returns {boolean} True when the position is known. + */ const moduleHasValidPosition = (position) => { - if (getAvailableModulePositions().indexOf(position) === -1) return false; - return true; + return getAvailableModulePositions().includes(position); }; +/** + * Discovers module positions from index.html and caches them. + * @returns {Array} Discovered module positions. + */ const getModulePositions = () => { // if not already discovered if (modulePositions.length === 0) { @@ -172,14 +187,13 @@ const loadConfig = () => { return configObj; } catch (error) { + let errorMessage = `Cannot access config file: ${configFilename}\n${error.message}`; if (error.code === "ENOENT") { - Log.error(`Could not find config file: ${configFilename}`); + errorMessage = `Could not find config file: ${configFilename}`; } else if (error.code === "EACCES") { - Log.error(`No permission to read config file: ${configFilename}`); - } else { - Log.error(`Cannot access config file: ${configFilename}\n${error.message}`); + errorMessage = `No permission to read config file: ${configFilename}`; } - throw new ConfigError(""); + throw new ConfigError(errorMessage); } }; @@ -189,7 +203,7 @@ const loadConfig = () => { * @param {string} configFileContent - The config file content to lint. * @returns {Array} ESLint messages for linting problems. */ -function lintConfigFile (configFileName, configFileContent) { +const lintConfigFile = (configFileName, configFileContent) => { return linter.verify( configFileContent, { @@ -207,14 +221,14 @@ function lintConfigFile (configFileName, configFileContent) { }, configFileName ); -} +}; /** * Formats ESLint messages for the config syntax error output. * @param {Array} errors - ESLint messages returned by `linter.verify`. * @returns {string} A user-facing error message. */ -function formatConfigSyntaxErrors (errors) { +const formatConfigSyntaxErrors = (errors) => { let errorMessage = "Your configuration file contains syntax errors :("; for (const error of errors) { @@ -222,7 +236,7 @@ function formatConfigSyntaxErrors (errors) { } return errorMessage; -} +}; /** * Validates the modules array in the config object. @@ -234,41 +248,33 @@ function formatConfigSyntaxErrors (errors) { * Unknown positions produce a warning; structural errors throw an error so the * outer validation flow can decide how to report and terminate. * @param {object} data - The full config object to validate. - * @throws {Error} When the modules structure is invalid. + * @throws {ConfigError} When the modules structure is invalid. */ -function validateModulePositions (data) { +const validateModulePositions = (data) => { Log.info("Checking modules structure configuration ..."); const positionList = getModulePositions(); // `modules` always exists (defaults.js provides a default array), but guard against it being overridden with a non-array value if (data.modules !== undefined && !Array.isArray(data.modules)) { - const errorMessage = "This module configuration contains errors:\nmodules must be an array"; - Log.error(errorMessage); - throw new ConfigError(errorMessage); + throw new ConfigError("This module configuration contains errors:\nmodules must be an array"); } // Validate each module entry for (const [index, mod] of (data.modules ?? []).entries()) { // Each module entry must be an object so we can safely inspect its fields if (mod === null || typeof mod !== "object" || Array.isArray(mod)) { - const errorMessage = `This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule entry must be an object`; - Log.error(errorMessage); - throw new ConfigError(errorMessage); + throw new ConfigError(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule entry must be an object`); } // `module` (the module name) is required and must be a string if (typeof mod.module !== "string") { - const errorMessage = `This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule: must be a string`; - Log.error(errorMessage); - throw new ConfigError(errorMessage); + throw new ConfigError(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule: must be a string`); } // `position` is optional, but must be a string when provided if (mod.position !== undefined && typeof mod.position !== "string") { - const errorMessage = `This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nposition: must be a string`; - Log.error(errorMessage); - throw new ConfigError(errorMessage); + throw new ConfigError(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nposition: must be a string`); } // `position` is optional, but when set it must match a known region @@ -279,7 +285,7 @@ function validateModulePositions (data) { } Log.info(styleText("green", "Your modules structure configuration doesn't contain errors :)")); -} +}; /** * Checks the config file by orchestrating syntax and structure validation. @@ -287,8 +293,9 @@ function validateModulePositions (data) { * Syntax errors and invalid module structures are fatal. Unknown module * positions remain warnings only. * @param {object} [configObject] - The loaded config object. When omitted, the config is loaded first. + * @throws {ConfigError} When the configuration contains fatal errors. */ -function checkConfigFile (configObject) { +const checkConfigFile = (configObject) => { let configObj = configObject; if (!configObj) configObj = loadConfig(); const configFileName = configObj.configFilename; @@ -302,9 +309,8 @@ function checkConfigFile (configObject) { Log.info(styleText("green", "Your configuration file doesn't contain syntax errors :)")); validateModulePositions(configObj.fullConf); } else { - Log.error(formatConfigSyntaxErrors(errors)); - throw new ConfigError(""); + throw new ConfigError(formatConfigSyntaxErrors(errors)); } -} +}; -module.exports = { loadConfig, getModulePositions, moduleHasValidPosition, getAvailableModulePositions, checkConfigFile, ConfigError }; +module.exports = { loadConfig, getModulePositions, moduleHasValidPosition, checkConfigFile, ConfigError }; From 8a7fc39918ca61e603e507fb87f18886966aa3a1 Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:40:38 +0200 Subject: [PATCH 3/4] refactor(utils): centralize config error logging --- js/app.js | 9 +++++---- js/check_config.js | 8 ++++++-- tests/unit/classes/utils_spec.js | 26 ++++++++++++++------------ 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/js/app.js b/js/app.js index 496a296041..be976873b7 100644 --- a/js/app.js +++ b/js/app.js @@ -255,10 +255,11 @@ function App () { Log.log("Sockets connected & modules started ..."); return global.config; - } catch (err) { - // planned ConfigErrors already logged their message before throwing - if (!(err instanceof ConfigError)) { - Log.error("Unexpected error during startup:", err); + } catch (error) { + if (error instanceof ConfigError) { + Log.error(error.message); + } else { + Log.error("Unexpected error during startup:", error); } const int32 = new Int32Array(new SharedArrayBuffer(4)); diff --git a/js/check_config.js b/js/check_config.js index 3ed042c10b..20e854d7db 100644 --- a/js/check_config.js +++ b/js/check_config.js @@ -10,7 +10,11 @@ const Utils = require(`${rootPath}/js/utils.js`); try { Utils.checkConfigFile(); } catch (error) { - const message = error && error.message ? error.message : error; - Log.error(`Unexpected error: ${message}`); + if (error instanceof Utils.ConfigError) { + Log.error(error.message); + } else { + const message = error && error.message ? error.message : error; + Log.error(`Unexpected error: ${message}`); + } process.exit(1); } diff --git a/tests/unit/classes/utils_spec.js b/tests/unit/classes/utils_spec.js index 1c293b188d..1960546ab8 100644 --- a/tests/unit/classes/utils_spec.js +++ b/tests/unit/classes/utils_spec.js @@ -3,14 +3,14 @@ const fs = require("node:fs"); const Log = require("../../../js/logger"); const { checkConfigFile, ConfigError } = require("../../../js/utils"); -const createConfigObject = (modules) => ({ +const createConfigObject = (modules, configContentFull = "module.exports = { modules: [] };") => ({ configFilename: "config.js", - configContentFull: "module.exports = { modules: [] };", + configContentFull, fullConf: { modules } }); -const runCheck = (modules) => { - checkConfigFile(createConfigObject(modules)); +const runCheck = (modules, configContentFull) => { + checkConfigFile(createConfigObject(modules, configContentFull)); }; const expectConfigErrorForModules = (modules) => { @@ -57,25 +57,27 @@ describe("utils", () => { it("throws when modules is not an array", () => { expectConfigErrorForModules("not-an-array"); - expect(Log.error).toHaveBeenCalledWith("This module configuration contains errors:\nmodules must be an array"); + expect(Log.error).not.toHaveBeenCalled(); }); it("throws when module field is missing or not a string", () => { expectConfigErrorForModules([{ module: 123, position: "top_bar" }]); - expect(Log.error).toHaveBeenCalled(); - expect(Log.error.mock.calls[0][0]).toContain("module: must be a string"); + expect(Log.error).not.toHaveBeenCalled(); }); it("warns for unknown positions without exiting", () => { - const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => { - throw new ConfigError(""); - }); - expect(() => { runCheck([{ module: "clock", position: "made_up_region" }]); }).not.toThrow(); - expect(exitSpy).not.toHaveBeenCalled(); + expect(process.exit).not.toHaveBeenCalled(); expect(Log.warn).toHaveBeenCalled(); expect(Log.warn.mock.calls[0][0]).toContain("uses unknown position"); }); + + it("throws syntax errors with their lint details", () => { + expect(() => { + runCheck([], "module.exports = { modules: [ };"); + }).toThrow(/Your configuration file contains syntax errors/); + expect(process.exit).not.toHaveBeenCalled(); + }); }); From 5a9e37e122c40de711091e7673abadf779f2b901 Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Tue, 1 Sep 2026 00:49:40 +0200 Subject: [PATCH 4/4] refactor(utils): remove redundant position getter --- js/utils.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/js/utils.js b/js/utils.js index a3f5cffff6..6fb1cdc0a8 100644 --- a/js/utils.js +++ b/js/utils.js @@ -32,21 +32,13 @@ const requireFromString = (src) => { return m.exports; }; -/** - * Returns all discovered module positions. - * @returns {Array} Known module positions. - */ -const getAvailableModulePositions = () => { - return modulePositions; -}; - /** * Checks whether the provided module position exists. * @param {string} position - Candidate module position. * @returns {boolean} True when the position is known. */ const moduleHasValidPosition = (position) => { - return getAvailableModulePositions().includes(position); + return getModulePositions().includes(position); }; /**