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/js/utils.js b/js/utils.js index be0aeb7ef8..4793046e99 100644 --- a/js/utils.js +++ b/js/utils.js @@ -21,24 +21,31 @@ class ConfigError extends Error { } } -const requireFromString = (src) => { +/** + * Executes CommonJS source code from a string and returns its exports. + * @param {string} src - JavaScript source code. + * @returns {object} The exported module value. + */ +function requireFromString (src) { const m = new module.constructor(); m._compile(src, ""); return m.exports; -}; - -// return all available module positions -const getAvailableModulePositions = () => { - return modulePositions; -}; +} -// return if position is on modulePositions Array (true/false) -const moduleHasValidPosition = (position) => { - if (getAvailableModulePositions().indexOf(position) === -1) return false; - return true; -}; +/** + * Checks whether the provided module position exists. + * @param {string} position - Candidate module position. + * @returns {boolean} True when the position is known. + */ +function moduleHasValidPosition (position) { + return getModulePositions().includes(position); +} -const getModulePositions = () => { +/** + * Discovers module positions from index.html and caches them. + * @returns {Array} Discovered module positions. + */ +function getModulePositions () { // if not already discovered if (modulePositions.length === 0) { // get the lines of the index.html @@ -66,14 +73,14 @@ const getModulePositions = () => { } // return the list to the caller return modulePositions; -}; +} /** * Checks the config for deprecated options and throws a warning in the logs * if it encounters one option from the deprecated.js list * @param {object} userConfig The user config */ -const checkDeprecatedOptions = (userConfig) => { +function checkDeprecatedOptions (userConfig) { const deprecated = require(`${global.root_path}/js/deprecated`); // check for deprecated core options @@ -93,13 +100,13 @@ const checkDeprecatedOptions = (userConfig) => { } } } -}; +} /** * Loads the config file. Combines it with the defaults and returns the config * @returns {object} an object holding full and redacted config */ -const loadConfig = () => { +function loadConfig () { Log.log("Loading config ..."); const defaults = require("./defaults"); if (global.mmTestMode) { @@ -172,34 +179,25 @@ 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); } -}; +} /** - * 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 +213,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 +237,36 @@ 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 {ConfigError} 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(""); + 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)) { - Log.error(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule entry must be an object`); - throw new ConfigError(""); + 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") { - Log.error(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nmodule: must be a string`); - throw new ConfigError(""); + 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") { - Log.error(`This module configuration contains errors:\n${JSON.stringify(mod, null, 2)}\nposition: must be a string`); - throw new ConfigError(""); + 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,6 +277,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. + * @throws {ConfigError} When the configuration contains fatal errors. + */ +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 { + throw new ConfigError(formatConfigSyntaxErrors(errors)); + } +} -module.exports = { loadConfig, getModulePositions, moduleHasValidPosition, getAvailableModulePositions, checkConfigFile, ConfigError }; +module.exports = { loadConfig, getModulePositions, moduleHasValidPosition, checkConfigFile, ConfigError }; diff --git a/tests/unit/classes/utils_spec.js b/tests/unit/classes/utils_spec.js index ee90e880fe..1960546ab8 100644 --- a/tests/unit/classes/utils_spec.js +++ b/tests/unit/classes/utils_spec.js @@ -3,24 +3,21 @@ 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 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,27 +55,29 @@ describe("utils", () => { expect(Log.error).not.toHaveBeenCalled(); }); - it("exits when modules is not an array", () => { - expectExitForModules("not-an-array"); - expect(Log.error).toHaveBeenCalledWith("This module configuration contains errors:\nmodules must be an array"); + it("throws when modules is not an array", () => { + expectConfigErrorForModules("not-an-array"); + expect(Log.error).not.toHaveBeenCalled(); }); - it("exits when module field is missing or not a string", () => { - expectExitForModules([{ module: 123, position: "top_bar" }]); - expect(Log.error).toHaveBeenCalled(); - expect(Log.error.mock.calls[0][0]).toContain("module: must be a string"); + it("throws when module field is missing or not a string", () => { + expectConfigErrorForModules([{ module: 123, position: "top_bar" }]); + 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(); + }); });