From 5ff188765ca5e6bbeaf62950a0413a7e137d89ad Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 12:19:12 +0000 Subject: [PATCH 01/49] New: Replace essential module system with isFatal errors and bootstrap libraries Absorbs config, logger, errors, and lang into core as bootstrap libraries loaded before any modules initialise. Replaces the two-phase essential/ non-essential loading with single-pass loading where any error can be marked isFatal in its JSON definition to halt the app. waitForModule handles all load ordering. --- index.js | 5 + lib/AbstractModule.js | 16 +-- lib/AdaptError.js | 57 ++++++++++ lib/App.js | 51 ++++++++- lib/Config.js | 185 +++++++++++++++++++++++++++++++++ lib/DependencyLoader.js | 49 +++------ lib/Errors.js | 53 ++++++++++ lib/Lang.js | 154 +++++++++++++++++++++++++++ lib/Logger.js | 156 +++++++++++++++++++++++++++ package.json | 2 + tests/AbstractModule.spec.js | 58 +---------- tests/AdaptError.spec.js | 62 +++++++++++ tests/Config.spec.js | 103 ++++++++++++++++++ tests/DependencyLoader.spec.js | 111 ++++++++------------ tests/Errors.spec.js | 91 ++++++++++++++++ tests/Lang.spec.js | 130 +++++++++++++++++++++++ tests/Logger.spec.js | 101 ++++++++++++++++++ 17 files changed, 1205 insertions(+), 179 deletions(-) create mode 100644 lib/AdaptError.js create mode 100644 lib/Config.js create mode 100644 lib/Errors.js create mode 100644 lib/Lang.js create mode 100644 lib/Logger.js create mode 100644 tests/AdaptError.spec.js create mode 100644 tests/Config.spec.js create mode 100644 tests/Errors.spec.js create mode 100644 tests/Lang.spec.js create mode 100644 tests/Logger.spec.js diff --git a/index.js b/index.js index e0cda4dd..6ddf7042 100644 --- a/index.js +++ b/index.js @@ -1,6 +1,11 @@ export { default as AbstractModule } from './lib/AbstractModule.js' +export { default as AdaptError } from './lib/AdaptError.js' export { default as App } from './lib/App.js' +export { default as Config } from './lib/Config.js' export { default as DataCache } from './lib/DataCache.js' export { default as DependencyLoader } from './lib/DependencyLoader.js' +export { default as Errors } from './lib/Errors.js' export { default as Hook } from './lib/Hook.js' +export { default as Lang } from './lib/Lang.js' +export { default as Logger } from './lib/Logger.js' export { metadataFileName, packageFileName, isObject, getArgs, spawn, readJson, writeJson, toBoolean, ensureDir, escapeRegExp, stringifyValues, loadDependencyFiles } from './lib/Utils.js' diff --git a/lib/AbstractModule.js b/lib/AbstractModule.js index 612214c2..fa2be899 100644 --- a/lib/AbstractModule.js +++ b/lib/AbstractModule.js @@ -105,26 +105,16 @@ class AbstractModule { * @return {*} */ getConfig (key) { - try { - return this.app.config.get(`${this.name}.${key}`) - } catch (e) { - return undefined - } + return this.app.config?.get(`${this.name}.${key}`) } /** - * Log a message using the Logger module + * Log a message using the Logger * @param {String} level Log level of message * @param {...*} rest Arguments to log */ log (level, ...rest) { - const _log = (e, instance) => { - if (!this.app.logger || (instance && instance.name !== this.app.logger.name)) return false - this.app.dependencyloader.moduleLoadedHook.untap(_log) - this.app.logger.log(level, this.name.replace(/^adapt-authoring-/, ''), ...rest) - return true - } - if (!_log()) this.app.dependencyloader.moduleLoadedHook.tap(_log) + this.app.logger?.log(level, this.name.replace(/^adapt-authoring-/, ''), ...rest) } } diff --git a/lib/AdaptError.js b/lib/AdaptError.js new file mode 100644 index 00000000..50fd8954 --- /dev/null +++ b/lib/AdaptError.js @@ -0,0 +1,57 @@ +/** + * A generic error class for use in Adapt applications + * @memberof core + */ +class AdaptError extends Error { + /** + * @constructor + * @param {string} code The human-readable error code + * @param {number} statusCode The HTTP status code + * @param {object} metadata Metadata describing the error + */ + constructor (code, statusCode = 500, metadata = {}) { + super(code) + /** + * The error code + * @type {String} + */ + this.code = code + /** + * The HTTP status code + * @type {String} + */ + this.statusCode = statusCode + /** + * Whether this error should halt the application + * @type {Boolean} + */ + this.isFatal = metadata.isFatal ?? false + /** + * Metadata describing the error + * @type {Object} + */ + this.meta = metadata + } + + /** + * Chainable function to allow setting of data for use in user-friendly error messages later on. + * @param {object} data + * @returns {AdaptError} + * @example + * // note calling this function will also return + * // the error itself to allow for easy error throwing + * throw this.app.errors.MY_ERROR + * .setData({ hello: 'world' }) + */ + setData (data) { + this.data = data + return this + } + + /** @override */ + toString () { + return `${this.constructor.name}: ${this.code} ${this.data ? JSON.stringify(this.data) : ''}` + } +} + +export default AdaptError diff --git a/lib/App.js b/lib/App.js index bad4665f..db473670 100644 --- a/lib/App.js +++ b/lib/App.js @@ -1,5 +1,9 @@ import AbstractModule from './AbstractModule.js' +import Config from './Config.js' import DependencyLoader from './DependencyLoader.js' +import Errors from './Errors.js' +import Lang from './Lang.js' +import Logger from './Logger.js' import fs from 'fs' import path from 'path' import { metadataFileName, packageFileName, getArgs } from './Utils.js' @@ -53,16 +57,13 @@ class App extends AbstractModule { /** @ignore */ this._isStarting = false - const configRootDir = this.getConfig('rootDir') - if (configRootDir) /** @ignore */this.rootDir = configRootDir - let startError try { await this.start() this.log('verbose', 'GIT', 'INFO', this.git) this.log('verbose', 'DIR', 'rootDir', this.rootDir) - this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) - this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) + this.log('verbose', 'DIR', 'dataDir', this.config.get('adapt-authoring-core.dataDir')) + this.log('verbose', 'DIR', 'tempDir', this.config.get('adapt-authoring-core.tempDir')) } catch (e) { startError = e } @@ -111,11 +112,51 @@ class App extends AbstractModule { this._isStarting = true + await this.dependencyloader.loadConfigs() + await this.loadLibraries() await this.dependencyloader.load() this._isStarting = false } + /** + * Loads the core bootstrap libraries (config, logger, errors, lang) before any modules initialise. + * @return {Promise} + */ + async loadLibraries () { + const deps = this.dependencies + /** + * Reference to the Config instance + * @type {Config} + */ + this.config = await Config.load(this.rootDir, deps, this.name) + /** + * Reference to the Logger instance + * @type {Logger} + */ + this.logger = new Logger({ + levels: this.config.get('adapt-authoring-logger.levels'), + mute: this.config.get('adapt-authoring-logger.mute'), + dateFormat: this.config.get('adapt-authoring-logger.dateFormat'), + showTimestamp: this.config.get('adapt-authoring-logger.showTimestamp') + }) + this.logger.log('info', 'config', `using config at ${this.config.configFilePath}`) + /** + * Reference to the error registry + * @type {Errors} + */ + this.errors = await Errors.load(deps, msg => this.logger.log('warn', 'errors', msg)) + /** + * Reference to the Lang instance + * @type {Lang} + */ + this.lang = new Lang() + await this.lang.loadPhrases(deps, this.rootDir, (...args) => this.logger.log('error', 'lang', ...args)) + + const configRootDir = this.config.get('adapt-authoring-core.rootDir') + if (configRootDir) /** @ignore */this.rootDir = configRootDir + } + /** * Enables waiting for other modules to load * @param {...String} modNames Names of modules to wait for diff --git a/lib/Config.js b/lib/Config.js new file mode 100644 index 00000000..55e4077a --- /dev/null +++ b/lib/Config.js @@ -0,0 +1,185 @@ +import { Schemas } from 'adapt-schemas' +import chalk from 'chalk' +import fs from 'fs/promises' +import path from 'path' + +/** + * Loads, validates, and provides access to application configuration. + * Configuration is sourced from user settings files, environment variables, and module schema defaults. + * @memberof core + */ +class Config { + /** + * Loads configuration from all sources + * @param {String} rootDir Application root directory + * @param {Object} dependencies Key/value map of dependency configs + * @param {String} appName The core module name (for sorting) + * @returns {Promise} + */ + static async load (rootDir, dependencies, appName) { + const instance = new Config() + instance.configFilePath = path.join(rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) + await instance.storeUserSettings() + instance.storeEnvSettings() + instance.storeSchemaSettings(dependencies, appName) + return instance + } + + constructor () { + /** @ignore */ + this._config = {} + /** + * Path to the user configuration file + * @type {String} + */ + this.configFilePath = undefined + /** + * The keys for all attributes marked as public + * @type {Array} + */ + this.publicAttributes = [] + } + + /** + * Determines whether an attribute has a set value + * @param {String} attr Attribute key name + * @return {Boolean} + */ + has (attr) { + return Object.hasOwn(this._config, attr) + } + + /** + * Returns a value for a given attribute + * @param {String} attr Attribute key name + * @return {*} + */ + get (attr) { + return this._config[attr] + } + + /** + * Retrieves all config options marked as 'public' + * @return {Object} + */ + getPublicConfig () { + return this.publicAttributes.reduce((m, a) => { + m[a] = this.get(a) + return m + }, {}) + } + + /** + * Loads the relevant config file into memory + * @return {Promise} + */ + async storeUserSettings () { + let configError + let config + try { + await fs.readFile(this.configFilePath) + } catch (e) { + if (e.code === 'ENOENT') configError = `No config file found at '${this.configFilePath}'` + } + try { + if (!configError) config = (await import(this.configFilePath)).default + } catch (e) { + configError = e.toString() + } + if (configError) { + console.log(chalk.yellow(`Failed to load config at ${this.configFilePath}:\n\n${configError}\n\nWill attempt to run with defaults.\n`)) + return + } + Object.entries(config).forEach(([name, c]) => { + Object.entries(c).forEach(([key, val]) => { + this._config[`${name}.${key}`] = val + }) + }) + } + + /** + * Copy env values to config + */ + storeEnvSettings () { + Object.entries(process.env).forEach(([key, val]) => { + try { + val = JSON.parse(val) + } catch {} // ignore parse errors for non-JSON values + this._config[Config.envVarToConfigKey(key)] = val + }) + } + + /** + * Processes all module config schema files + * @param {Object} dependencies Key/value map of dependency configs + * @param {String} appName The core module name (for sorting) + */ + storeSchemaSettings (dependencies, appName) { + const schemas = new Schemas() + const isCore = d => d.name === appName + const deps = Object.values(dependencies).sort((a, b) => { + if (isCore(a)) return -1 + if (isCore(b)) return 1 + return a.name.localeCompare(b.name) + }) + const coreDep = deps.find(d => isCore(d)) + if (coreDep) this.processModuleSchema(coreDep, schemas) + + let hasErrored = false + for (const d of deps.filter(d => !isCore(d))) { + try { + this.processModuleSchema(d, schemas) + } catch (e) { + hasErrored = true + if (e?.data?.errors) { + console.log(`${e.modName}: ${e.data.errors}`) + } else { + console.log(e) + } + } + } + if (hasErrored) throw new Error('Config validation failed') + } + + /** + * Processes and validates a single module config schema + * @param {Object} pkg Package.json data + * @param {Schemas} schemas Schemas library instance + */ + processModuleSchema (pkg, schemas) { + if (!pkg.name || !pkg.rootDir) return + const schemaPath = path.resolve(pkg.rootDir, 'conf/config.schema.json') + let schema + try { + schema = schemas.createSchema(schemaPath).build() + } catch (e) { + return + } + let data = Object.entries(schema.raw.properties).reduce((m, [k, v]) => { + if (v?._adapt?.isPublic) this.publicAttributes.push(`${pkg.name}.${k}`) + return { ...m, [k]: this.get(`${pkg.name}.${k}`) } + }, {}) + try { + data = schema.validate(data) + } catch (e) { + e.modName = pkg.name + throw e + } + Object.entries(data).forEach(([key, val]) => { this._config[`${pkg.name}.${key}`] = val }) + } + + /** + * Parses an environment variable key into a format expected by Config + * @param {String} envVar + * @return {String} + */ + static envVarToConfigKey (envVar) { + if (envVar.startsWith('ADAPT_AUTHORING_')) { + const [modPrefix, key] = envVar.split('__') + return `${modPrefix.replace(/_/g, '-').toLowerCase()}.${key}` + } + return `env.${envVar}` + } +} + +export default Config diff --git a/lib/DependencyLoader.js b/lib/DependencyLoader.js index c95aa005..96ae4a0b 100644 --- a/lib/DependencyLoader.js +++ b/lib/DependencyLoader.js @@ -60,26 +60,12 @@ class DependencyLoader { } /** - * Loads all Adapt module dependencies. Essential modules are loaded first, then non-essential modules (with force mode). + * Loads all Adapt module dependencies in a single pass. Load ordering is handled by waitForModule. Fatal errors from any module will halt the application. loadConfigs() must be called before this method. * @return {Promise} - * @throws {Error} When any essential module fails to load */ async load () { - await this.loadConfigs() - - const configValues = Object.values(this.configs) - // sort dependencies into priority - const { essential, theRest } = configValues.reduce((m, c) => { - this.app.pkg.essentialApis.includes(c.essentialType) ? m.essential.push(c.name) : m.theRest.push(c.name) - return m - }, { essential: [], theRest: [] }) - // load each set of deps - await this.loadModules(essential) - await this.loadModules(theRest, { force: true }) - - if (this.failedModules.length) { - throw new Error(`Failed to load modules ${this.failedModules.join(', ')}`) - } + const modules = Object.values(this.configs).map(c => c.name) + await this.loadModules(modules) } /** @@ -180,21 +166,16 @@ class DependencyLoader { /** * Loads a list of Adapt modules. Should not need to be called directly. * @param {Array} modules Module names to load - * @param {Object} [options] Loading options - * @param {boolean} [options.force=false] If true, logs errors and continues loading other modules when a module fails. If false, throws a DependencyError on first failure. - * @return {Promise} Resolves when all modules have loaded (or failed to load in force mode) - * @throws {DependencyError} When a module fails to load and options.force is not true + * @return {Promise} Resolves when all modules have loaded or failed + * @throws {Error} When any module throws a fatal error (error.isFatal or error.cause.isFatal) */ - async loadModules (modules, options = {}) { + async loadModules (modules) { await Promise.all(modules.map(async m => { try { await this.loadModule(m) } catch (e) { - if (options.force !== true) { - const error = new Error(`Failed to load '${m}'`) - error.name = 'DependencyError' - error.cause = e - throw error + if (e.isFatal || e.cause?.isFatal) { + throw e } this.logError(`Failed to load '${m}',`, e) const deps = this.peerDependencies[m] @@ -272,15 +253,11 @@ class DependencyLoader { } /** - * Logs a message using the app logger if available, otherwise falls back to console.log + * Logs a message using the app logger * @param {...*} args Arguments to be logged */ log (level, ...args) { - if (this.app.logger?._isReady) { - this.app.logger.log(level, this.name, ...args) - } else { - console.log(...args) - } + this.app.logger?.log(level, this.name, ...args) } /** @@ -294,12 +271,10 @@ class DependencyLoader { /** * Retrieves a configuration value from this module's config * @param {string} key - The configuration key to retrieve - * @returns {*|undefined} The configuration value if config is ready, undefined otherwise + * @returns {*|undefined} The configuration value, or undefined if config is not yet loaded */ getConfig (key) { - if (this.app.config?._isReady) { - return this.app.config.get(`adapt-authoring-core.${key}`) - } + return this.app.config?.get(`adapt-authoring-core.${key}`) } } diff --git a/lib/Errors.js b/lib/Errors.js new file mode 100644 index 00000000..ed4ebb54 --- /dev/null +++ b/lib/Errors.js @@ -0,0 +1,53 @@ +import AdaptError from './AdaptError.js' +import fs from 'fs/promises' +import { glob } from 'glob' + +/** + * Loads and stores all error definitions for the application. Errors are accessed via human-readable error codes for better readability when thrown in code. + * @memberof core + */ +class Errors { + /** + * Loads all errors defined in Adapt module dependencies + * @param {Object} dependencies Key/value map of dependency configs (each with a rootDir) + * @param {Function} [logWarn] Optional logging function for duplicate warnings + * @returns {Promise} + */ + static async load (dependencies, logWarn) { + const instance = new Errors() + const errorDefs = {} + await Promise.all(Object.values(dependencies).map(async d => { + const files = await glob('errors/*.json', { cwd: d.rootDir, absolute: true }) + await Promise.all(files.map(async f => { + try { + const contents = JSON.parse(await fs.readFile(f)) + Object.entries(contents).forEach(([k, v]) => { + if (errorDefs[k]) { + if (logWarn) logWarn(`error code '${k}' already defined`) + return + } + errorDefs[k] = v + }) + } catch (e) { + if (logWarn) logWarn(e.message) + } + })) + })) + Object.entries(errorDefs) + .sort() + .forEach(([k, { description, statusCode, isFatal, data }]) => { + Object.defineProperty(instance, k, { + get: () => { + const metadata = { description } + if (isFatal) metadata.isFatal = true + if (data) metadata.data = data + return new AdaptError(k, statusCode, metadata) + }, + enumerable: true + }) + }) + return instance + } +} + +export default Errors diff --git a/lib/Lang.js b/lib/Lang.js new file mode 100644 index 00000000..fef0a139 --- /dev/null +++ b/lib/Lang.js @@ -0,0 +1,154 @@ +import fs from 'node:fs/promises' +import { glob } from 'glob' +import path from 'node:path' + +/** + * Handles loading and translation of language strings. + * @memberof core + */ +class Lang { + constructor () { + /** + * The loaded language phrases + * @type {Object} + */ + this.phrases = {} + } + + /** + * Returns the languages supported by the application + * @type {Array} + */ + get supportedLanguages () { + return Object.keys(this.phrases) + } + + /** + * Loads and merges all language phrases from dependencies + * @param {Object} dependencies Key/value map of dependency configs (each with a rootDir) + * @param {String} appRootDir The application root directory + * @param {Function} [logError] Optional logging function for errors + * @returns {Promise} + */ + async loadPhrases (dependencies, appRootDir, logError) { + const dirs = [ + appRootDir, + ...Object.values(dependencies).map(d => d.rootDir) + ] + await Promise.all(dirs.map(async dir => { + const files = await glob('lang/*.json', { cwd: dir, absolute: true }) + await Promise.all(files.map(async f => { + try { + const contents = JSON.parse((await fs.readFile(f)).toString()) + Object.entries(contents).forEach(([k, v]) => Lang.storeStrings(this.phrases, `${path.basename(f).replace('.json', '')}.${k}`, v)) + } catch (e) { + if (logError) logError(e.message, f) + } + })) + })) + } + + /** + * Load all lang phrases for a language + * @param {String} lang The language of strings to load + * @return {Object|undefined} The phrases, or undefined if the language is not found + */ + getPhrasesForLang (lang) { + const phrases = this.phrases[lang] + return phrases && Object.keys(phrases).length ? phrases : undefined + } + + /** + * Returns translated language string + * @param {String} defaultLang Default language to use when lang is not a string + * @param {Function} logWarn Logging function for missing keys + * @param {String} lang The target language + * @param {String|AdaptError} key The unique string key + * @param {Object} data Dynamic data to be inserted into translated string + * @return {String} + */ + translate (defaultLang, logWarn, lang, key, data) { + return Lang.translate(this.phrases, defaultLang, logWarn, lang, key, data) + } + + /** + * Translates an AdaptError + * @param {String} defaultLang Default language to use + * @param {Function} logWarn Logging function for missing keys + * @param {String} lang The target language + * @param {AdaptError} error Error to translate + * @returns The translated error + */ + translateError (defaultLang, logWarn, lang, error) { + return Lang.translateError(this.phrases, defaultLang, logWarn, lang, error) + } + + /** + * Parses a dotted language key and stores the value in the phrases dictionary + * @param {Object} phrases The phrases dictionary to store into + * @param {String} key Key in the format 'lang.namespace.key' + * @param {String} value The string value to store + */ + static storeStrings (phrases, key, value) { + const i = key.indexOf('.') + const lang = key.slice(0, i) + if (!phrases[lang]) phrases[lang] = {} + phrases[lang][key.slice(i + 1)] = value + } + + /** + * Returns translated language string + * @param {Object} phrases The phrases dictionary + * @param {String} defaultLang Default language to use when lang is not a string + * @param {Function} logWarn Logging function for missing keys + * @param {String} lang The target language + * @param {String|AdaptError} key The unique string key + * @param {Object} data Dynamic data to be inserted into translated string + * @return {String} + */ + static translate (phrases, defaultLang, logWarn, lang, key, data) { + if (typeof lang !== 'string') { + lang = defaultLang + } + if (key.constructor.name.endsWith('Error')) { + return Lang.translateError(phrases, defaultLang, logWarn, lang, key) + } + const s = phrases[lang]?.[key] + if (!s) { + logWarn(`missing key '${lang}.${key}'`) + return key + } + if (!data) { + return s + } + return Object.entries(data).reduce((s, [k, v]) => { + v = Array.isArray(v) ? v.map(v2 => Lang.translateError(phrases, defaultLang, logWarn, lang, v2)) : Lang.translateError(phrases, defaultLang, logWarn, lang, v) + s = s.replaceAll(`\${${k}}`, v) + if (Array.isArray(v)) { + const matches = [...s.matchAll(new RegExp(String.raw`\$map{${k}:(.+)}`, 'g'))] + matches.forEach(([replace, data]) => { + const [attrs, delim] = data.split(':') + s = s.replace(replace, v.map(val => attrs.split(',').map(a => Object.prototype.hasOwnProperty.call(val, a) ? val[a] : a)).join(delim)) + }) + } + return s + }, s) + } + + /** + * Translates an AdaptError + * @param {Object} phrases The phrases dictionary + * @param {String} defaultLang Default language to use + * @param {Function} logWarn Logging function for missing keys + * @param {String} lang The target language + * @param {AdaptError} error Error to translate + * @returns The translated error + */ + static translateError (phrases, defaultLang, logWarn, lang, error) { + return error?.constructor.name.endsWith('Error') + ? Lang.translate(phrases, defaultLang, logWarn, lang, `error.${error.code}`, error.data ?? error) + : error + } +} + +export default Lang diff --git a/lib/Logger.js b/lib/Logger.js new file mode 100644 index 00000000..000bb0de --- /dev/null +++ b/lib/Logger.js @@ -0,0 +1,156 @@ +import chalk from 'chalk' +import Hook from './Hook.js' + +/** + * Provides console logging with configurable levels, colours, and module-specific overrides. + * @memberof core + */ +class Logger { + /** + * Creates a Logger instance from config values + * @param {Object} options + * @param {Array} options.levels Log level config strings + * @param {Boolean} options.mute Whether to mute all output + * @param {String} options.dateFormat Date format ('iso' or 'short') + * @param {Boolean} options.showTimestamp Whether to show timestamps + */ + constructor ({ levels = ['error', 'warn', 'success', 'info', 'debug', 'verbose'], mute = false, dateFormat = 'iso', showTimestamp = true } = {}) { + /** + * Hook invoked on each message logged + * @type {Hook} + */ + this.logHook = new Hook() + /** @ignore */ + this.config = { + levels: { + error: { + enable: Logger.isLevelEnabled(levels, 'error'), + moduleOverrides: Logger.getModuleOverrides(levels, 'error'), + colour: chalk.red + }, + warn: { + enable: Logger.isLevelEnabled(levels, 'warn'), + moduleOverrides: Logger.getModuleOverrides(levels, 'warn'), + colour: chalk.yellow + }, + success: { + enable: Logger.isLevelEnabled(levels, 'success'), + moduleOverrides: Logger.getModuleOverrides(levels, 'success'), + colour: chalk.green + }, + info: { + enable: Logger.isLevelEnabled(levels, 'info'), + moduleOverrides: Logger.getModuleOverrides(levels, 'info'), + colour: chalk.cyan + }, + debug: { + enable: Logger.isLevelEnabled(levels, 'debug'), + moduleOverrides: Logger.getModuleOverrides(levels, 'debug'), + colour: chalk.dim + }, + verbose: { + enable: Logger.isLevelEnabled(levels, 'verbose'), + moduleOverrides: Logger.getModuleOverrides(levels, 'verbose'), + colour: chalk.grey.italic + } + }, + timestamp: showTimestamp, + dateFormat, + mute: mute.toString() === 'true' + } + } + + /** + * Logs a message to the console + * @param {String} level Severity of the message + * @param {String} id Identifier for the message + * @param {...*} args Arguments to be logged + */ + log (level, id, ...args) { + if (this.config.mute) { + return + } + if (!Logger.isLoggingEnabled(this.config.levels, level, id)) { + return + } + const colour = this.config.levels[level]?.colour + const logFunc = console[level] ?? console.log + logFunc(`${Logger.getDateStamp(this.config)}${Logger.colourise(level, colour)} ${Logger.colourise(id, chalk.magenta)}`, ...args) + this.logHook.invoke(new Date(), level, id, ...args) + } + + /** + * Determines whether a specific log level is enabled + * @param {Array} levelsConfig Array of level configuration strings + * @param {String} level The log level to check + * @return {Boolean} + */ + static isLevelEnabled (levelsConfig, level) { + return !levelsConfig.includes(`!${level}`) && levelsConfig.includes(level) + } + + /** + * Returns module-specific log level overrides + * @param {Array} levelsConfig Array of level configuration strings + * @param {String} level The log level to find overrides for + * @return {Array} + */ + static getModuleOverrides (levelsConfig, level) { + const overrides = [] + levelsConfig.forEach(l => { + const s = `${level}.`; const notS = `!${level}.` + if (l.indexOf(s) === 0 || l.indexOf(notS) === 0) overrides.push(l) + }) + return overrides + } + + /** + * Returns whether a message should be logged based on the resolved config + * @param {Object} configLevels The resolved levels config object + * @param {String} level Logging level + * @param {String} id Id of log caller + * @returns {Boolean} + */ + static isLoggingEnabled (configLevels, level, id) { + const { enable, moduleOverrides = [] } = configLevels?.[level] || {} + const isEnabled = enable || moduleOverrides.includes(`${level}.${id}`) + const disableOverride = moduleOverrides.includes(`!${level}.${id}`) + return isEnabled && !disableOverride + } + + /** + * Colours a string using a chalk function + * @param {String} str The string to colourise + * @param {Function} colourFunc A chalk colour function + * @return {String} + */ + static colourise (str, colourFunc) { + if (typeof colourFunc === 'string') colourFunc = chalk[colourFunc] + return colourFunc ? colourFunc(str) : str + } + + /** + * Returns a formatted date stamp string + * @param {Object} config Logger configuration + * @return {String} + */ + static getDateStamp (config) { + if (!config.timestamp) { + return '' + } + let str + if (config.dateFormat === 'iso') { + str = new Date().toISOString() + } else if (config.dateFormat === 'short') { + const d = new Date() + const m = d.getMonth() + 1 + const s = d.getSeconds() + const date = `${d.getDate()}/${m < 10 ? `0${m}` : m}/${d.getFullYear().toString().slice(2)}` + const time = `${d.getHours()}:${d.getMinutes()}:${s < 10 ? `0${s}` : s}` + str = `${date}-${time}` + } + return Logger.colourise(`${str} `, chalk.dim) + } +} + +export default Logger diff --git a/package.json b/package.json index 9e81df83..bc98fae9 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,8 @@ }, "repository": "github:adapt-security/adapt-authoring-core", "dependencies": { + "adapt-schemas": "^3.1.0", + "chalk": "^5.4.1", "fs-extra": "11.3.3", "glob": "^13.0.0", "lodash": "^4.17.21", diff --git a/tests/AbstractModule.spec.js b/tests/AbstractModule.spec.js index bd2c9cb1..134ed9a4 100644 --- a/tests/AbstractModule.spec.js +++ b/tests/AbstractModule.spec.js @@ -448,13 +448,8 @@ describe('AbstractModule', () => { assert.equal(result, 'testValue') }) - it('should return undefined if config.get throws', async () => { + it('should return undefined when config is not available', async () => { const mockApp = { - config: { - get: () => { - throw new Error('config error') - } - }, dependencyloader: { moduleLoadedHook: { tap: () => {}, @@ -615,57 +610,12 @@ describe('AbstractModule', () => { assert.deepEqual(loggedArgs, ['arg1', 'arg2', 'arg3']) }) - it('should queue log and deliver when logger module loads', async () => { - let loggedLevel - let tapCallback - const mockApp = { - dependencyloader: { - moduleLoadedHook: { - tap: (fn) => { tapCallback = fn }, - untap: () => {} - } - } - } + it('should silently skip when logger is not available', async () => { + const mockApp = {} const module = new AbstractModule(mockApp, { name: 'test-mod' }) await module.onReady() - module.log('warn', 'deferred message') - assert.ok(tapCallback) - - mockApp.logger = { - name: 'adapt-authoring-logger', - log: (level) => { - loggedLevel = level - } - } - - tapCallback(null, { name: 'adapt-authoring-logger' }) - assert.equal(loggedLevel, 'warn') - }) - - it('should not log when loaded module is not the logger', async () => { - const logCalled = false - let tapCallback - const mockApp = { - dependencyloader: { - moduleLoadedHook: { - tap: (fn) => { tapCallback = fn }, - untap: () => {} - } - } - } - const module = new AbstractModule(mockApp, { name: 'test-mod' }) - await module.onReady() - - // No logger set yet, so log queues the callback - module.log('info', 'some message') - assert.ok(tapCallback) - - // Now simulate a non-logger module loading - _log checks !this.app.logger - // which is true (no logger), so it returns false - const result = tapCallback(null, { name: 'adapt-authoring-other' }) - assert.equal(result, false) - assert.equal(logCalled, false) + assert.doesNotThrow(() => module.log('warn', 'no logger')) }) }) diff --git a/tests/AdaptError.spec.js b/tests/AdaptError.spec.js new file mode 100644 index 00000000..cdefdf18 --- /dev/null +++ b/tests/AdaptError.spec.js @@ -0,0 +1,62 @@ +import { describe, it } from 'node:test' +import assert from 'node:assert/strict' +import AdaptError from '../lib/AdaptError.js' + +describe('AdaptError', () => { + describe('constructor', () => { + it('should set code and default statusCode', () => { + const error = new AdaptError('TEST_ERROR') + assert.equal(error.code, 'TEST_ERROR') + assert.equal(error.statusCode, 500) + assert.equal(error.isFatal, false) + }) + + it('should set custom statusCode', () => { + const error = new AdaptError('NOT_FOUND', 404) + assert.equal(error.statusCode, 404) + }) + + it('should set isFatal from metadata', () => { + const error = new AdaptError('FATAL_ERROR', 500, { isFatal: true }) + assert.equal(error.isFatal, true) + }) + + it('should default isFatal to false when not in metadata', () => { + const error = new AdaptError('ERROR', 500, { description: 'test' }) + assert.equal(error.isFatal, false) + }) + + it('should store metadata', () => { + const meta = { description: 'test error', data: { id: 'test' } } + const error = new AdaptError('ERROR', 500, meta) + assert.deepEqual(error.meta, meta) + }) + + it('should extend Error', () => { + const error = new AdaptError('TEST') + assert.ok(error instanceof Error) + }) + }) + + describe('#setData()', () => { + it('should set data and return self for chaining', () => { + const error = new AdaptError('TEST') + const result = error.setData({ id: '123' }) + assert.equal(result, error) + assert.deepEqual(error.data, { id: '123' }) + }) + }) + + describe('#toString()', () => { + it('should include code without data', () => { + const error = new AdaptError('TEST') + assert.ok(error.toString().includes('TEST')) + }) + + it('should include stringified data when set', () => { + const error = new AdaptError('TEST') + error.setData({ id: '123' }) + assert.ok(error.toString().includes('123')) + }) + }) +}) diff --git a/tests/Config.spec.js b/tests/Config.spec.js new file mode 100644 index 00000000..310d9365 --- /dev/null +++ b/tests/Config.spec.js @@ -0,0 +1,103 @@ +import { describe, it, before, after } from 'node:test' +import assert from 'node:assert/strict' +import Config from '../lib/Config.js' +import fs from 'fs-extra' +import path from 'path' +import { fileURLToPath } from 'url' + +const __dirname = path.dirname(fileURLToPath(import.meta.url)) + +describe('Config', () => { + describe('constructor', () => { + it('should initialise with empty config', () => { + const config = new Config() + assert.deepEqual(config.publicAttributes, []) + }) + }) + + describe('#has()', () => { + it('should return true for existing keys', () => { + const config = new Config() + config._config['test.key'] = 'value' + assert.equal(config.has('test.key'), true) + }) + + it('should return false for missing keys', () => { + const config = new Config() + assert.equal(config.has('missing.key'), false) + }) + }) + + describe('#get()', () => { + it('should return value for existing key', () => { + const config = new Config() + config._config['test.key'] = 'value' + assert.equal(config.get('test.key'), 'value') + }) + + it('should return undefined for missing key', () => { + const config = new Config() + assert.equal(config.get('missing'), undefined) + }) + }) + + describe('#getPublicConfig()', () => { + it('should return only public attributes', () => { + const config = new Config() + config._config['mod.public'] = 'yes' + config._config['mod.private'] = 'no' + config.publicAttributes = ['mod.public'] + const result = config.getPublicConfig() + assert.deepEqual(result, { 'mod.public': 'yes' }) + }) + }) + + describe('.envVarToConfigKey()', () => { + it('should convert ADAPT_AUTHORING_ prefixed vars', () => { + const result = Config.envVarToConfigKey('ADAPT_AUTHORING_CORE__dataDir') + assert.equal(result, 'adapt-authoring-core.dataDir') + }) + + it('should prefix non-adapt vars with env.', () => { + const result = Config.envVarToConfigKey('NODE_ENV') + assert.equal(result, 'env.NODE_ENV') + }) + }) + + describe('#storeEnvSettings()', () => { + it('should store env vars in config', () => { + const config = new Config() + process.env.TEST_CONFIG_VAR = 'test_value' + config.storeEnvSettings() + assert.equal(config.get('env.TEST_CONFIG_VAR'), 'test_value') + delete process.env.TEST_CONFIG_VAR + }) + + it('should parse JSON env values', () => { + const config = new Config() + process.env.TEST_JSON_VAR = '42' + config.storeEnvSettings() + assert.equal(config.get('env.TEST_JSON_VAR'), 42) + delete process.env.TEST_JSON_VAR + }) + }) + + describe('#storeUserSettings()', () => { + let confDir + + before(async () => { + confDir = path.join(__dirname, 'data', 'config-test', 'conf') + await fs.ensureDir(confDir) + }) + + after(async () => { + await fs.remove(path.join(__dirname, 'data', 'config-test')) + }) + + it('should handle missing config file gracefully', async () => { + const config = new Config() + config.configFilePath = path.join(confDir, 'nonexistent.config.js') + await assert.doesNotReject(() => config.storeUserSettings()) + }) + }) +}) diff --git a/tests/DependencyLoader.spec.js b/tests/DependencyLoader.spec.js index 52e73e98..4c1f1b68 100644 --- a/tests/DependencyLoader.spec.js +++ b/tests/DependencyLoader.spec.js @@ -62,12 +62,11 @@ describe('DependencyLoader', () => { }) }) - it('should call app.logger when available and ready', () => { + it('should call app.logger when available', () => { let logged = false const mockApp = { rootDir: '/test', logger: { - _isReady: true, // Note: Mock uses private property to simulate ready state log: () => { logged = true } } } @@ -78,27 +77,11 @@ describe('DependencyLoader', () => { assert.equal(logged, true) }) - it('should fall back to console.log when logger not ready', () => { - const mockApp = { - rootDir: '/test', - logger: { - _isReady: false, // Note: Mock uses private property to check ready state - log: () => {} - } - } - const loader = new DependencyLoader(mockApp) - - assert.doesNotThrow(() => { - loader.log('info', 'test message') - }) - }) - it('should pass level and args to app.logger.log', () => { let loggedLevel, loggedName, loggedArgs const mockApp = { rootDir: '/test', logger: { - _isReady: true, log: (level, name, ...args) => { loggedLevel = level loggedName = name @@ -121,7 +104,6 @@ describe('DependencyLoader', () => { const mockApp = { rootDir: '/test', logger: { - _isReady: true, log: (level) => { loggedLevel = level } } } @@ -137,7 +119,6 @@ describe('DependencyLoader', () => { const mockApp = { rootDir: '/test', logger: { - _isReady: true, log: (level, name, ...args) => { loggedArgs = args } } } @@ -148,7 +129,7 @@ describe('DependencyLoader', () => { }) describe('#getConfig()', () => { - it('should return undefined when config is not ready', () => { + it('should return undefined when config is not available', () => { const mockApp = { rootDir: '/test' } @@ -159,11 +140,10 @@ describe('DependencyLoader', () => { assert.equal(result, undefined) }) - it('should return config value when config is ready', () => { + it('should return config value when config is available', () => { const mockApp = { rootDir: '/test', config: { - _isReady: true, // Note: Mock uses private property to simulate ready state get: (key) => { if (key === 'adapt-authoring-core.testKey') return 'testValue' } @@ -176,27 +156,11 @@ describe('DependencyLoader', () => { assert.equal(result, 'testValue') }) - it('should return undefined when config exists but is not ready', () => { - const mockApp = { - rootDir: '/test', - config: { - _isReady: false, - get: () => 'should not be called' - } - } - const loader = new DependencyLoader(mockApp) - - const result = loader.getConfig('someKey') - - assert.equal(result, undefined) - }) - it('should always use adapt-authoring-core prefix for config keys', () => { let requestedKey const mockApp = { rootDir: '/test', config: { - _isReady: true, get: (key) => { requestedKey = key } } } @@ -339,63 +303,72 @@ describe('DependencyLoader', () => { }) describe('#loadModules()', () => { - it('should throw DependencyError when module fails without force', async () => { + it('should add non-fatal failures to failedModules', async () => { const mockApp = { rootDir: '/test' } const loader = new DependencyLoader(mockApp) loader.configs = { 'nonexistent-module': { module: true, name: 'nonexistent-module' } } - await assert.rejects( - loader.loadModules(['nonexistent-module']), - { name: 'DependencyError' } - ) - }) - - it('should not throw when module fails with force option', async () => { - const mockApp = { rootDir: '/test' } - const loader = new DependencyLoader(mockApp) - loader.configs = { 'nonexistent-module': { module: true, name: 'nonexistent-module' } } - - await loader.loadModules(['nonexistent-module'], { force: true }) + await loader.loadModules(['nonexistent-module']) assert.ok(loader.failedModules.includes('nonexistent-module')) }) - it('should log peer dependency warnings on failure with force', async () => { + it('should throw when module throws a fatal error', async () => { const mockApp = { rootDir: '/test' } const loader = new DependencyLoader(mockApp) - loader.configs = { 'nonexistent-module': { module: true, name: 'nonexistent-module' } } - loader.peerDependencies = { 'nonexistent-module': ['dependent-mod'] } + loader.configs = { 'fatal-module': { module: true, name: 'fatal-module' } } - await loader.loadModules(['nonexistent-module'], { force: true }) + // monkey-patch loadModule to throw a fatal error + const originalLoadModule = loader.loadModule.bind(loader) + loader.loadModule = async (name) => { + if (name === 'fatal-module') { + const error = new Error('Fatal') + error.isFatal = true + throw error + } + return originalLoadModule(name) + } - assert.ok(loader.failedModules.includes('nonexistent-module')) + await assert.rejects( + loader.loadModules(['fatal-module']), + (err) => { + assert.equal(err.isFatal, true) + return true + } + ) }) - it('should include module name in DependencyError message', async () => { + it('should throw when error.cause is fatal', async () => { const mockApp = { rootDir: '/test' } const loader = new DependencyLoader(mockApp) - loader.configs = { 'nonexistent-module': { module: true, name: 'nonexistent-module' } } + loader.configs = { 'fatal-module': { module: true, name: 'fatal-module' } } + + loader.loadModule = async () => { + const cause = new Error('Root cause') + cause.isFatal = true + const error = new Error('Wrapper') + error.cause = cause + throw error + } await assert.rejects( - loader.loadModules(['nonexistent-module']), + loader.loadModules(['fatal-module']), (err) => { - assert.ok(err.message.includes('nonexistent-module')) + assert.equal(err.cause.isFatal, true) return true } ) }) - it('should set cause on DependencyError', async () => { + it('should log peer dependency warnings on non-fatal failure', async () => { const mockApp = { rootDir: '/test' } const loader = new DependencyLoader(mockApp) loader.configs = { 'nonexistent-module': { module: true, name: 'nonexistent-module' } } + loader.peerDependencies = { 'nonexistent-module': ['dependent-mod'] } - try { - await loader.loadModules(['nonexistent-module']) - assert.fail('should have thrown') - } catch (err) { - assert.ok(err.cause) - } + await loader.loadModules(['nonexistent-module']) + + assert.ok(loader.failedModules.includes('nonexistent-module')) }) it('should handle empty module list', async () => { @@ -601,7 +574,6 @@ describe('DependencyLoader', () => { const mockApp = { rootDir: '/test', logger: { - _isReady: true, log: (level, name, ...args) => { if (typeof args[0] === 'object' && !Array.isArray(args[0]) && typeof args[0] !== 'string') { loggedTimes = args[0] @@ -628,7 +600,6 @@ describe('DependencyLoader', () => { const mockApp = { rootDir: '/test', logger: { - _isReady: true, log: (level, name, ...args) => { if (typeof args[0] === 'string' && args[0] === 'LOAD') { loggedMessage = args[1] diff --git a/tests/Errors.spec.js b/tests/Errors.spec.js new file mode 100644 index 00000000..12c2135f --- /dev/null +++ b/tests/Errors.spec.js @@ -0,0 +1,91 @@ +import { describe, it, before, after } from 'node:test' +import assert from 'node:assert/strict' +import Errors from '../lib/Errors.js' +import AdaptError from '../lib/AdaptError.js' +import fs from 'fs-extra' +import path from 'path' +import { fileURLToPath } from 'url' + +const __dirname = path.dirname(fileURLToPath(import.meta.url)) + +describe('Errors', () => { + let testDir + + before(async () => { + testDir = path.join(__dirname, 'data', 'errors-test') + const errorsDir = path.join(testDir, 'errors') + await fs.ensureDir(errorsDir) + await fs.writeJson(path.join(errorsDir, 'test.json'), { + TEST_ERROR: { + description: 'A test error', + statusCode: 400, + data: { id: 'The item ID' } + }, + FATAL_ERROR: { + description: 'A fatal error', + statusCode: 500, + isFatal: true + } + }) + }) + + after(async () => { + await fs.remove(testDir) + }) + + describe('.load()', () => { + it('should load error definitions from dependencies', async () => { + const deps = { test: { name: 'test', rootDir: testDir } } + const errors = await Errors.load(deps) + assert.ok(errors.TEST_ERROR) + assert.ok(errors.FATAL_ERROR) + }) + + it('should return AdaptError instances', async () => { + const deps = { test: { name: 'test', rootDir: testDir } } + const errors = await Errors.load(deps) + assert.ok(errors.TEST_ERROR instanceof AdaptError) + }) + + it('should return fresh instances on each access', async () => { + const deps = { test: { name: 'test', rootDir: testDir } } + const errors = await Errors.load(deps) + assert.notEqual(errors.TEST_ERROR, errors.TEST_ERROR) + }) + + it('should set statusCode from definition', async () => { + const deps = { test: { name: 'test', rootDir: testDir } } + const errors = await Errors.load(deps) + assert.equal(errors.TEST_ERROR.statusCode, 400) + }) + + it('should set isFatal from definition', async () => { + const deps = { test: { name: 'test', rootDir: testDir } } + const errors = await Errors.load(deps) + assert.equal(errors.FATAL_ERROR.isFatal, true) + assert.equal(errors.TEST_ERROR.isFatal, false) + }) + + it('should warn on duplicate error codes', async () => { + const dupDir = path.join(__dirname, 'data', 'errors-dup') + const errorsDir = path.join(dupDir, 'errors') + await fs.ensureDir(errorsDir) + await fs.writeJson(path.join(errorsDir, 'dup.json'), { + TEST_ERROR: { description: 'duplicate', statusCode: 500 } + }) + const deps = { + test: { name: 'test', rootDir: testDir }, + dup: { name: 'dup', rootDir: dupDir } + } + let warned = false + await Errors.load(deps, () => { warned = true }) + assert.ok(warned) + await fs.remove(dupDir) + }) + + it('should handle empty dependencies', async () => { + const errors = await Errors.load({}) + assert.deepEqual(Object.keys(errors), []) + }) + }) +}) diff --git a/tests/Lang.spec.js b/tests/Lang.spec.js new file mode 100644 index 00000000..54437ae2 --- /dev/null +++ b/tests/Lang.spec.js @@ -0,0 +1,130 @@ +import { describe, it, before, after } from 'node:test' +import assert from 'node:assert/strict' +import Lang from '../lib/Lang.js' +import fs from 'fs-extra' +import path from 'path' +import { fileURLToPath } from 'url' + +const __dirname = path.dirname(fileURLToPath(import.meta.url)) + +describe('Lang', () => { + let testDir + + before(async () => { + testDir = path.join(__dirname, 'data', 'lang-test') + const langDir = path.join(testDir, 'lang') + await fs.ensureDir(langDir) + await fs.writeJson(path.join(langDir, 'en.json'), { + 'app.name': 'Test App', + 'app.greeting': 'Hello ${name}', // eslint-disable-line no-template-curly-in-string + 'error.TEST_ERROR': 'A test error occurred' + }) + await fs.writeJson(path.join(langDir, 'fr.json'), { + 'app.name': 'Application Test' + }) + }) + + after(async () => { + await fs.remove(testDir) + }) + + describe('#loadPhrases()', () => { + it('should load phrases from dependencies', async () => { + const lang = new Lang() + await lang.loadPhrases({ test: { rootDir: testDir } }, testDir) + assert.ok(lang.phrases.en) + assert.equal(lang.phrases.en['app.name'], 'Test App') + }) + + it('should load multiple languages', async () => { + const lang = new Lang() + await lang.loadPhrases({ test: { rootDir: testDir } }, testDir) + assert.ok(lang.phrases.fr) + assert.equal(lang.phrases.fr['app.name'], 'Application Test') + }) + }) + + describe('#supportedLanguages', () => { + it('should return loaded language keys', async () => { + const lang = new Lang() + await lang.loadPhrases({ test: { rootDir: testDir } }, testDir) + const languages = lang.supportedLanguages + assert.ok(languages.includes('en')) + assert.ok(languages.includes('fr')) + }) + }) + + describe('#getPhrasesForLang()', () => { + it('should return phrases for a specific language', async () => { + const lang = new Lang() + await lang.loadPhrases({ test: { rootDir: testDir } }, testDir) + const phrases = lang.getPhrasesForLang('en') + assert.ok(phrases) + assert.equal(phrases['app.name'], 'Test App') + }) + + it('should return undefined for unknown language', async () => { + const lang = new Lang() + await lang.loadPhrases({ test: { rootDir: testDir } }, testDir) + assert.equal(lang.getPhrasesForLang('de'), undefined) + }) + }) + + describe('.storeStrings()', () => { + it('should store a string under lang.key', () => { + const phrases = {} + Lang.storeStrings(phrases, 'en.hello', 'world') + assert.equal(phrases.en.hello, 'world') + }) + + it('should create lang bucket if missing', () => { + const phrases = {} + Lang.storeStrings(phrases, 'de.greeting', 'Hallo') + assert.ok(phrases.de) + }) + }) + + describe('.translate()', () => { + it('should return translated string', () => { + const phrases = { en: { hello: 'Hello' } } + const result = Lang.translate(phrases, 'en', () => {}, 'en', 'hello') + assert.equal(result, 'Hello') + }) + + it('should substitute data placeholders', () => { + // eslint-disable-next-line no-template-curly-in-string + const phrases = { en: { greeting: 'Hello ${name}' } } + const result = Lang.translate(phrases, 'en', () => {}, 'en', 'greeting', { name: 'World' }) + assert.equal(result, 'Hello World') + }) + + it('should fall back to default lang when lang is not a string', () => { + const phrases = { en: { hello: 'Hello' } } + const result = Lang.translate(phrases, 'en', () => {}, undefined, 'hello') + assert.equal(result, 'Hello') + }) + + it('should return key and warn when key is missing', () => { + const phrases = { en: {} } + let warned = false + const result = Lang.translate(phrases, 'en', () => { warned = true }, 'en', 'missing.key') + assert.equal(result, 'missing.key') + assert.ok(warned) + }) + }) + + describe('.translateError()', () => { + it('should return non-error values unchanged', () => { + const result = Lang.translateError({}, 'en', () => {}, 'en', 'plain string') + assert.equal(result, 'plain string') + }) + + it('should translate an error using its code', () => { + const phrases = { en: { 'error.TEST': 'Translated error' } } + const error = new Error('TEST') + error.code = 'TEST' + const result = Lang.translateError(phrases, 'en', () => {}, 'en', error) + assert.equal(result, 'Translated error') + }) + }) +}) diff --git a/tests/Logger.spec.js b/tests/Logger.spec.js new file mode 100644 index 00000000..3d2f4c44 --- /dev/null +++ b/tests/Logger.spec.js @@ -0,0 +1,101 @@ +import { describe, it } from 'node:test' +import assert from 'node:assert/strict' +import Logger from '../lib/Logger.js' + +describe('Logger', () => { + describe('constructor', () => { + it('should create with defaults', () => { + const logger = new Logger() + assert.ok(logger.config) + assert.ok(logger.logHook) + assert.equal(logger.config.mute, false) + }) + + it('should respect mute option', () => { + const logger = new Logger({ mute: true }) + assert.equal(logger.config.mute, true) + }) + + it('should configure levels from options', () => { + const logger = new Logger({ levels: ['error', 'warn'] }) + assert.equal(logger.config.levels.error.enable, true) + assert.equal(logger.config.levels.debug.enable, false) + }) + }) + + describe('.isLevelEnabled()', () => { + it('should return true when level is in config', () => { + assert.equal(Logger.isLevelEnabled(['error', 'warn'], 'error'), true) + }) + + it('should return false when level is not in config', () => { + assert.equal(Logger.isLevelEnabled(['error'], 'debug'), false) + }) + + it('should return false when level is negated', () => { + assert.equal(Logger.isLevelEnabled(['error', '!error'], 'error'), false) + }) + }) + + describe('.getModuleOverrides()', () => { + it('should return matching overrides', () => { + const config = ['error', 'debug.mymod', '!debug.other'] + assert.deepEqual(Logger.getModuleOverrides(config, 'debug'), ['debug.mymod', '!debug.other']) + }) + + it('should return empty array when no overrides', () => { + assert.deepEqual(Logger.getModuleOverrides(['error'], 'debug'), []) + }) + }) + + describe('.isLoggingEnabled()', () => { + it('should return true when level is enabled', () => { + const levels = { error: { enable: true, moduleOverrides: [] } } + assert.equal(Logger.isLoggingEnabled(levels, 'error', 'test'), true) + }) + + it('should return false when level is disabled', () => { + const levels = { error: { enable: false, moduleOverrides: [] } } + assert.equal(Logger.isLoggingEnabled(levels, 'error', 'test'), false) + }) + + it('should allow module-specific override', () => { + const levels = { debug: { enable: false, moduleOverrides: ['debug.mymod'] } } + assert.equal(Logger.isLoggingEnabled(levels, 'debug', 'mymod'), true) + }) + + it('should allow module-specific disable override', () => { + const levels = { debug: { enable: true, moduleOverrides: ['!debug.mymod'] } } + assert.equal(Logger.isLoggingEnabled(levels, 'debug', 'mymod'), false) + }) + }) + + describe('.colourise()', () => { + it('should return string unchanged when no colour function', () => { + assert.equal(Logger.colourise('test', undefined), 'test') + }) + + it('should apply colour function', () => { + const result = Logger.colourise('test', s => `[${s}]`) + assert.equal(result, '[test]') + }) + }) + + describe('.getDateStamp()', () => { + it('should return empty string when timestamp disabled', () => { + assert.equal(Logger.getDateStamp({ timestamp: false }), '') + }) + + it('should return ISO format when configured', () => { + const result = Logger.getDateStamp({ timestamp: true, dateFormat: 'iso' }) + assert.ok(result.length > 0) + }) + }) + + describe('#log()', () => { + it('should not throw when muted', () => { + const logger = new Logger({ mute: true }) + assert.doesNotThrow(() => logger.log('error', 'test', 'message')) + }) + }) +}) From d76ecd5b045cdc2dfa9fe4506ce888924fce54e4 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 12:26:30 +0000 Subject: [PATCH 02/49] Update: Merge error definitions and config schemas from absorbed modules Merges all error JSON and config schema properties from the absorbed modules (errors, config, lang, logger) into core. Logger config keys use loggerX prefix (e.g. loggerLevels, loggerMute). Lang defaultLang included as-is. --- conf/config.schema.json | 27 ++++++++++++++++++++ errors/errors.json | 55 +++++++++++++++++++++++++++++++++++++++++ errors/node-core.json | 39 +++++++++++++++++++++++++++++ lib/App.js | 8 +++--- 4 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 errors/node-core.json diff --git a/conf/config.schema.json b/conf/config.schema.json index 3d651c4b..b47d54e8 100644 --- a/conf/config.schema.json +++ b/conf/config.schema.json @@ -24,6 +24,33 @@ "type": "string", "isDirectory": true, "default": "$ROOT/APP_DATA/temp" + }, + "loggerLevels": { + "description": "Configures which log levels of log should be shown. Accepts generic levels, module-specific levels and not logic (e.g. 'debug', 'debug.core' and '!debug' respectively).", + "type": "array", + "items": { "type": "string" }, + "default": ["error", "warn", "success", "info", "debug", "verbose"] + }, + "loggerMute": { + "description": "Whether to mute log messages", + "type": ["boolean", "string"], + "default": false + }, + "loggerDateFormat": { + "description": "The date format to use for log timestamps", + "type": "string", + "enum": ["short", "iso"], + "default": "iso" + }, + "loggerShowTimestamp": { + "description": "Whether to prepend a timestamp to each log message", + "type": "boolean", + "default": true + }, + "defaultLang": { + "description": "The default language used by the server", + "type": "string", + "default": "en" } } } diff --git a/errors/errors.json b/errors/errors.json index 51095e92..e585fdd4 100644 --- a/errors/errors.json +++ b/errors/errors.json @@ -1,4 +1,52 @@ { + "FILE_SYNTAX_ERROR": { + "data": { + "path": "Path to the invalid file", + "message": "The error message" + }, + "description": "File contains a syntax error", + "statusCode": 500 + }, + "FUNC_DISABLED": { + "data": { + "name": "The name of the function" + }, + "description": "Function has been disabled", + "statusCode": 500 + }, + "FUNC_NOT_OVERRIDDEN": { + "data": { + "name": "The name of the function" + }, + "description": "Function must be overridden in child class", + "statusCode": 500 + }, + "INVALID_PARAMS": { + "data": { + "params": "The invalid params" + }, + "description": "Invalid parameters have been provided", + "statusCode": 400 + }, + "LOAD_ERROR": { + "description": "Config failed to load", + "statusCode": 500 + }, + "NOT_FOUND": { + "data": { + "id": "An identifier for the missing item", + "type": "Type of the missing item" + }, + "description": "Requested item could not be found", + "statusCode": 404 + }, + "SERVER_ERROR": { + "description": "Generic server error", + "statusCode": 500, + "data": { + "error": "The original error" + } + }, "SPAWN": { "data": { "cmd": "The command", @@ -7,5 +55,12 @@ }, "description": "Error occurred spawning command", "statusCode": 500 + }, + "UNKNOWN_LANG": { + "data": { + "lang": "language" + }, + "description": "unknown language", + "statusCode": 400 } } diff --git a/errors/node-core.json b/errors/node-core.json new file mode 100644 index 00000000..e31cfe9b --- /dev/null +++ b/errors/node-core.json @@ -0,0 +1,39 @@ +{ + "EACCES": { + "description": "An attempt was made to access a file in a way forbidden by its file access permissions", + "statusCode": 500 + }, + "EADDRINUSE": { + "description": "An attempt to bind a server to a local address failed due to another server on the local system already occupying that address", + "statusCode": 500 + }, + "ECONNREFUSED": { + "description": "No connection could be made because the target machine actively refused it", + "statusCode": 500 + }, + "EEXIST": { + "description": "An existing file was the target of an operation that required that the target not exist", + "statusCode": 500, + "data": { + "path": "Path to target file or directory" + } + }, + "ENOENT": { + "description": "No entity (file or directory) could be found by the given path", + "statusCode": 500, + "data": { + "path": "Path to target file or directory" + } + }, + "ENOTEMPTY": { + "description": "A directory with entries was the target of an operation that requires an empty directory", + "statusCode": 500, + "data": { + "path": "Path to target file or directory" + } + }, + "MODULE_NOT_FOUND": { + "description": "A module file could not be resolved while attempting a require() or import operation", + "statusCode": 500 + } +} diff --git a/lib/App.js b/lib/App.js index db473670..dd77e2d7 100644 --- a/lib/App.js +++ b/lib/App.js @@ -135,10 +135,10 @@ class App extends AbstractModule { * @type {Logger} */ this.logger = new Logger({ - levels: this.config.get('adapt-authoring-logger.levels'), - mute: this.config.get('adapt-authoring-logger.mute'), - dateFormat: this.config.get('adapt-authoring-logger.dateFormat'), - showTimestamp: this.config.get('adapt-authoring-logger.showTimestamp') + levels: this.config.get('adapt-authoring-core.loggerLevels'), + mute: this.config.get('adapt-authoring-core.loggerMute'), + dateFormat: this.config.get('adapt-authoring-core.loggerDateFormat'), + showTimestamp: this.config.get('adapt-authoring-core.loggerShowTimestamp') }) this.logger.log('info', 'config', `using config at ${this.config.configFilePath}`) /** From ce1f3e1b9b397f704593b173057e36fc87ff0a6f Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 12:30:03 +0000 Subject: [PATCH 03/49] =?UTF-8?q?Update:=20Simplify=20logger=20config=20?= =?UTF-8?q?=E2=80=94=20remove=20mute=20and=20dateFormat=20options?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Empty loggerLevels array now implies mute. Removes loggerMute and loggerDateFormat config options. Timestamps always use ISO format. --- conf/config.schema.json | 13 +------------ lib/App.js | 2 -- lib/Logger.js | 24 +++++------------------- tests/Logger.spec.js | 13 +++++++++---- 4 files changed, 15 insertions(+), 37 deletions(-) diff --git a/conf/config.schema.json b/conf/config.schema.json index b47d54e8..b13c9f55 100644 --- a/conf/config.schema.json +++ b/conf/config.schema.json @@ -26,22 +26,11 @@ "default": "$ROOT/APP_DATA/temp" }, "loggerLevels": { - "description": "Configures which log levels of log should be shown. Accepts generic levels, module-specific levels and not logic (e.g. 'debug', 'debug.core' and '!debug' respectively).", + "description": "Configures which log levels should be shown. Accepts generic levels, module-specific levels and not logic (e.g. 'debug', 'debug.core' and '!debug' respectively). Set to an empty array to mute all logging.", "type": "array", "items": { "type": "string" }, "default": ["error", "warn", "success", "info", "debug", "verbose"] }, - "loggerMute": { - "description": "Whether to mute log messages", - "type": ["boolean", "string"], - "default": false - }, - "loggerDateFormat": { - "description": "The date format to use for log timestamps", - "type": "string", - "enum": ["short", "iso"], - "default": "iso" - }, "loggerShowTimestamp": { "description": "Whether to prepend a timestamp to each log message", "type": "boolean", diff --git a/lib/App.js b/lib/App.js index dd77e2d7..28bb5adf 100644 --- a/lib/App.js +++ b/lib/App.js @@ -136,8 +136,6 @@ class App extends AbstractModule { */ this.logger = new Logger({ levels: this.config.get('adapt-authoring-core.loggerLevels'), - mute: this.config.get('adapt-authoring-core.loggerMute'), - dateFormat: this.config.get('adapt-authoring-core.loggerDateFormat'), showTimestamp: this.config.get('adapt-authoring-core.loggerShowTimestamp') }) this.logger.log('info', 'config', `using config at ${this.config.configFilePath}`) diff --git a/lib/Logger.js b/lib/Logger.js index 000bb0de..079df98a 100644 --- a/lib/Logger.js +++ b/lib/Logger.js @@ -9,12 +9,10 @@ class Logger { /** * Creates a Logger instance from config values * @param {Object} options - * @param {Array} options.levels Log level config strings - * @param {Boolean} options.mute Whether to mute all output - * @param {String} options.dateFormat Date format ('iso' or 'short') + * @param {Array} options.levels Log level config strings. An empty array mutes all output. * @param {Boolean} options.showTimestamp Whether to show timestamps */ - constructor ({ levels = ['error', 'warn', 'success', 'info', 'debug', 'verbose'], mute = false, dateFormat = 'iso', showTimestamp = true } = {}) { + constructor ({ levels = ['error', 'warn', 'success', 'info', 'debug', 'verbose'], showTimestamp = true } = {}) { /** * Hook invoked on each message logged * @type {Hook} @@ -55,8 +53,7 @@ class Logger { } }, timestamp: showTimestamp, - dateFormat, - mute: mute.toString() === 'true' + mute: levels.length === 0 } } @@ -130,7 +127,7 @@ class Logger { } /** - * Returns a formatted date stamp string + * Returns a formatted ISO date stamp string * @param {Object} config Logger configuration * @return {String} */ @@ -138,18 +135,7 @@ class Logger { if (!config.timestamp) { return '' } - let str - if (config.dateFormat === 'iso') { - str = new Date().toISOString() - } else if (config.dateFormat === 'short') { - const d = new Date() - const m = d.getMonth() + 1 - const s = d.getSeconds() - const date = `${d.getDate()}/${m < 10 ? `0${m}` : m}/${d.getFullYear().toString().slice(2)}` - const time = `${d.getHours()}:${d.getMinutes()}:${s < 10 ? `0${s}` : s}` - str = `${date}-${time}` - } - return Logger.colourise(`${str} `, chalk.dim) + return Logger.colourise(`${new Date().toISOString()} `, chalk.dim) } } diff --git a/tests/Logger.spec.js b/tests/Logger.spec.js index 3d2f4c44..64538eb2 100644 --- a/tests/Logger.spec.js +++ b/tests/Logger.spec.js @@ -11,11 +11,16 @@ describe('Logger', () => { assert.equal(logger.config.mute, false) }) - it('should respect mute option', () => { - const logger = new Logger({ mute: true }) + it('should mute when levels is empty', () => { + const logger = new Logger({ levels: [] }) assert.equal(logger.config.mute, true) }) + it('should not mute when levels are provided', () => { + const logger = new Logger({ levels: ['error'] }) + assert.equal(logger.config.mute, false) + }) + it('should configure levels from options', () => { const logger = new Logger({ levels: ['error', 'warn'] }) assert.equal(logger.config.levels.error.enable, true) @@ -93,8 +98,8 @@ describe('Logger', () => { }) describe('#log()', () => { - it('should not throw when muted', () => { - const logger = new Logger({ mute: true }) + it('should not throw when muted via empty levels', () => { + const logger = new Logger({ levels: [] }) assert.doesNotThrow(() => logger.log('error', 'test', 'message')) }) }) From 18f646f78882c3928fc7fbcc88b728893c6b71f2 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 12:32:27 +0000 Subject: [PATCH 04/49] Update: Rename logger config keys to logLevels and showLogTimestamp --- conf/config.schema.json | 4 ++-- lib/App.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/conf/config.schema.json b/conf/config.schema.json index b13c9f55..3fd8d871 100644 --- a/conf/config.schema.json +++ b/conf/config.schema.json @@ -25,13 +25,13 @@ "isDirectory": true, "default": "$ROOT/APP_DATA/temp" }, - "loggerLevels": { + "logLevels": { "description": "Configures which log levels should be shown. Accepts generic levels, module-specific levels and not logic (e.g. 'debug', 'debug.core' and '!debug' respectively). Set to an empty array to mute all logging.", "type": "array", "items": { "type": "string" }, "default": ["error", "warn", "success", "info", "debug", "verbose"] }, - "loggerShowTimestamp": { + "showLogTimestamp": { "description": "Whether to prepend a timestamp to each log message", "type": "boolean", "default": true diff --git a/lib/App.js b/lib/App.js index 28bb5adf..01894e6e 100644 --- a/lib/App.js +++ b/lib/App.js @@ -135,8 +135,8 @@ class App extends AbstractModule { * @type {Logger} */ this.logger = new Logger({ - levels: this.config.get('adapt-authoring-core.loggerLevels'), - showTimestamp: this.config.get('adapt-authoring-core.loggerShowTimestamp') + levels: this.config.get('adapt-authoring-core.logLevels'), + showTimestamp: this.config.get('adapt-authoring-core.showLogTimestamp') }) this.logger.log('info', 'config', `using config at ${this.config.configFilePath}`) /** From b18db7e3f8c532e63fa3bd7218ac99f45e823d99 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 12:53:01 +0000 Subject: [PATCH 05/49] New: Generic deprecated config key detection via conf/deprecated*.json Modules can define conf/deprecated.json to map old config keys to new ones. Supports _module (check a different module section) and _fatal (throw instead of warn). Core includes deprecation files for absorbed logger and lang modules. --- conf/deprecated-lang.json | 4 ++ conf/deprecated-logger.json | 7 +++ lib/Config.js | 54 +++++++++++++++++- tests/Config.spec.js | 110 ++++++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 conf/deprecated-lang.json create mode 100644 conf/deprecated-logger.json diff --git a/conf/deprecated-lang.json b/conf/deprecated-lang.json new file mode 100644 index 00000000..2166f4a6 --- /dev/null +++ b/conf/deprecated-lang.json @@ -0,0 +1,4 @@ +{ + "_module": "adapt-authoring-lang", + "defaultLang": "adapt-authoring-core.defaultLang" +} diff --git a/conf/deprecated-logger.json b/conf/deprecated-logger.json new file mode 100644 index 00000000..51c0195b --- /dev/null +++ b/conf/deprecated-logger.json @@ -0,0 +1,7 @@ +{ + "_module": "adapt-authoring-logger", + "levels": "adapt-authoring-core.logLevels", + "showTimestamp": "adapt-authoring-core.showLogTimestamp", + "mute": null, + "dateFormat": null +} diff --git a/lib/Config.js b/lib/Config.js index 55e4077a..04ee5dc2 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -1,6 +1,7 @@ import { Schemas } from 'adapt-schemas' import chalk from 'chalk' import fs from 'fs/promises' +import { glob } from 'glob' import path from 'path' /** @@ -19,7 +20,8 @@ class Config { static async load (rootDir, dependencies, appName) { const instance = new Config() instance.configFilePath = path.join(rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) - await instance.storeUserSettings() + const userConfig = await instance.storeUserSettings() + if (userConfig) await instance.checkDeprecatedConfig(userConfig, dependencies) instance.storeEnvSettings() instance.storeSchemaSettings(dependencies, appName) return instance @@ -71,7 +73,7 @@ class Config { /** * Loads the relevant config file into memory - * @return {Promise} + * @return {Promise} The parsed config object, or undefined if loading failed */ async storeUserSettings () { let configError @@ -95,6 +97,54 @@ class Config { this._config[`${name}.${key}`] = val }) }) + return config + } + + /** + * Scans all dependencies for conf/deprecated*.json files and checks the user config for deprecated keys. + * Each file maps old key names to new key names (or null if removed). Reserved keys: + * - `_module`: check a different module section in the user config (for keys that moved between modules) + * - `_fatal`: if true, throw an error instead of logging a warning when deprecated keys are found + * @param {Object} userConfig The parsed user config object + * @param {Object} dependencies Key/value map of dependency configs + * @return {Promise} + */ + async checkDeprecatedConfig (userConfig, dependencies) { + const warnings = [] + let hasFatal = false + await Promise.all(Object.values(dependencies).map(async dep => { + const files = await glob('conf/deprecated*.json', { cwd: dep.rootDir, absolute: true }) + await Promise.all(files.map(async f => { + try { + const mappings = JSON.parse(await fs.readFile(f)) + const modName = mappings._module ?? dep.name + const isFatal = mappings._fatal ?? false + delete mappings._module + delete mappings._fatal + if (!userConfig[modName]) return + for (const [oldKey, newKey] of Object.entries(mappings)) { + if (!(oldKey in userConfig[modName])) continue + if (isFatal) hasFatal = true + if (newKey) { + warnings.push(` '${modName}.${oldKey}' has moved to '${newKey}'`) + } else { + warnings.push(` '${modName}.${oldKey}' has been removed`) + } + } + } catch (e) {} // ignore malformed files + })) + })) + if (warnings.length) { + const message = [ + '\nDeprecated config keys detected. Please update your config file:\n', + ...warnings, + '\n' + ].join('\n') + if (hasFatal) { + throw new Error(message) + } + console.log(chalk.yellow(message)) + } } /** diff --git a/tests/Config.spec.js b/tests/Config.spec.js index 310d9365..8fdae1cf 100644 --- a/tests/Config.spec.js +++ b/tests/Config.spec.js @@ -100,4 +100,114 @@ describe('Config', () => { await assert.doesNotReject(() => config.storeUserSettings()) }) }) + + describe('#checkDeprecatedConfig()', () => { + let deprecatedDir + + before(async () => { + deprecatedDir = path.join(__dirname, 'data', 'deprecated-test') + await fs.ensureDir(path.join(deprecatedDir, 'conf')) + await fs.writeJson(path.join(deprecatedDir, 'conf', 'deprecated.json'), { + oldKey: 'test-module.newKey', + removedKey: null + }) + }) + + after(async () => { + await fs.remove(deprecatedDir) + }) + + it('should warn on moved keys', async () => { + let output = '' + const originalLog = console.log + console.log = (msg) => { output += msg } + const config = new Config() + await config.checkDeprecatedConfig( + { 'test-module': { oldKey: 'value' } }, + { test: { name: 'test-module', rootDir: deprecatedDir } } + ) + console.log = originalLog + assert.ok(output.includes("'test-module.oldKey' has moved to 'test-module.newKey'")) + }) + + it('should warn on removed keys', async () => { + let output = '' + const originalLog = console.log + console.log = (msg) => { output += msg } + const config = new Config() + await config.checkDeprecatedConfig( + { 'test-module': { removedKey: true } }, + { test: { name: 'test-module', rootDir: deprecatedDir } } + ) + console.log = originalLog + assert.ok(output.includes('has been removed')) + }) + + it('should not warn when no deprecated keys present', async () => { + let output = '' + const originalLog = console.log + console.log = (msg) => { output += msg } + const config = new Config() + await config.checkDeprecatedConfig( + { 'test-module': { safeKey: 'value' } }, + { test: { name: 'test-module', rootDir: deprecatedDir } } + ) + console.log = originalLog + assert.equal(output, '') + }) + + it('should use _module override to check a different module section', async () => { + const overrideDir = path.join(__dirname, 'data', 'deprecated-override') + await fs.ensureDir(path.join(overrideDir, 'conf')) + await fs.writeJson(path.join(overrideDir, 'conf', 'deprecated.json'), { + _module: 'old-module', + oldKey: 'new-module.newKey' + }) + let output = '' + const originalLog = console.log + console.log = (msg) => { output += msg } + const config = new Config() + await config.checkDeprecatedConfig( + { 'old-module': { oldKey: 'value' } }, + { test: { name: 'new-module', rootDir: overrideDir } } + ) + console.log = originalLog + assert.ok(output.includes("'old-module.oldKey' has moved to 'new-module.newKey'")) + await fs.remove(overrideDir) + }) + + it('should throw when _fatal is true and deprecated keys are found', async () => { + const fatalDir = path.join(__dirname, 'data', 'deprecated-fatal') + await fs.ensureDir(path.join(fatalDir, 'conf')) + await fs.writeJson(path.join(fatalDir, 'conf', 'deprecated.json'), { + _fatal: true, + oldKey: 'test-module.newKey' + }) + const config = new Config() + await assert.rejects( + config.checkDeprecatedConfig( + { 'test-module': { oldKey: 'value' } }, + { test: { name: 'test-module', rootDir: fatalDir } } + ) + ) + await fs.remove(fatalDir) + }) + + it('should not throw when _fatal is true but no deprecated keys are found', async () => { + const fatalDir = path.join(__dirname, 'data', 'deprecated-fatal2') + await fs.ensureDir(path.join(fatalDir, 'conf')) + await fs.writeJson(path.join(fatalDir, 'conf', 'deprecated.json'), { + _fatal: true, + oldKey: 'test-module.newKey' + }) + const config = new Config() + await assert.doesNotReject( + config.checkDeprecatedConfig( + { 'test-module': { safeKey: 'value' } }, + { test: { name: 'test-module', rootDir: fatalDir } } + ) + ) + await fs.remove(fatalDir) + }) + }) }) From a8a982643412e8704e476de859fda7a5ca94cd50 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 12:56:28 +0000 Subject: [PATCH 06/49] =?UTF-8?q?Update:=20Remove=20=5Ffatal=20from=20depr?= =?UTF-8?q?ecated=20config=20=E2=80=94=20schema=20validation=20covers=20th?= =?UTF-8?q?is?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/Config.js | 17 ++++------------- tests/Config.spec.js | 34 ---------------------------------- 2 files changed, 4 insertions(+), 47 deletions(-) diff --git a/lib/Config.js b/lib/Config.js index 04ee5dc2..cb4af17b 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -102,29 +102,24 @@ class Config { /** * Scans all dependencies for conf/deprecated*.json files and checks the user config for deprecated keys. - * Each file maps old key names to new key names (or null if removed). Reserved keys: - * - `_module`: check a different module section in the user config (for keys that moved between modules) - * - `_fatal`: if true, throw an error instead of logging a warning when deprecated keys are found + * Each file maps old key names to new key names (or null if removed). The reserved `_module` key + * can be used to check a different module section in the user config (for keys that moved between modules). * @param {Object} userConfig The parsed user config object * @param {Object} dependencies Key/value map of dependency configs * @return {Promise} */ async checkDeprecatedConfig (userConfig, dependencies) { const warnings = [] - let hasFatal = false await Promise.all(Object.values(dependencies).map(async dep => { const files = await glob('conf/deprecated*.json', { cwd: dep.rootDir, absolute: true }) await Promise.all(files.map(async f => { try { const mappings = JSON.parse(await fs.readFile(f)) const modName = mappings._module ?? dep.name - const isFatal = mappings._fatal ?? false delete mappings._module - delete mappings._fatal if (!userConfig[modName]) return for (const [oldKey, newKey] of Object.entries(mappings)) { if (!(oldKey in userConfig[modName])) continue - if (isFatal) hasFatal = true if (newKey) { warnings.push(` '${modName}.${oldKey}' has moved to '${newKey}'`) } else { @@ -135,15 +130,11 @@ class Config { })) })) if (warnings.length) { - const message = [ + console.log(chalk.yellow([ '\nDeprecated config keys detected. Please update your config file:\n', ...warnings, '\n' - ].join('\n') - if (hasFatal) { - throw new Error(message) - } - console.log(chalk.yellow(message)) + ].join('\n'))) } } diff --git a/tests/Config.spec.js b/tests/Config.spec.js index 8fdae1cf..a402a449 100644 --- a/tests/Config.spec.js +++ b/tests/Config.spec.js @@ -175,39 +175,5 @@ describe('Config', () => { assert.ok(output.includes("'old-module.oldKey' has moved to 'new-module.newKey'")) await fs.remove(overrideDir) }) - - it('should throw when _fatal is true and deprecated keys are found', async () => { - const fatalDir = path.join(__dirname, 'data', 'deprecated-fatal') - await fs.ensureDir(path.join(fatalDir, 'conf')) - await fs.writeJson(path.join(fatalDir, 'conf', 'deprecated.json'), { - _fatal: true, - oldKey: 'test-module.newKey' - }) - const config = new Config() - await assert.rejects( - config.checkDeprecatedConfig( - { 'test-module': { oldKey: 'value' } }, - { test: { name: 'test-module', rootDir: fatalDir } } - ) - ) - await fs.remove(fatalDir) - }) - - it('should not throw when _fatal is true but no deprecated keys are found', async () => { - const fatalDir = path.join(__dirname, 'data', 'deprecated-fatal2') - await fs.ensureDir(path.join(fatalDir, 'conf')) - await fs.writeJson(path.join(fatalDir, 'conf', 'deprecated.json'), { - _fatal: true, - oldKey: 'test-module.newKey' - }) - const config = new Config() - await assert.doesNotReject( - config.checkDeprecatedConfig( - { 'test-module': { safeKey: 'value' } }, - { test: { name: 'test-module', rootDir: fatalDir } } - ) - ) - await fs.remove(fatalDir) - }) }) }) From a7b4c592a727cb555d3449738661788c76b96fc3 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 13:28:27 +0000 Subject: [PATCH 07/49] Chore: Merge docs and doc plugins from config and errors modules --- docs/configure-environment.md | 78 +++++++++++++++++++++++++++++++++++ docs/error-handling.md | 50 ++++++++++++++++++++++ docs/plugins/configuration.js | 75 +++++++++++++++++++++++++++++++++ docs/plugins/configuration.md | 14 +++++++ docs/plugins/errors.js | 21 ++++++++++ docs/plugins/errorsref.md | 9 ++++ 6 files changed, 247 insertions(+) create mode 100644 docs/configure-environment.md create mode 100644 docs/error-handling.md create mode 100644 docs/plugins/configuration.js create mode 100644 docs/plugins/configuration.md create mode 100644 docs/plugins/errors.js create mode 100644 docs/plugins/errorsref.md diff --git a/docs/configure-environment.md b/docs/configure-environment.md new file mode 100644 index 00000000..bf3aad99 --- /dev/null +++ b/docs/configure-environment.md @@ -0,0 +1,78 @@ +# Configuring your environment +> For a list of all supported configuration options, see [this page](configuration). + +The authoring tool has been built to allow for multiple configurations for different system environments (e.g. testing, production, development). + +## Set up your environment + +To configure your tool for a specific environment, you must create a config file in `/conf` named according to the env value your system will be using (e.g. `dev.config.js`, `production.config.js`, `helloworld.config.js`). We recommend sticking to something short like `dev`, or `test`, but it's up to you what you name these; just make sure to set the environment variable to the same. + +> The `NODE_ENV` environment variable is used to determine the current environment, so make sure that this is set appropriately when running the application: + +Express.js has a number of [performance enhancing features](https://expressjs.com/en/advanced/best-practice-performance.html#set-node_env-to-production) which are only enabled when the NODE_ENV is set to `production`, so we strongly recommend you use this for your production env name. + +### Creating your config + +Each config file is a JavaScript file which exports a single object. Within this file, settings are grouped by module: + +```Javascript +export default { + 'modulename': { + // settings + } +}; +``` + +See [this page](configuration) for a complete list of all configuration options. + +#### +For convenience, we've bundled a script which will generate a new config file for you automatically. + +You can do this by running the following: +```bash +npx at-confgen [NODE_ENV] +``` + +> If you choose to include the default settings in your configuration, please be aware that once set, these values will not be updated if the defaults change in the future. It is advised therefore that you leave out any settings that you don't wish to change. + +See the [Bin scripts](binscripts#at-confgen) page for more information, included supported flags. + +### Setting your 'env' + +You can do this temporarily using the following: + +**Bash/Mac OS Terminal**: +```bash +$ NODE_ENV=dev npm start +``` +**Windows Powershell/Command Prompt**: +```bash +> set NODE_ENV=dev | npm start +``` + +Please see the documentation for your own operating system for instructions on how to save environment variables in a more permanent way. + +## Using environment variables for configuration + +As an alternative to config files, any configuration option can be set via an environment variable using the following naming convention: + +``` +ADAPT_AUTHORING___ +``` + +- The module name is converted to uppercase with underscores replacing hyphens (e.g. `adapt-authoring-server` becomes `ADAPT_AUTHORING_SERVER`) +- A double underscore (`__`) separates the module name from the property name +- The property name is kept in its original camelCase format + +For example: + +| Environment variable | Config equivalent | +| --- | --- | +| `ADAPT_AUTHORING_SERVER__host` | `adapt-authoring-server.host` | +| `ADAPT_AUTHORING_SERVER__port` | `adapt-authoring-server.port` | +| `ADAPT_AUTHORING_MONGODB__connectionUri` | `adapt-authoring-mongodb.connectionUri` | +| `ADAPT_AUTHORING_AUTH_LOCAL__saltRounds` | `adapt-authoring-auth-local.saltRounds` | + +Values are parsed as JSON where possible, so non-string types like numbers and booleans can be set directly (e.g. `ADAPT_AUTHORING_SERVER__port=5678`). + +Any environment variables that do not start with `ADAPT_AUTHORING_` are available under the `env` namespace (e.g. `NODE_ENV` becomes `env.NODE_ENV`). diff --git a/docs/error-handling.md b/docs/error-handling.md new file mode 100644 index 00000000..e7c54974 --- /dev/null +++ b/docs/error-handling.md @@ -0,0 +1,50 @@ +# Error handling +Handling errors correctly is a key aspect of writing stable software. This guide will give some tips on how you should deal with errors, as well as the utilities available to make error handling simpler. + +Before going into specifics, it would be useful to discuss application errors in general terms. The errors you experience are likely to fall into one of the following broad groups: +- **Initialisation errors**: i.e. problems during start-up +- **General server errors**: errors which occur outside of user requests, possibly during automated tasks +- **User errors**: errors which are a direct result of a user request + +You will need to deal with each category of error differently. Below are some general tips on handling each type of error. + +## Initialisation errors +## Initialisation errors +Any errors which occur during initialisation should be captured and logged as appropriate. Depending on the type of error, it may or may not be considered fatal to your code. + +Some examples: +- For a database-handler module, failing to connect to the database would be considered a fatal error, as no further actions can be executed. In this case, the code should perform any clean-up and exit. +- For a configuration module, failing to load the user configuration file may not be fatal if the application can run without it (e.g. with default settings). In this case the error should be logged, but the code can continue to initialise post-error. +- For a module which attempts to load a specific file in each module connected to the core system, failing to load a single configuration file may not be an error as such, but rather an expected outcome if the configuration file in question is not something that's required to be defined for every module. In this case, the code can continue and it may not even be necessary to log a message. + +## General server errors +'General server errors' is a broad category which covers other errors that don't take place at either initialisation or as a result of direct user action. Again, depending on the specific error, these may or may not be fatal. + +Some examples: +- For a database-handler module, disconnecting from the database is an expected error, and can be handled and rectified easily. + +## User errors errors +User errors are any errors which are caused as a direct result of a user performing an action incorrectly. It is even *more* critical with user errors that the error is as specific and descriptive as possible, as the response needs to be informative and instructive to the user that caused the error. Failing to do so will result in an unpleasant user experience. + +Some examples: +- A user uploads a file in an invalid format. This definitely isn't a fatal error, as the code can continue post-error. The returned error should inform the user of the issue, as well as how it can be rectified. + +## Defining errors +Depending on the kinds of error that you're dealing with in your code, it may be useful to include a set of custom error definitions specific to your code. + +Defining useful errors is a critical part of any software system. The ErrorsModule makes it easy to define errors for your own modules, and make use of errors defined in other modules. + +## Catching errors + +## Throwing errors +As mentioned above, it is preferable to catch errors internally in your code and re-throw these errors + +The ErrorsModule acts as a central store for all errors defined in the system, and errors can be accessed and thrown from here. For convenience, the Errors module is made available directly as a property of the main App instance, or by referencing the module in the usual way: `App#waitForModule('errors)` + +```js +try { + // do something here +} catch(e) { + throw this.app.errors.MY_ERROR +} +``` diff --git a/docs/plugins/configuration.js b/docs/plugins/configuration.js new file mode 100644 index 00000000..204ae5e9 --- /dev/null +++ b/docs/plugins/configuration.js @@ -0,0 +1,75 @@ +import fs from 'fs' +import path from 'path' + +export default class Configuration { + async run () { + const schemas = this.loadSchemas() + this.contents = Object.keys(schemas).sort() + this.manualFile = 'configuration.md' + this.replace = { + CODE_EXAMPLE: this.generateCodeExample(schemas), + LIST: this.generateList(schemas) + } + } + + loadSchemas () { + const schemas = {} + Object.values(this.app.dependencies).forEach(c => { + const confDir = path.join(c.rootDir, 'conf') + try { + schemas[c.name] = JSON.parse(fs.readFileSync(path.join(confDir, 'config.schema.json'))) + } catch (e) {} + }) + return schemas + } + + generateCodeExample (schemas) { + let output = '```javascript\nexport default {\n' + this.contents.forEach((name) => { + const schema = schemas[name] + output += ` '${name}': {\n` + Object.entries(schema.properties).forEach(([attr, config]) => { + const required = schema.required && schema.required.includes(attr) + if (config.description) output += ` // ${config.description}\n` + output += ` ${attr}: ${this.defaultToMd(config)}, // ${config.type}, ${required ? 'required' : 'optional'}\n` + }) + output += ' },\n' + }) + output += '};\n```' + return output + } + + generateList (schemas) { + let output = '' + + this.contents.forEach(dep => { + const schema = schemas[dep] + output += `

${dep}

\n\n` + output += '
\n' + Object.entries(schema.properties).forEach(([attr, config]) => { + const required = schema.required && schema.required.includes(attr) + output += '
\n' + output += `
${attr} (${config.type || ''}, ${required ? 'required' : 'optional'})
\n` + output += '
\n' + output += `
${config.description}
\n` + if (!required) { + output += `
Default:
${this.defaultToMd(config)}
\n` + } + output += '
\n' + output += '
\n' + }) + output += '
' + output += '\n\n' + }) + + return output + } + + /** + * Returns a string formatted nicely for markdown + */ + defaultToMd (config) { + const s = JSON.stringify(config.default, null, 2) + return s?.length < 75 ? s : s?.replaceAll('\n', '\n ') + } +} diff --git a/docs/plugins/configuration.md b/docs/plugins/configuration.md new file mode 100644 index 00000000..a920420c --- /dev/null +++ b/docs/plugins/configuration.md @@ -0,0 +1,14 @@ +# Configuration reference +This page lists all configuration options supported by the [core bundle](coremodules) of Adapt authoring modules. For details on how to set up your configuration, including using environment variables, see [Configuring your environment](configure-environment). + +{{{TABLE_OF_CONTENTS}}} + +## Quick reference +See below for an overview of all available configuration options. + +{{{CODE_EXAMPLE}}} + +## Complete reference +See below for a full list of available configuration options. + +{{{LIST}}} diff --git a/docs/plugins/errors.js b/docs/plugins/errors.js new file mode 100644 index 00000000..3ef6efc2 --- /dev/null +++ b/docs/plugins/errors.js @@ -0,0 +1,21 @@ +export default class Errors { + async run () { + this.manualFile = 'errorsref.md' + this.contents = Object.keys(this.app.errors) + this.replace = { ERRORS: this.generateMd() } + } + + generateMd () { + return Object.keys(this.app.errors).reduce((md, k) => { + const e = this.app.errors[k] + return `${md}\n| \`${e.code}\` | ${e.meta.description} | ${e.statusCode} |
    ${this.dataToMd(e.meta.data)}
|` + }, '| Error code | Description | HTTP status code | Supplemental data |\n| - | - | :-: | - |') + } + + dataToMd (data, s = '') { + if (!data) return s + return Object.entries(data).reduce((s, [k, v]) => { + return `${s}
  • \`${k}\`: ${typeof v === 'object' ? this.dataToMd(v, s) : v}
  • ` + }, s) + } +} diff --git a/docs/plugins/errorsref.md b/docs/plugins/errorsref.md new file mode 100644 index 00000000..fc7dd523 --- /dev/null +++ b/docs/plugins/errorsref.md @@ -0,0 +1,9 @@ +# Errors Reference + +This page documents all errors which are likely to be thrown in the system, along with the appropriate HTTP status code and any supplemental data which is stored with the error. + +Supplemental data can be used at the point that errors are translated to provide more context to a specific error. All data stored with an error can be assumed to be a primitive type for easy printing. + +{{{TABLE_OF_CONTENTS}}} + +{{{ERRORS}}} From 6da2b2faa30c8f4fc66c20a5fd3fac7659f1ceb0 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 14:28:31 +0000 Subject: [PATCH 08/49] Update: Remove checkDeprecatedConfig in favour of migrations module Config deprecation warnings are superseded by the migrations module's new conf migration type, which can actually rewrite config files rather than just warning about stale keys. --- lib/Config.js | 42 +------------------------ tests/Config.spec.js | 75 -------------------------------------------- 2 files changed, 1 insertion(+), 116 deletions(-) diff --git a/lib/Config.js b/lib/Config.js index cb4af17b..092bd898 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -1,7 +1,6 @@ import { Schemas } from 'adapt-schemas' import chalk from 'chalk' import fs from 'fs/promises' -import { glob } from 'glob' import path from 'path' /** @@ -20,8 +19,7 @@ class Config { static async load (rootDir, dependencies, appName) { const instance = new Config() instance.configFilePath = path.join(rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) - const userConfig = await instance.storeUserSettings() - if (userConfig) await instance.checkDeprecatedConfig(userConfig, dependencies) + await instance.storeUserSettings() instance.storeEnvSettings() instance.storeSchemaSettings(dependencies, appName) return instance @@ -100,44 +98,6 @@ class Config { return config } - /** - * Scans all dependencies for conf/deprecated*.json files and checks the user config for deprecated keys. - * Each file maps old key names to new key names (or null if removed). The reserved `_module` key - * can be used to check a different module section in the user config (for keys that moved between modules). - * @param {Object} userConfig The parsed user config object - * @param {Object} dependencies Key/value map of dependency configs - * @return {Promise} - */ - async checkDeprecatedConfig (userConfig, dependencies) { - const warnings = [] - await Promise.all(Object.values(dependencies).map(async dep => { - const files = await glob('conf/deprecated*.json', { cwd: dep.rootDir, absolute: true }) - await Promise.all(files.map(async f => { - try { - const mappings = JSON.parse(await fs.readFile(f)) - const modName = mappings._module ?? dep.name - delete mappings._module - if (!userConfig[modName]) return - for (const [oldKey, newKey] of Object.entries(mappings)) { - if (!(oldKey in userConfig[modName])) continue - if (newKey) { - warnings.push(` '${modName}.${oldKey}' has moved to '${newKey}'`) - } else { - warnings.push(` '${modName}.${oldKey}' has been removed`) - } - } - } catch (e) {} // ignore malformed files - })) - })) - if (warnings.length) { - console.log(chalk.yellow([ - '\nDeprecated config keys detected. Please update your config file:\n', - ...warnings, - '\n' - ].join('\n'))) - } - } - /** * Copy env values to config */ diff --git a/tests/Config.spec.js b/tests/Config.spec.js index a402a449..768bc746 100644 --- a/tests/Config.spec.js +++ b/tests/Config.spec.js @@ -101,79 +101,4 @@ describe('Config', () => { }) }) - describe('#checkDeprecatedConfig()', () => { - let deprecatedDir - - before(async () => { - deprecatedDir = path.join(__dirname, 'data', 'deprecated-test') - await fs.ensureDir(path.join(deprecatedDir, 'conf')) - await fs.writeJson(path.join(deprecatedDir, 'conf', 'deprecated.json'), { - oldKey: 'test-module.newKey', - removedKey: null - }) - }) - - after(async () => { - await fs.remove(deprecatedDir) - }) - - it('should warn on moved keys', async () => { - let output = '' - const originalLog = console.log - console.log = (msg) => { output += msg } - const config = new Config() - await config.checkDeprecatedConfig( - { 'test-module': { oldKey: 'value' } }, - { test: { name: 'test-module', rootDir: deprecatedDir } } - ) - console.log = originalLog - assert.ok(output.includes("'test-module.oldKey' has moved to 'test-module.newKey'")) - }) - - it('should warn on removed keys', async () => { - let output = '' - const originalLog = console.log - console.log = (msg) => { output += msg } - const config = new Config() - await config.checkDeprecatedConfig( - { 'test-module': { removedKey: true } }, - { test: { name: 'test-module', rootDir: deprecatedDir } } - ) - console.log = originalLog - assert.ok(output.includes('has been removed')) - }) - - it('should not warn when no deprecated keys present', async () => { - let output = '' - const originalLog = console.log - console.log = (msg) => { output += msg } - const config = new Config() - await config.checkDeprecatedConfig( - { 'test-module': { safeKey: 'value' } }, - { test: { name: 'test-module', rootDir: deprecatedDir } } - ) - console.log = originalLog - assert.equal(output, '') - }) - - it('should use _module override to check a different module section', async () => { - const overrideDir = path.join(__dirname, 'data', 'deprecated-override') - await fs.ensureDir(path.join(overrideDir, 'conf')) - await fs.writeJson(path.join(overrideDir, 'conf', 'deprecated.json'), { - _module: 'old-module', - oldKey: 'new-module.newKey' - }) - let output = '' - const originalLog = console.log - console.log = (msg) => { output += msg } - const config = new Config() - await config.checkDeprecatedConfig( - { 'old-module': { oldKey: 'value' } }, - { test: { name: 'new-module', rootDir: overrideDir } } - ) - console.log = originalLog - assert.ok(output.includes("'old-module.oldKey' has moved to 'new-module.newKey'")) - await fs.remove(overrideDir) - }) - }) }) From 54bbd5887fc1f2de2d9d3a2d87076d3a24b20bd5 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 14:48:08 +0000 Subject: [PATCH 09/49] New: Add config migrations for absorbed logger and lang modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrates config keys from their old module sections to adapt-authoring-core: - adapt-authoring-logger.levels → adapt-authoring-core.logLevels - adapt-authoring-logger.showTimestamp → adapt-authoring-core.showLogTimestamp - adapt-authoring-logger.mute → removed - adapt-authoring-logger.dateFormat → removed - adapt-authoring-lang.defaultLang → adapt-authoring-core.defaultLang --- migrations/3.0.0-conf-migrate-lang-config.js | 40 +++++++++++++ .../3.0.0-conf-migrate-logger-config.js | 56 +++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 migrations/3.0.0-conf-migrate-lang-config.js create mode 100644 migrations/3.0.0-conf-migrate-logger-config.js diff --git a/migrations/3.0.0-conf-migrate-lang-config.js b/migrations/3.0.0-conf-migrate-lang-config.js new file mode 100644 index 00000000..c452aad1 --- /dev/null +++ b/migrations/3.0.0-conf-migrate-lang-config.js @@ -0,0 +1,40 @@ +import fs from 'fs/promises' +import path from 'path' +import { glob } from 'glob' + +export default function (migration) { + migration.describe('Move adapt-authoring-lang config keys to adapt-authoring-core') + + migration.run(async ({ appDir, dryRun, log }) => { + const files = await glob('conf/*.config.js', { cwd: appDir, absolute: true }) + for (const filePath of files) { + const config = (await import(filePath)).default + const lang = config['adapt-authoring-lang'] + if (!lang) continue + + const core = config['adapt-authoring-core'] || {} + let changed = false + + if ('defaultLang' in lang) { + core.defaultLang = lang.defaultLang + delete lang.defaultLang + changed = true + log('info', `${path.basename(filePath)}: moved defaultLang to adapt-authoring-core`) + } + + if (!changed) continue + + config['adapt-authoring-core'] = core + if (!Object.keys(lang).length) { + delete config['adapt-authoring-lang'] + } + + const output = `export default ${JSON.stringify(config, null, 2)}\n` + if (dryRun) { + log('info', `[DRY RUN] would write ${filePath}`) + continue + } + await fs.writeFile(filePath, output, 'utf8') + } + }) +} diff --git a/migrations/3.0.0-conf-migrate-logger-config.js b/migrations/3.0.0-conf-migrate-logger-config.js new file mode 100644 index 00000000..e01486a4 --- /dev/null +++ b/migrations/3.0.0-conf-migrate-logger-config.js @@ -0,0 +1,56 @@ +import fs from 'fs/promises' +import path from 'path' +import { glob } from 'glob' + +export default function (migration) { + migration.describe('Move adapt-authoring-logger config keys to adapt-authoring-core') + + migration.run(async ({ appDir, dryRun, log }) => { + const files = await glob('conf/*.config.js', { cwd: appDir, absolute: true }) + for (const filePath of files) { + const config = (await import(filePath)).default + const logger = config['adapt-authoring-logger'] + if (!logger) continue + + const core = config['adapt-authoring-core'] || {} + let changed = false + + if ('levels' in logger) { + core.logLevels = logger.levels + delete logger.levels + changed = true + log('info', `${path.basename(filePath)}: moved levels → logLevels`) + } + if ('showTimestamp' in logger) { + core.showLogTimestamp = logger.showTimestamp + delete logger.showTimestamp + changed = true + log('info', `${path.basename(filePath)}: moved showTimestamp → showLogTimestamp`) + } + if ('mute' in logger) { + delete logger.mute + changed = true + log('info', `${path.basename(filePath)}: removed mute (use empty logLevels array instead)`) + } + if ('dateFormat' in logger) { + delete logger.dateFormat + changed = true + log('info', `${path.basename(filePath)}: removed dateFormat`) + } + + if (!changed) continue + + config['adapt-authoring-core'] = core + if (!Object.keys(logger).length) { + delete config['adapt-authoring-logger'] + } + + const output = `export default ${JSON.stringify(config, null, 2)}\n` + if (dryRun) { + log('info', `[DRY RUN] would write ${filePath}`) + continue + } + await fs.writeFile(filePath, output, 'utf8') + } + }) +} From 0db6a9db0668578850e5f9c314a295dee250d00e Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 15:00:28 +0000 Subject: [PATCH 10/49] Update: Simplify config migrations to use where/mutate DSL --- migrations/3.0.0-conf-migrate-lang-config.js | 31 +++---------- .../3.0.0-conf-migrate-logger-config.js | 45 +++---------------- 2 files changed, 12 insertions(+), 64 deletions(-) diff --git a/migrations/3.0.0-conf-migrate-lang-config.js b/migrations/3.0.0-conf-migrate-lang-config.js index c452aad1..99bbadc7 100644 --- a/migrations/3.0.0-conf-migrate-lang-config.js +++ b/migrations/3.0.0-conf-migrate-lang-config.js @@ -1,40 +1,19 @@ -import fs from 'fs/promises' -import path from 'path' -import { glob } from 'glob' - export default function (migration) { migration.describe('Move adapt-authoring-lang config keys to adapt-authoring-core') - migration.run(async ({ appDir, dryRun, log }) => { - const files = await glob('conf/*.config.js', { cwd: appDir, absolute: true }) - for (const filePath of files) { - const config = (await import(filePath)).default + migration + .where('adapt-authoring-lang') + .mutate(config => { const lang = config['adapt-authoring-lang'] - if (!lang) continue - - const core = config['adapt-authoring-core'] || {} - let changed = false + const core = config['adapt-authoring-core'] ||= {} if ('defaultLang' in lang) { core.defaultLang = lang.defaultLang delete lang.defaultLang - changed = true - log('info', `${path.basename(filePath)}: moved defaultLang to adapt-authoring-core`) } - if (!changed) continue - - config['adapt-authoring-core'] = core if (!Object.keys(lang).length) { delete config['adapt-authoring-lang'] } - - const output = `export default ${JSON.stringify(config, null, 2)}\n` - if (dryRun) { - log('info', `[DRY RUN] would write ${filePath}`) - continue - } - await fs.writeFile(filePath, output, 'utf8') - } - }) + }) } diff --git a/migrations/3.0.0-conf-migrate-logger-config.js b/migrations/3.0.0-conf-migrate-logger-config.js index e01486a4..158b6430 100644 --- a/migrations/3.0.0-conf-migrate-logger-config.js +++ b/migrations/3.0.0-conf-migrate-logger-config.js @@ -1,56 +1,25 @@ -import fs from 'fs/promises' -import path from 'path' -import { glob } from 'glob' - export default function (migration) { migration.describe('Move adapt-authoring-logger config keys to adapt-authoring-core') - migration.run(async ({ appDir, dryRun, log }) => { - const files = await glob('conf/*.config.js', { cwd: appDir, absolute: true }) - for (const filePath of files) { - const config = (await import(filePath)).default + migration + .where('adapt-authoring-logger') + .mutate(config => { const logger = config['adapt-authoring-logger'] - if (!logger) continue - - const core = config['adapt-authoring-core'] || {} - let changed = false + const core = config['adapt-authoring-core'] ||= {} if ('levels' in logger) { core.logLevels = logger.levels delete logger.levels - changed = true - log('info', `${path.basename(filePath)}: moved levels → logLevels`) } if ('showTimestamp' in logger) { core.showLogTimestamp = logger.showTimestamp delete logger.showTimestamp - changed = true - log('info', `${path.basename(filePath)}: moved showTimestamp → showLogTimestamp`) - } - if ('mute' in logger) { - delete logger.mute - changed = true - log('info', `${path.basename(filePath)}: removed mute (use empty logLevels array instead)`) - } - if ('dateFormat' in logger) { - delete logger.dateFormat - changed = true - log('info', `${path.basename(filePath)}: removed dateFormat`) } + delete logger.mute + delete logger.dateFormat - if (!changed) continue - - config['adapt-authoring-core'] = core if (!Object.keys(logger).length) { delete config['adapt-authoring-logger'] } - - const output = `export default ${JSON.stringify(config, null, 2)}\n` - if (dryRun) { - log('info', `[DRY RUN] would write ${filePath}`) - continue - } - await fs.writeFile(filePath, output, 'utf8') - } - }) + }) } From 917c49bd0fb091d3df46ac90a19fa6c5ce645dc5 Mon Sep 17 00:00:00 2001 From: Tom Taylor Date: Thu, 26 Mar 2026 15:09:56 +0000 Subject: [PATCH 11/49] Simplify config migration --- migrations/3.0.0-conf-migrate-lang-config.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/migrations/3.0.0-conf-migrate-lang-config.js b/migrations/3.0.0-conf-migrate-lang-config.js index 99bbadc7..b7fa8349 100644 --- a/migrations/3.0.0-conf-migrate-lang-config.js +++ b/migrations/3.0.0-conf-migrate-lang-config.js @@ -7,12 +7,8 @@ export default function (migration) { const lang = config['adapt-authoring-lang'] const core = config['adapt-authoring-core'] ||= {} - if ('defaultLang' in lang) { + if (lang.defaultLang && !core.defaultLang) { core.defaultLang = lang.defaultLang - delete lang.defaultLang - } - - if (!Object.keys(lang).length) { delete config['adapt-authoring-lang'] } }) From 77da6e2a8de99f2d10dde6a367fc3e0a63aaf96c Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 15:22:40 +0000 Subject: [PATCH 12/49] Update: Use replace/remove DSL in config migrations --- migrations/3.0.0-conf-migrate-lang-config.js | 10 +-------- .../3.0.0-conf-migrate-logger-config.js | 22 +++---------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/migrations/3.0.0-conf-migrate-lang-config.js b/migrations/3.0.0-conf-migrate-lang-config.js index b7fa8349..cc224870 100644 --- a/migrations/3.0.0-conf-migrate-lang-config.js +++ b/migrations/3.0.0-conf-migrate-lang-config.js @@ -3,13 +3,5 @@ export default function (migration) { migration .where('adapt-authoring-lang') - .mutate(config => { - const lang = config['adapt-authoring-lang'] - const core = config['adapt-authoring-core'] ||= {} - - if (lang.defaultLang && !core.defaultLang) { - core.defaultLang = lang.defaultLang - delete config['adapt-authoring-lang'] - } - }) + .replace('defaultLang', 'adapt-authoring-core') } diff --git a/migrations/3.0.0-conf-migrate-logger-config.js b/migrations/3.0.0-conf-migrate-logger-config.js index 158b6430..e4e79c0e 100644 --- a/migrations/3.0.0-conf-migrate-logger-config.js +++ b/migrations/3.0.0-conf-migrate-logger-config.js @@ -3,23 +3,7 @@ export default function (migration) { migration .where('adapt-authoring-logger') - .mutate(config => { - const logger = config['adapt-authoring-logger'] - const core = config['adapt-authoring-core'] ||= {} - - if ('levels' in logger) { - core.logLevels = logger.levels - delete logger.levels - } - if ('showTimestamp' in logger) { - core.showLogTimestamp = logger.showTimestamp - delete logger.showTimestamp - } - delete logger.mute - delete logger.dateFormat - - if (!Object.keys(logger).length) { - delete config['adapt-authoring-logger'] - } - }) + .replace('levels', 'adapt-authoring-core', 'logLevels') + .replace('showTimestamp', 'adapt-authoring-core', 'showLogTimestamp') + .remove('mute', 'dateFormat') } From e8a7c563b67674afa15dede4866675b221d67e6f Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 16:47:48 +0000 Subject: [PATCH 13/49] Fix: Remove blank line padding in test block --- tests/Config.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Config.spec.js b/tests/Config.spec.js index 768bc746..310d9365 100644 --- a/tests/Config.spec.js +++ b/tests/Config.spec.js @@ -100,5 +100,4 @@ describe('Config', () => { await assert.doesNotReject(() => config.storeUserSettings()) }) }) - }) From 45ac51c2830376681f7761b2a10a846618aceb8d Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Thu, 26 Mar 2026 18:18:11 +0000 Subject: [PATCH 14/49] Fix: Resolve directory path variables in Config Adds resolveDirectory() to Config which handles $ROOT, $DATA, $TEMP variable substitution. This was previously done by jsonschema's isDirectory keyword, but config now loads before jsonschema. JsonSchemaModule's isDirectory keyword now delegates to this method. --- lib/Config.js | 34 +++++++++++++++++++++++++++++++++- tests/Config.spec.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/lib/Config.js b/lib/Config.js index 092bd898..6becfaa3 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -18,6 +18,7 @@ class Config { */ static async load (rootDir, dependencies, appName) { const instance = new Config() + instance.rootDir = rootDir instance.configFilePath = path.join(rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) await instance.storeUserSettings() instance.storeEnvSettings() @@ -28,6 +29,11 @@ class Config { constructor () { /** @ignore */ this._config = {} + /** + * Application root directory + * @type {String} + */ + this.rootDir = undefined /** * Path to the user configuration file * @type {String} @@ -156,8 +162,10 @@ class Config { } catch (e) { return } + const dirKeys = new Set() let data = Object.entries(schema.raw.properties).reduce((m, [k, v]) => { if (v?._adapt?.isPublic) this.publicAttributes.push(`${pkg.name}.${k}`) + if (v?.isDirectory) dirKeys.add(k) return { ...m, [k]: this.get(`${pkg.name}.${k}`) } }, {}) try { @@ -166,7 +174,31 @@ class Config { e.modName = pkg.name throw e } - Object.entries(data).forEach(([key, val]) => { this._config[`${pkg.name}.${key}`] = val }) + Object.entries(data).forEach(([key, val]) => { + if (dirKeys.has(key) && typeof val === 'string') { + val = this.resolveDirectory(val) + } + this._config[`${pkg.name}.${key}`] = val + }) + } + + /** + * Resolves directory path variables ($ROOT, $DATA, $TEMP) + * @param {String} value The path string to resolve + * @return {String} + */ + resolveDirectory (value) { + const vars = [ + ['$ROOT', this.rootDir], + ['$DATA', this._config['adapt-authoring-core.dataDir']], + ['$TEMP', this._config['adapt-authoring-core.tempDir']] + ] + for (const [key, replacement] of vars) { + if (value.startsWith(key) && replacement && !replacement.startsWith('$')) { + return path.resolve(replacement, value.replace(key, '').slice(1)) + } + } + return value } /** diff --git a/tests/Config.spec.js b/tests/Config.spec.js index 310d9365..0925e806 100644 --- a/tests/Config.spec.js +++ b/tests/Config.spec.js @@ -82,6 +82,40 @@ describe('Config', () => { }) }) + describe('#resolveDirectory()', () => { + it('should resolve $ROOT', () => { + const config = new Config() + config.rootDir = '/app' + assert.equal(config.resolveDirectory('$ROOT/APP_DATA/data'), path.resolve('/app', 'APP_DATA/data')) + }) + + it('should resolve $DATA', () => { + const config = new Config() + config.rootDir = '/app' + config._config['adapt-authoring-core.dataDir'] = '/app/APP_DATA/data' + assert.equal(config.resolveDirectory('$DATA/uploads'), path.resolve('/app/APP_DATA/data', 'uploads')) + }) + + it('should resolve $TEMP', () => { + const config = new Config() + config.rootDir = '/app' + config._config['adapt-authoring-core.tempDir'] = '/app/APP_DATA/temp' + assert.equal(config.resolveDirectory('$TEMP/cache'), path.resolve('/app/APP_DATA/temp', 'cache')) + }) + + it('should not resolve unresolved variables', () => { + const config = new Config() + config.rootDir = '/app' + assert.equal(config.resolveDirectory('$DATA/uploads'), '$DATA/uploads') + }) + + it('should return non-variable paths unchanged', () => { + const config = new Config() + config.rootDir = '/app' + assert.equal(config.resolveDirectory('/absolute/path'), '/absolute/path') + }) + }) + describe('#storeUserSettings()', () => { let confDir From 7e52b4041e021537c96071e517fd1fc9d6b311ae Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 00:45:41 +0000 Subject: [PATCH 15/49] Fix: Run migrations at boot, fix schema defaults, improve config errors - Call runMigrations() in App.start() between loadConfigs() and loadLibraries(), so config migrations run before schema validation - Fix Schemas().init() not being called in Config.storeSchemaSettings(), which caused all schema defaults to be silently skipped - Improve config validation error formatting with chalk colours Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 39 +++++++++++++++++++++++++++++++++++++++ lib/Config.js | 20 +++++++++++--------- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/lib/App.js b/lib/App.js index 01894e6e..66e7a81a 100644 --- a/lib/App.js +++ b/lib/App.js @@ -1,4 +1,5 @@ import AbstractModule from './AbstractModule.js' +import chalk from 'chalk' import Config from './Config.js' import DependencyLoader from './DependencyLoader.js' import Errors from './Errors.js' @@ -113,12 +114,50 @@ class App extends AbstractModule { this._isStarting = true await this.dependencyloader.loadConfigs() + await this.runMigrations() await this.loadLibraries() await this.dependencyloader.load() this._isStarting = false } + /** + * Runs pending migrations before config validation and module loading. + * Reads the user config file directly to obtain the MongoDB connection URI. + * @return {Promise} + */ + async runMigrations () { + const configFilePath = path.join(this.rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) + let userConfig + try { + userConfig = (await import(configFilePath)).default + } catch (e) { + return // no config file, nothing to migrate + } + const connectionUri = userConfig['adapt-authoring-mongodb']?.connectionUri + if (!connectionUri) return + + const log = (level, ...args) => { + const tag = chalk.cyan(args[0]) + const msg = args.slice(1).join(' ') + if (level === 'error') { + console.log(chalk.red(`${tag} ${msg}`)) + } else if (level === 'warn') { + console.log(chalk.yellow(`${tag} ${msg}`)) + } else { + console.log(`${tag} ${msg}`) + } + } + const { runMigrations } = await import('adapt-authoring-migrations') + await runMigrations({ + dependencies: this.dependencies, + connectionUri, + rootDir: this.rootDir, + log, + dryRun: this.args['dry-run'] === true + }) + } + /** * Loads the core bootstrap libraries (config, logger, errors, lang) before any modules initialise. * @return {Promise} diff --git a/lib/Config.js b/lib/Config.js index 6becfaa3..b46e8da4 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -122,7 +122,7 @@ class Config { * @param {String} appName The core module name (for sorting) */ storeSchemaSettings (dependencies, appName) { - const schemas = new Schemas() + const schemas = new Schemas().init() const isCore = d => d.name === appName const deps = Object.values(dependencies).sort((a, b) => { if (isCore(a)) return -1 @@ -132,20 +132,22 @@ class Config { const coreDep = deps.find(d => isCore(d)) if (coreDep) this.processModuleSchema(coreDep, schemas) - let hasErrored = false + const errors = [] for (const d of deps.filter(d => !isCore(d))) { try { this.processModuleSchema(d, schemas) } catch (e) { - hasErrored = true - if (e?.data?.errors) { - console.log(`${e.modName}: ${e.data.errors}`) - } else { - console.log(e) - } + errors.push(e?.data?.errors ? { modName: e.modName, message: e.data.errors } : { message: String(e) }) } } - if (hasErrored) throw new Error('Config validation failed') + if (errors.length) { + console.log(chalk.red('\nConfig validation failed:\n')) + errors.forEach(e => { + console.log(` • ${e.modName ? chalk.cyan(e.modName) + ': ' : ''}${e.message}`) + }) + console.log() + throw new Error('Config validation failed') + } } /** From 7979b165e1d75718bf240e46f46f5460cb4ee41c Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 09:22:01 +0000 Subject: [PATCH 16/49] Update: Initialise Logger early so all boot phases can use it Create Logger with defaults at the start of App.start(), before loadConfigs/migrations/loadLibraries. Config and migrations now use the logger instead of raw console.log + chalk. Logger is reconfigured with user settings once Config is loaded. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 25 ++++++++----------------- lib/Config.js | 11 +++++------ 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/lib/App.js b/lib/App.js index 66e7a81a..ec6c32da 100644 --- a/lib/App.js +++ b/lib/App.js @@ -1,5 +1,4 @@ import AbstractModule from './AbstractModule.js' -import chalk from 'chalk' import Config from './Config.js' import DependencyLoader from './DependencyLoader.js' import Errors from './Errors.js' @@ -113,6 +112,12 @@ class App extends AbstractModule { this._isStarting = true + /** + * Reference to the Logger instance + * @type {Logger} + */ + this.logger = new Logger() + await this.dependencyloader.loadConfigs() await this.runMigrations() await this.loadLibraries() @@ -137,17 +142,7 @@ class App extends AbstractModule { const connectionUri = userConfig['adapt-authoring-mongodb']?.connectionUri if (!connectionUri) return - const log = (level, ...args) => { - const tag = chalk.cyan(args[0]) - const msg = args.slice(1).join(' ') - if (level === 'error') { - console.log(chalk.red(`${tag} ${msg}`)) - } else if (level === 'warn') { - console.log(chalk.yellow(`${tag} ${msg}`)) - } else { - console.log(`${tag} ${msg}`) - } - } + const log = (level, ...args) => this.logger.log(level, ...args) const { runMigrations } = await import('adapt-authoring-migrations') await runMigrations({ dependencies: this.dependencies, @@ -168,11 +163,7 @@ class App extends AbstractModule { * Reference to the Config instance * @type {Config} */ - this.config = await Config.load(this.rootDir, deps, this.name) - /** - * Reference to the Logger instance - * @type {Logger} - */ + this.config = await Config.load(this.rootDir, deps, this.name, this.logger) this.logger = new Logger({ levels: this.config.get('adapt-authoring-core.logLevels'), showTimestamp: this.config.get('adapt-authoring-core.showLogTimestamp') diff --git a/lib/Config.js b/lib/Config.js index b46e8da4..7c43255c 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -1,5 +1,4 @@ import { Schemas } from 'adapt-schemas' -import chalk from 'chalk' import fs from 'fs/promises' import path from 'path' @@ -14,11 +13,13 @@ class Config { * @param {String} rootDir Application root directory * @param {Object} dependencies Key/value map of dependency configs * @param {String} appName The core module name (for sorting) + * @param {Logger} logger Logger instance for output * @returns {Promise} */ - static async load (rootDir, dependencies, appName) { + static async load (rootDir, dependencies, appName, logger) { const instance = new Config() instance.rootDir = rootDir + instance.logger = logger instance.configFilePath = path.join(rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) await instance.storeUserSettings() instance.storeEnvSettings() @@ -93,7 +94,7 @@ class Config { configError = e.toString() } if (configError) { - console.log(chalk.yellow(`Failed to load config at ${this.configFilePath}:\n\n${configError}\n\nWill attempt to run with defaults.\n`)) + this.logger.log('warn', 'config', `Failed to load config at ${this.configFilePath}: ${configError}. Will attempt to run with defaults.`) return } Object.entries(config).forEach(([name, c]) => { @@ -141,11 +142,9 @@ class Config { } } if (errors.length) { - console.log(chalk.red('\nConfig validation failed:\n')) errors.forEach(e => { - console.log(` • ${e.modName ? chalk.cyan(e.modName) + ': ' : ''}${e.message}`) + this.logger.log('error', 'config', `${e.modName ? e.modName + ': ' : ''}${e.message}`) }) - console.log() throw new Error('Config validation failed') } } From f6126161177aea25125b3f5993d6281da3d41c9c Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 09:24:17 +0000 Subject: [PATCH 17/49] Update: Use getConfig() for core config access in App Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/App.js b/lib/App.js index ec6c32da..086eaaea 100644 --- a/lib/App.js +++ b/lib/App.js @@ -62,8 +62,8 @@ class App extends AbstractModule { await this.start() this.log('verbose', 'GIT', 'INFO', this.git) this.log('verbose', 'DIR', 'rootDir', this.rootDir) - this.log('verbose', 'DIR', 'dataDir', this.config.get('adapt-authoring-core.dataDir')) - this.log('verbose', 'DIR', 'tempDir', this.config.get('adapt-authoring-core.tempDir')) + this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) + this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) } catch (e) { startError = e } @@ -165,8 +165,8 @@ class App extends AbstractModule { */ this.config = await Config.load(this.rootDir, deps, this.name, this.logger) this.logger = new Logger({ - levels: this.config.get('adapt-authoring-core.logLevels'), - showTimestamp: this.config.get('adapt-authoring-core.showLogTimestamp') + levels: this.getConfig('logLevels'), + showTimestamp: this.getConfig('showLogTimestamp') }) this.logger.log('info', 'config', `using config at ${this.config.configFilePath}`) /** @@ -181,7 +181,7 @@ class App extends AbstractModule { this.lang = new Lang() await this.lang.loadPhrases(deps, this.rootDir, (...args) => this.logger.log('error', 'lang', ...args)) - const configRootDir = this.config.get('adapt-authoring-core.rootDir') + const configRootDir = this.getConfig('rootDir') if (configRootDir) /** @ignore */this.rootDir = configRootDir } From de655db20e14f84552de584c3a3871380affdb4c Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 09:27:39 +0000 Subject: [PATCH 18/49] Update: Simplify Logger constructor with static level colour map Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/Logger.js | 49 +++++++++++++++++-------------------------------- 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/lib/Logger.js b/lib/Logger.js index 079df98a..bb331919 100644 --- a/lib/Logger.js +++ b/lib/Logger.js @@ -12,7 +12,16 @@ class Logger { * @param {Array} options.levels Log level config strings. An empty array mutes all output. * @param {Boolean} options.showTimestamp Whether to show timestamps */ - constructor ({ levels = ['error', 'warn', 'success', 'info', 'debug', 'verbose'], showTimestamp = true } = {}) { + static levelColours = { + error: chalk.red, + warn: chalk.yellow, + success: chalk.green, + info: chalk.cyan, + debug: chalk.dim, + verbose: chalk.grey.italic + } + + constructor ({ levels = Object.keys(Logger.levelColours), showTimestamp = true } = {}) { /** * Hook invoked on each message logged * @type {Hook} @@ -20,38 +29,14 @@ class Logger { this.logHook = new Hook() /** @ignore */ this.config = { - levels: { - error: { - enable: Logger.isLevelEnabled(levels, 'error'), - moduleOverrides: Logger.getModuleOverrides(levels, 'error'), - colour: chalk.red - }, - warn: { - enable: Logger.isLevelEnabled(levels, 'warn'), - moduleOverrides: Logger.getModuleOverrides(levels, 'warn'), - colour: chalk.yellow - }, - success: { - enable: Logger.isLevelEnabled(levels, 'success'), - moduleOverrides: Logger.getModuleOverrides(levels, 'success'), - colour: chalk.green - }, - info: { - enable: Logger.isLevelEnabled(levels, 'info'), - moduleOverrides: Logger.getModuleOverrides(levels, 'info'), - colour: chalk.cyan - }, - debug: { - enable: Logger.isLevelEnabled(levels, 'debug'), - moduleOverrides: Logger.getModuleOverrides(levels, 'debug'), - colour: chalk.dim - }, - verbose: { - enable: Logger.isLevelEnabled(levels, 'verbose'), - moduleOverrides: Logger.getModuleOverrides(levels, 'verbose'), - colour: chalk.grey.italic + levels: Object.entries(Logger.levelColours).reduce((m, [level, colour]) => { + m[level] = { + enable: Logger.isLevelEnabled(levels, level), + moduleOverrides: Logger.getModuleOverrides(levels, level), + colour } - }, + return m + }, {}), timestamp: showTimestamp, mute: levels.length === 0 } From 26a88b11fc024c2a9fb61dd6663a2b0ec951b5a1 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 09:30:27 +0000 Subject: [PATCH 19/49] Update: Streamline Logger internals - Simplify getModuleOverrides with filter/startsWith - Inline colourise and getDateStamp into log method - Merge early returns in log method Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/Logger.js | 52 ++++++++++++--------------------------------------- 1 file changed, 12 insertions(+), 40 deletions(-) diff --git a/lib/Logger.js b/lib/Logger.js index bb331919..58e717ec 100644 --- a/lib/Logger.js +++ b/lib/Logger.js @@ -6,12 +6,6 @@ import Hook from './Hook.js' * @memberof core */ class Logger { - /** - * Creates a Logger instance from config values - * @param {Object} options - * @param {Array} options.levels Log level config strings. An empty array mutes all output. - * @param {Boolean} options.showTimestamp Whether to show timestamps - */ static levelColours = { error: chalk.red, warn: chalk.yellow, @@ -21,6 +15,12 @@ class Logger { verbose: chalk.grey.italic } + /** + * Creates a Logger instance from config values + * @param {Object} options + * @param {Array} options.levels Log level config strings. An empty array mutes all output. + * @param {Boolean} options.showTimestamp Whether to show timestamps + */ constructor ({ levels = Object.keys(Logger.levelColours), showTimestamp = true } = {}) { /** * Hook invoked on each message logged @@ -49,15 +49,13 @@ class Logger { * @param {...*} args Arguments to be logged */ log (level, id, ...args) { - if (this.config.mute) { - return - } - if (!Logger.isLoggingEnabled(this.config.levels, level, id)) { + if (this.config.mute || !Logger.isLoggingEnabled(this.config.levels, level, id)) { return } const colour = this.config.levels[level]?.colour const logFunc = console[level] ?? console.log - logFunc(`${Logger.getDateStamp(this.config)}${Logger.colourise(level, colour)} ${Logger.colourise(id, chalk.magenta)}`, ...args) + const timestamp = this.config.timestamp ? chalk.dim(`${new Date().toISOString()} `) : '' + logFunc(`${timestamp}${colour ? colour(level) : level} ${chalk.magenta(id)}`, ...args) this.logHook.invoke(new Date(), level, id, ...args) } @@ -78,12 +76,9 @@ class Logger { * @return {Array} */ static getModuleOverrides (levelsConfig, level) { - const overrides = [] - levelsConfig.forEach(l => { - const s = `${level}.`; const notS = `!${level}.` - if (l.indexOf(s) === 0 || l.indexOf(notS) === 0) overrides.push(l) - }) - return overrides + const prefix = `${level}.` + const negPrefix = `!${level}.` + return levelsConfig.filter(l => l.startsWith(prefix) || l.startsWith(negPrefix)) } /** @@ -99,29 +94,6 @@ class Logger { const disableOverride = moduleOverrides.includes(`!${level}.${id}`) return isEnabled && !disableOverride } - - /** - * Colours a string using a chalk function - * @param {String} str The string to colourise - * @param {Function} colourFunc A chalk colour function - * @return {String} - */ - static colourise (str, colourFunc) { - if (typeof colourFunc === 'string') colourFunc = chalk[colourFunc] - return colourFunc ? colourFunc(str) : str - } - - /** - * Returns a formatted ISO date stamp string - * @param {Object} config Logger configuration - * @return {String} - */ - static getDateStamp (config) { - if (!config.timestamp) { - return '' - } - return Logger.colourise(`${new Date().toISOString()} `, chalk.dim) - } } export default Logger From b05d15aa27ed48624807b8bdad61e28af23bf78c Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 09:38:43 +0000 Subject: [PATCH 20/49] Update: Remove redundant DependencyLoader.load() wrapper Merge load() into loadModules() by defaulting the modules parameter to all configured dependencies. App.start() now calls loadModules() directly. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 2 +- lib/DependencyLoader.js | 15 ++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/App.js b/lib/App.js index 086eaaea..4092ce97 100644 --- a/lib/App.js +++ b/lib/App.js @@ -121,7 +121,7 @@ class App extends AbstractModule { await this.dependencyloader.loadConfigs() await this.runMigrations() await this.loadLibraries() - await this.dependencyloader.load() + await this.dependencyloader.loadModules() this._isStarting = false } diff --git a/lib/DependencyLoader.js b/lib/DependencyLoader.js index 96ae4a0b..8c4d2a29 100644 --- a/lib/DependencyLoader.js +++ b/lib/DependencyLoader.js @@ -59,14 +59,7 @@ class DependencyLoader { this.moduleLoadedHook.tap(this.logProgress, this) } - /** - * Loads all Adapt module dependencies in a single pass. Load ordering is handled by waitForModule. Fatal errors from any module will halt the application. loadConfigs() must be called before this method. - * @return {Promise} - */ - async load () { - const modules = Object.values(this.configs).map(c => c.name) - await this.loadModules(modules) - } + /** * Loads configuration files for all Adapt dependencies found in node_modules. @@ -164,12 +157,12 @@ class DependencyLoader { } /** - * Loads a list of Adapt modules. Should not need to be called directly. - * @param {Array} modules Module names to load + * Loads Adapt modules. If no list is provided, loads all configured dependencies. + * @param {Array} [modules] Module names to load (defaults to all dependencies) * @return {Promise} Resolves when all modules have loaded or failed * @throws {Error} When any module throws a fatal error (error.isFatal or error.cause.isFatal) */ - async loadModules (modules) { + async loadModules (modules = Object.values(this.configs).map(c => c.name)) { await Promise.all(modules.map(async m => { try { await this.loadModule(m) From 1172817d456a28a5e198adfcbe17f6d174114761 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 09:47:39 +0000 Subject: [PATCH 21/49] Update: Remove redundant _isStarting flag and setReady override The _isReady check is sufficient to guard against double-start. The setReady override was only resetting _isStarting. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/lib/App.js b/lib/App.js index 4092ce97..0015ed24 100644 --- a/lib/App.js +++ b/lib/App.js @@ -55,8 +55,6 @@ class App extends AbstractModule { */ this.dependencyloader = new DependencyLoader(this) - /** @ignore */ this._isStarting = false - let startError try { await this.start() @@ -107,10 +105,7 @@ class App extends AbstractModule { * @return {Promise} Resolves when the app has started */ async start () { - if (this._isReady) throw new Error('warn', 'cannot start app, already started') - if (this._isStarting) throw new Error('warn', 'cannot start app, already initialising') - - this._isStarting = true + if (this._isReady) throw new Error('cannot start app, already started') /** * Reference to the Logger instance @@ -122,8 +117,6 @@ class App extends AbstractModule { await this.runMigrations() await this.loadLibraries() await this.dependencyloader.loadModules() - - this._isStarting = false } /** @@ -195,11 +188,6 @@ class App extends AbstractModule { return results.length > 1 ? results : results[0] } - /** @override */ - setReady (error) { - this._isStarting = false - super.setReady(error) - } } export default App From c461f62ce2ba10d05bb602bded424b0138853269 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 09:51:35 +0000 Subject: [PATCH 22/49] Update: Move config file path log into Config.load() Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 1 - lib/Config.js | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/App.js b/lib/App.js index 0015ed24..cc2c2a8f 100644 --- a/lib/App.js +++ b/lib/App.js @@ -161,7 +161,6 @@ class App extends AbstractModule { levels: this.getConfig('logLevels'), showTimestamp: this.getConfig('showLogTimestamp') }) - this.logger.log('info', 'config', `using config at ${this.config.configFilePath}`) /** * Reference to the error registry * @type {Errors} diff --git a/lib/Config.js b/lib/Config.js index 7c43255c..f840746d 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -24,6 +24,7 @@ class Config { await instance.storeUserSettings() instance.storeEnvSettings() instance.storeSchemaSettings(dependencies, appName) + logger.log('info', 'config', `using config at ${instance.configFilePath}`) return instance } From 0675effbd7d926b0cb8972a8250e60a28c1875b0 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 09:55:41 +0000 Subject: [PATCH 23/49] Update: Remove config override of rootDir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rootDir is determined by cwd/ROOT_DIR env var at construction time. Allowing config to override it after the fact is circular — config, dependencies, and metadata have already been loaded using the original rootDir by this point. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/App.js b/lib/App.js index cc2c2a8f..9e3b42d0 100644 --- a/lib/App.js +++ b/lib/App.js @@ -172,9 +172,6 @@ class App extends AbstractModule { */ this.lang = new Lang() await this.lang.loadPhrases(deps, this.rootDir, (...args) => this.logger.log('error', 'lang', ...args)) - - const configRootDir = this.getConfig('rootDir') - if (configRootDir) /** @ignore */this.rootDir = configRootDir } /** From cd84b9e50af2ec40dcb6ceba8267bfd6e62d644d Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 09:59:17 +0000 Subject: [PATCH 24/49] Update: Restructure App init/start lifecycle - Move app, args, dependencyloader setup to constructor - Inline loadLibraries into start() - Move verbose DIR/GIT logs to end of start() - Simplify init() to just error handling and failed module reporting Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 92 ++++++++++++++++++++++++------------------------------ 1 file changed, 40 insertions(+), 52 deletions(-) diff --git a/lib/App.js b/lib/App.js index 9e3b42d0..e61c8ec5 100644 --- a/lib/App.js +++ b/lib/App.js @@ -34,45 +34,36 @@ class App extends AbstractModule { const adaptJson = JSON.parse(fs.readFileSync(path.join(rootDir, metadataFileName))) const packageJson = JSON.parse(fs.readFileSync(path.join(rootDir, packageFileName))) super(null, { ...packageJson, ...adaptJson, name: 'adapt-authoring-core', rootDir }) - this.git = this.getGitInfo() - } - - /** @override */ - async init () { - /** - * Reference to the passed arguments (parsed for easy reference) - * @type {Object} - */ - this.args = getArgs() /** * Instance of App instance (required by all AbstractModules) * @type {App} */ this.app = this + /** + * Reference to the passed arguments (parsed for easy reference) + * @type {Object} + */ + this.args = getArgs() /** * Reference to the DependencyLoader instance * @type {DependencyLoader} */ this.dependencyloader = new DependencyLoader(this) + this.git = this.getGitInfo() + } - let startError + /** @override */ + async init () { try { await this.start() - this.log('verbose', 'GIT', 'INFO', this.git) - this.log('verbose', 'DIR', 'rootDir', this.rootDir) - this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) - this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) } catch (e) { - startError = e + process.exitCode = 1 + const error = new Error('Failed to start App') + error.cause = e + throw error } const failedMods = this.dependencyloader.failedModules if (failedMods.length) this.log('warn', `${failedMods.length} module${failedMods.length === 1 ? '' : 's'} failed to load: ${failedMods}. See above for details`) - if (startError) { - process.exitCode = 1 - const e = new Error('Failed to start App') - e.cause = startError - throw e - } } /** @@ -115,8 +106,34 @@ class App extends AbstractModule { await this.dependencyloader.loadConfigs() await this.runMigrations() - await this.loadLibraries() + + /** + * Reference to the Config instance + * @type {Config} + */ + this.config = await Config.load(this.rootDir, this.dependencies, this.name, this.logger) + this.logger = new Logger({ + levels: this.getConfig('logLevels'), + showTimestamp: this.getConfig('showLogTimestamp') + }) + /** + * Reference to the error registry + * @type {Errors} + */ + this.errors = await Errors.load(this.dependencies, msg => this.logger.log('warn', 'errors', msg)) + /** + * Reference to the Lang instance + * @type {Lang} + */ + this.lang = new Lang() + await this.lang.loadPhrases(this.dependencies, this.rootDir, (...args) => this.logger.log('error', 'lang', ...args)) + await this.dependencyloader.loadModules() + + this.log('verbose', 'GIT', 'INFO', this.git) + this.log('verbose', 'DIR', 'rootDir', this.rootDir) + this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) + this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) } /** @@ -146,34 +163,6 @@ class App extends AbstractModule { }) } - /** - * Loads the core bootstrap libraries (config, logger, errors, lang) before any modules initialise. - * @return {Promise} - */ - async loadLibraries () { - const deps = this.dependencies - /** - * Reference to the Config instance - * @type {Config} - */ - this.config = await Config.load(this.rootDir, deps, this.name, this.logger) - this.logger = new Logger({ - levels: this.getConfig('logLevels'), - showTimestamp: this.getConfig('showLogTimestamp') - }) - /** - * Reference to the error registry - * @type {Errors} - */ - this.errors = await Errors.load(deps, msg => this.logger.log('warn', 'errors', msg)) - /** - * Reference to the Lang instance - * @type {Lang} - */ - this.lang = new Lang() - await this.lang.loadPhrases(deps, this.rootDir, (...args) => this.logger.log('error', 'lang', ...args)) - } - /** * Enables waiting for other modules to load * @param {...String} modNames Names of modules to wait for @@ -183,7 +172,6 @@ class App extends AbstractModule { const results = await Promise.all(modNames.map(m => this.dependencyloader.waitForModule(m))) return results.length > 1 ? results : results[0] } - } export default App From bf636e7dd9f8fb88f315c3f76211bb46087f013b Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:01:44 +0000 Subject: [PATCH 25/49] Update: Merge start() into init() start() was only called from init() and added no value as a separate method. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 83 ++++++++++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 46 deletions(-) diff --git a/lib/App.js b/lib/App.js index e61c8ec5..0ba9f579 100644 --- a/lib/App.js +++ b/lib/App.js @@ -52,10 +52,46 @@ class App extends AbstractModule { this.git = this.getGitInfo() } + /** @override */ /** @override */ async init () { + /** + * Reference to the Logger instance + * @type {Logger} + */ + this.logger = new Logger() + try { - await this.start() + await this.dependencyloader.loadConfigs() + await this.runMigrations() + + /** + * Reference to the Config instance + * @type {Config} + */ + this.config = await Config.load(this.rootDir, this.dependencies, this.name, this.logger) + this.logger = new Logger({ + levels: this.getConfig('logLevels'), + showTimestamp: this.getConfig('showLogTimestamp') + }) + /** + * Reference to the error registry + * @type {Errors} + */ + this.errors = await Errors.load(this.dependencies, msg => this.logger.log('warn', 'errors', msg)) + /** + * Reference to the Lang instance + * @type {Lang} + */ + this.lang = new Lang() + await this.lang.loadPhrases(this.dependencies, this.rootDir, (...args) => this.logger.log('error', 'lang', ...args)) + + await this.dependencyloader.loadModules() + + this.log('verbose', 'GIT', 'INFO', this.git) + this.log('verbose', 'DIR', 'rootDir', this.rootDir) + this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) + this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) } catch (e) { process.exitCode = 1 const error = new Error('Failed to start App') @@ -91,51 +127,6 @@ class App extends AbstractModule { } } - /** - * Starts the app - * @return {Promise} Resolves when the app has started - */ - async start () { - if (this._isReady) throw new Error('cannot start app, already started') - - /** - * Reference to the Logger instance - * @type {Logger} - */ - this.logger = new Logger() - - await this.dependencyloader.loadConfigs() - await this.runMigrations() - - /** - * Reference to the Config instance - * @type {Config} - */ - this.config = await Config.load(this.rootDir, this.dependencies, this.name, this.logger) - this.logger = new Logger({ - levels: this.getConfig('logLevels'), - showTimestamp: this.getConfig('showLogTimestamp') - }) - /** - * Reference to the error registry - * @type {Errors} - */ - this.errors = await Errors.load(this.dependencies, msg => this.logger.log('warn', 'errors', msg)) - /** - * Reference to the Lang instance - * @type {Lang} - */ - this.lang = new Lang() - await this.lang.loadPhrases(this.dependencies, this.rootDir, (...args) => this.logger.log('error', 'lang', ...args)) - - await this.dependencyloader.loadModules() - - this.log('verbose', 'GIT', 'INFO', this.git) - this.log('verbose', 'DIR', 'rootDir', this.rootDir) - this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) - this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) - } - /** * Runs pending migrations before config validation and module loading. * Reads the user config file directly to obtain the MongoDB connection URI. From 8af4108a16b83e16dd7d87d7da8b2a3f9cdd4ab7 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:06:08 +0000 Subject: [PATCH 26/49] Update: Use Error cause option parameter Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/App.js b/lib/App.js index 0ba9f579..a03c2b69 100644 --- a/lib/App.js +++ b/lib/App.js @@ -92,11 +92,9 @@ class App extends AbstractModule { this.log('verbose', 'DIR', 'rootDir', this.rootDir) this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) - } catch (e) { + } catch (cause) { process.exitCode = 1 - const error = new Error('Failed to start App') - error.cause = e - throw error + throw new Error('Failed to start App', { cause }) } const failedMods = this.dependencyloader.failedModules if (failedMods.length) this.log('warn', `${failedMods.length} module${failedMods.length === 1 ? '' : 's'} failed to load: ${failedMods}. See above for details`) From b35bbffe1bc5485a2ff1af57cacbff9f8bb561d6 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:15:07 +0000 Subject: [PATCH 27/49] Update: Calculate config file path in App, pass to Config and migrations Config file path is now determined once in App.init() and shared with both the migrations runner and Config.load(). Config.load() accepts a named options object instead of positional params. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 32 +++++++++++++++++++------------- lib/Config.js | 21 ++++++--------------- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/lib/App.js b/lib/App.js index a03c2b69..2c305e4a 100644 --- a/lib/App.js +++ b/lib/App.js @@ -52,7 +52,6 @@ class App extends AbstractModule { this.git = this.getGitInfo() } - /** @override */ /** @override */ async init () { /** @@ -63,13 +62,23 @@ class App extends AbstractModule { try { await this.dependencyloader.loadConfigs() - await this.runMigrations() + + const configFilePath = path.join(this.rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) + + await this.runMigrations(configFilePath) /** * Reference to the Config instance * @type {Config} */ - this.config = await Config.load(this.rootDir, this.dependencies, this.name, this.logger) + this.config = await Config.load({ + rootDir: this.rootDir, + configFilePath, + dependencies: this.dependencies, + appName: this.name, + logger: this.logger + }) + this.logger = new Logger({ levels: this.getConfig('logLevels'), showTimestamp: this.getConfig('showLogTimestamp') @@ -127,27 +136,24 @@ class App extends AbstractModule { /** * Runs pending migrations before config validation and module loading. - * Reads the user config file directly to obtain the MongoDB connection URI. + * @param {string} configFilePath Path to the user config file * @return {Promise} */ - async runMigrations () { - const configFilePath = path.join(this.rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) - let userConfig + async runMigrations (configFilePath) { + let connectionUri try { - userConfig = (await import(configFilePath)).default - } catch (e) { - return // no config file, nothing to migrate + connectionUri = (await import(configFilePath)).default?.['adapt-authoring-mongodb']?.connectionUri + } catch { + return } - const connectionUri = userConfig['adapt-authoring-mongodb']?.connectionUri if (!connectionUri) return - const log = (level, ...args) => this.logger.log(level, ...args) const { runMigrations } = await import('adapt-authoring-migrations') await runMigrations({ dependencies: this.dependencies, connectionUri, rootDir: this.rootDir, - log, + log: (level, ...args) => this.logger.log(level, ...args), dryRun: this.args['dry-run'] === true }) } diff --git a/lib/Config.js b/lib/Config.js index f840746d..6925b655 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -16,15 +16,15 @@ class Config { * @param {Logger} logger Logger instance for output * @returns {Promise} */ - static async load (rootDir, dependencies, appName, logger) { + static async load ({ rootDir, configFilePath, dependencies, appName, logger }) { const instance = new Config() instance.rootDir = rootDir instance.logger = logger - instance.configFilePath = path.join(rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) + instance.configFilePath = configFilePath await instance.storeUserSettings() instance.storeEnvSettings() instance.storeSchemaSettings(dependencies, appName) - logger.log('info', 'config', `using config at ${instance.configFilePath}`) + logger.log('info', 'config', `using config at ${configFilePath}`) return instance } @@ -79,23 +79,15 @@ class Config { /** * Loads the relevant config file into memory - * @return {Promise} The parsed config object, or undefined if loading failed + * @return {Promise} */ async storeUserSettings () { - let configError let config try { await fs.readFile(this.configFilePath) + config = (await import(this.configFilePath)).default } catch (e) { - if (e.code === 'ENOENT') configError = `No config file found at '${this.configFilePath}'` - } - try { - if (!configError) config = (await import(this.configFilePath)).default - } catch (e) { - configError = e.toString() - } - if (configError) { - this.logger.log('warn', 'config', `Failed to load config at ${this.configFilePath}: ${configError}. Will attempt to run with defaults.`) + this.logger.log('warn', 'config', `Failed to load config at ${this.configFilePath}: ${e}. Will attempt to run with defaults.`) return } Object.entries(config).forEach(([name, c]) => { @@ -103,7 +95,6 @@ class Config { this._config[`${name}.${key}`] = val }) }) - return config } /** From 62ea7f659219a876dcf9846ef5c80ef6b513aeaf Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:16:40 +0000 Subject: [PATCH 28/49] Update: Pass configFilePath to migrations, let it read the file Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/App.js b/lib/App.js index 2c305e4a..c5e1afa6 100644 --- a/lib/App.js +++ b/lib/App.js @@ -140,18 +140,10 @@ class App extends AbstractModule { * @return {Promise} */ async runMigrations (configFilePath) { - let connectionUri - try { - connectionUri = (await import(configFilePath)).default?.['adapt-authoring-mongodb']?.connectionUri - } catch { - return - } - if (!connectionUri) return - const { runMigrations } = await import('adapt-authoring-migrations') await runMigrations({ dependencies: this.dependencies, - connectionUri, + configFilePath, rootDir: this.rootDir, log: (level, ...args) => this.logger.log(level, ...args), dryRun: this.args['dry-run'] === true From 390e0dc656490a1684b9c312217cf25e16d3e1b1 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:19:00 +0000 Subject: [PATCH 29/49] Update: Inline runMigrations into init(), use static import Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/lib/App.js b/lib/App.js index c5e1afa6..5a633d01 100644 --- a/lib/App.js +++ b/lib/App.js @@ -6,6 +6,7 @@ import Lang from './Lang.js' import Logger from './Logger.js' import fs from 'fs' import path from 'path' +import { runMigrations } from 'adapt-authoring-migrations' import { metadataFileName, packageFileName, getArgs } from './Utils.js' let instance @@ -65,7 +66,13 @@ class App extends AbstractModule { const configFilePath = path.join(this.rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) - await this.runMigrations(configFilePath) + await runMigrations({ + dependencies: this.dependencies, + configFilePath, + rootDir: this.rootDir, + log: (level, ...args) => this.logger.log(level, ...args), + dryRun: this.args['dry-run'] === true + }) /** * Reference to the Config instance @@ -134,22 +141,6 @@ class App extends AbstractModule { } } - /** - * Runs pending migrations before config validation and module loading. - * @param {string} configFilePath Path to the user config file - * @return {Promise} - */ - async runMigrations (configFilePath) { - const { runMigrations } = await import('adapt-authoring-migrations') - await runMigrations({ - dependencies: this.dependencies, - configFilePath, - rootDir: this.rootDir, - log: (level, ...args) => this.logger.log(level, ...args), - dryRun: this.args['dry-run'] === true - }) - } - /** * Enables waiting for other modules to load * @param {...String} modNames Names of modules to wait for From bcb16d3070690e5348402fa05ea4cb25a7ed7a42 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:27:52 +0000 Subject: [PATCH 30/49] Update: Streamline Lang internals - Use instanceof Error instead of constructor name string check - Use fs.readFile with utf8 encoding directly - Remove getPhrasesForLang (callers can use phrases[lang] directly) Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/Lang.js | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/lib/Lang.js b/lib/Lang.js index fef0a139..95150372 100644 --- a/lib/Lang.js +++ b/lib/Lang.js @@ -39,7 +39,7 @@ class Lang { const files = await glob('lang/*.json', { cwd: dir, absolute: true }) await Promise.all(files.map(async f => { try { - const contents = JSON.parse((await fs.readFile(f)).toString()) + const contents = JSON.parse(await fs.readFile(f, 'utf8')) Object.entries(contents).forEach(([k, v]) => Lang.storeStrings(this.phrases, `${path.basename(f).replace('.json', '')}.${k}`, v)) } catch (e) { if (logError) logError(e.message, f) @@ -48,16 +48,6 @@ class Lang { })) } - /** - * Load all lang phrases for a language - * @param {String} lang The language of strings to load - * @return {Object|undefined} The phrases, or undefined if the language is not found - */ - getPhrasesForLang (lang) { - const phrases = this.phrases[lang] - return phrases && Object.keys(phrases).length ? phrases : undefined - } - /** * Returns translated language string * @param {String} defaultLang Default language to use when lang is not a string @@ -110,7 +100,7 @@ class Lang { if (typeof lang !== 'string') { lang = defaultLang } - if (key.constructor.name.endsWith('Error')) { + if (key instanceof Error) { return Lang.translateError(phrases, defaultLang, logWarn, lang, key) } const s = phrases[lang]?.[key] @@ -145,7 +135,7 @@ class Lang { * @returns The translated error */ static translateError (phrases, defaultLang, logWarn, lang, error) { - return error?.constructor.name.endsWith('Error') + return error instanceof Error ? Lang.translate(phrases, defaultLang, logWarn, lang, `error.${error.code}`, error.data ?? error) : error } From fd5c11876a4969c2cb192d7496b7e9c04d8ff18d Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:29:57 +0000 Subject: [PATCH 31/49] Update: Remove logError, use log('error', ...) directly Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/DependencyLoader.js | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/DependencyLoader.js b/lib/DependencyLoader.js index 8c4d2a29..ddd8a5c6 100644 --- a/lib/DependencyLoader.js +++ b/lib/DependencyLoader.js @@ -91,8 +91,8 @@ class DependencyLoader { } } } catch (e) { - this.logError(`Failed to load config for '${d}', module will not be loaded`) - this.logError(e) + this.log('error',`Failed to load config for '${d}', module will not be loaded`) + this.log('error',e) } } this._configsLoaded = true @@ -170,11 +170,11 @@ class DependencyLoader { if (e.isFatal || e.cause?.isFatal) { throw e } - this.logError(`Failed to load '${m}',`, e) + this.log('error',`Failed to load '${m}',`, e) const deps = this.peerDependencies[m] if (deps && deps.length) { - this.logError('The following modules are peer dependencies, and may not work:') - deps.forEach(d => this.logError(`- ${d}`)) + this.log('error','The following modules are peer dependencies, and may not work:') + deps.forEach(d => this.log('error',`- ${d}`)) } this.failedModules.push(m) } @@ -253,13 +253,6 @@ class DependencyLoader { this.app.logger?.log(level, this.name, ...args) } - /** - * Logs an error message using the app logger if available, otherwise falls back to console.log - * @param {...*} args Arguments to be logged - */ - logError (...args) { - this.log('error', ...args) - } /** * Retrieves a configuration value from this module's config From ed6654e89eff2c162f7b3eee463148c6f0455b9a Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:33:05 +0000 Subject: [PATCH 32/49] Update: Streamline DependencyLoader - Replace lodash _.isFunction with typeof checks - Replace fs-extra with core readJson util - Combine two sorts in loadConfigs into one - Use Object.fromEntries for init times - Lazy-create DependencyError in waitForModule - Remove eslint directive and cosmetic cleanup Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/DependencyLoader.js | 65 ++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 37 deletions(-) diff --git a/lib/DependencyLoader.js b/lib/DependencyLoader.js index ddd8a5c6..fab8a680 100644 --- a/lib/DependencyLoader.js +++ b/lib/DependencyLoader.js @@ -1,10 +1,8 @@ -/* eslint no-console: 0 */ -import _ from 'lodash' -import fs from 'fs-extra' import { glob } from 'glob' import path from 'path' import Hook from './Hook.js' -import { metadataFileName, packageFileName, stripScope } from './Utils.js' +import { metadataFileName, packageFileName, stripScope, readJson } from './Utils.js' + /** * Handles the loading of Adapt authoring tool module dependencies. * @memberof core @@ -59,26 +57,21 @@ class DependencyLoader { this.moduleLoadedHook.tap(this.logProgress, this) } - - /** * Loads configuration files for all Adapt dependencies found in node_modules. * @return {Promise} */ async loadConfigs () { /** @ignore */ this._configsLoaded = false + const corePathSegment = `/${this.app.name}/` const files = await glob(`${this.app.rootDir}/node_modules/**/${metadataFileName}`) const deps = files .map(d => d.replace(`${metadataFileName}`, '')) - .sort((a, b) => a.length < b.length ? -1 : 1) - - // sort so that core is loaded first, as other modules may use its config values - const corePathSegment = `/${this.app.name}/` - deps.sort((a, b) => { - if (a.endsWith(corePathSegment)) return -1 - if (b.endsWith(corePathSegment)) return 1 - return 0 - }) + .sort((a, b) => { + if (a.endsWith(corePathSegment)) return -1 + if (b.endsWith(corePathSegment)) return 1 + return a.length - b.length + }) for (const d of deps) { try { const c = await this.loadModuleConfig(d) @@ -91,8 +84,8 @@ class DependencyLoader { } } } catch (e) { - this.log('error',`Failed to load config for '${d}', module will not be loaded`) - this.log('error',e) + this.log('error', `Failed to load config for '${d}', module will not be loaded`) + this.log('error', e) } } this._configsLoaded = true @@ -105,10 +98,10 @@ class DependencyLoader { * @return {Promise} Resolves with configuration object */ async loadModuleConfig (modDir) { - const pkg = await fs.readJson(path.join(modDir, packageFileName)) + const pkg = await readJson(path.join(modDir, packageFileName)) return { ...pkg, - ...await fs.readJson(path.join(modDir, metadataFileName)), + ...await readJson(path.join(modDir, metadataFileName)), name: stripScope(pkg.name), packageName: pkg.name, rootDir: modDir @@ -132,12 +125,12 @@ class DependencyLoader { } const { default: ModClass } = await import(config.packageName) - if (!_.isFunction(ModClass)) { + if (typeof ModClass !== 'function') { throw new Error('Expected class to be exported') } const instance = new ModClass(this.app, config) - if (!_.isFunction(instance.onReady)) { + if (typeof instance.onReady !== 'function') { throw new Error('Module must define onReady function') } try { @@ -170,11 +163,11 @@ class DependencyLoader { if (e.isFatal || e.cause?.isFatal) { throw e } - this.log('error',`Failed to load '${m}',`, e) + this.log('error', `Failed to load '${m}',`, e) const deps = this.peerDependencies[m] - if (deps && deps.length) { - this.log('error','The following modules are peer dependencies, and may not work:') - deps.forEach(d => this.log('error',`- ${d}`)) + if (deps?.length) { + this.log('error', 'The following modules are peer dependencies, and may not work:') + deps.forEach(d => this.log('error', `- ${d}`)) } this.failedModules.push(m) } @@ -191,14 +184,12 @@ class DependencyLoader { if (!this._configsLoaded) { await this.configsLoadedHook.onInvoke() } - const longPrefix = 'adapt-authoring-' - if (!modName.startsWith(longPrefix)) modName = `adapt-authoring-${modName}` + if (!modName.startsWith('adapt-authoring-')) modName = `adapt-authoring-${modName}` if (!this.configs[modName]) { throw new Error(`Missing required module '${modName}'`) } - const DependencyError = new Error(`Dependency '${modName}' failed to load`) if (this.failedModules.includes(modName)) { - throw DependencyError + throw new Error(`Dependency '${modName}' failed to load`) } const instance = this.instances[modName] if (instance) { @@ -206,7 +197,7 @@ class DependencyLoader { } return new Promise((resolve, reject) => { this.moduleLoadedHook.tap((error, instance) => { - if (error) return reject(DependencyError) + if (error) return reject(new Error(`Dependency '${modName}' failed to load`)) if (instance?.name === modName) resolve(instance) }) }) @@ -217,9 +208,8 @@ class DependencyLoader { * @param {AbstractModule} instance The last loaded instance */ logProgress (error, instance) { - if (error) { - return - } + if (error) return + const toShort = names => names.map(n => n.replace('adapt-authoring-', '')).join(', ') const loaded = [] const notLoaded = [] @@ -238,9 +228,11 @@ class DependencyLoader { ].filter(Boolean).join(', ')) if (progress === 100) { - const initTimes = Object.entries(this.instances) - .sort((a, b) => a[1].initTime < b[1].initTime ? -1 : a[1].initTime > b[1].initTime ? 1 : 0) - .reduce((memo, [modName, instance]) => Object.assign(memo, { [modName]: instance.initTime }), {}) + const initTimes = Object.fromEntries( + Object.entries(this.instances) + .sort(([, a], [, b]) => a.initTime - b.initTime) + .map(([name, inst]) => [name, inst.initTime]) + ) this.log('verbose', initTimes) } } @@ -253,7 +245,6 @@ class DependencyLoader { this.app.logger?.log(level, this.name, ...args) } - /** * Retrieves a configuration value from this module's config * @param {string} key - The configuration key to retrieve From 8f3faa6aa86ef4d4b4989e98ea1305ce2d012c2e Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:34:25 +0000 Subject: [PATCH 33/49] Update: Remove DependencyLoader.getConfig, use app.getConfig Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/DependencyLoader.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/DependencyLoader.js b/lib/DependencyLoader.js index fab8a680..94e60246 100644 --- a/lib/DependencyLoader.js +++ b/lib/DependencyLoader.js @@ -135,7 +135,7 @@ class DependencyLoader { } try { // all essential modules will use hard-coded value, as config won't be loaded yet - const timeout = this.getConfig('moduleLoadTimeout') ?? 10000 + const timeout = this.app.getConfig('moduleLoadTimeout') ?? 10000 await Promise.race([ instance.onReady(), new Promise((resolve, reject) => setTimeout(() => reject(new Error(`${modName} load exceeded timeout (${timeout})`)), timeout)) @@ -245,14 +245,6 @@ class DependencyLoader { this.app.logger?.log(level, this.name, ...args) } - /** - * Retrieves a configuration value from this module's config - * @param {string} key - The configuration key to retrieve - * @returns {*|undefined} The configuration value, or undefined if config is not yet loaded - */ - getConfig (key) { - return this.app.config?.get(`adapt-authoring-core.${key}`) - } } export default DependencyLoader From a1157b11631eecb468d06a63f8e203d8f002d168 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:38:25 +0000 Subject: [PATCH 34/49] New: Add error definitions for dependency loading Replace plain Error throws in DependencyLoader with AdaptError definitions: DEP_ALREADY_LOADED, DEP_FAILED, DEP_INVALID_EXPORT, DEP_MISSING, DEP_NO_ONREADY, DEP_TIMEOUT. Co-Authored-By: Claude Opus 4.6 (1M context) --- errors/errors.json | 32 ++++++++++++++++++++++++++++++++ lib/DependencyLoader.js | 15 +++++++-------- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/errors/errors.json b/errors/errors.json index e585fdd4..2a3de6d8 100644 --- a/errors/errors.json +++ b/errors/errors.json @@ -7,6 +7,38 @@ "description": "File contains a syntax error", "statusCode": 500 }, + "DEP_ALREADY_LOADED": { + "data": { "module": "The module name" }, + "description": "Module has already been loaded", + "statusCode": 500 + }, + "DEP_FAILED": { + "data": { "module": "The module name" }, + "description": "Required dependency failed to load", + "statusCode": 500, + "isFatal": true + }, + "DEP_INVALID_EXPORT": { + "data": { "module": "The module name" }, + "description": "Module must export a class as its default export", + "statusCode": 500 + }, + "DEP_MISSING": { + "data": { "module": "The module name" }, + "description": "Required module is not installed", + "statusCode": 500, + "isFatal": true + }, + "DEP_NO_ONREADY": { + "data": { "module": "The module name" }, + "description": "Module must define an onReady function", + "statusCode": 500 + }, + "DEP_TIMEOUT": { + "data": { "module": "The module name", "timeout": "The timeout in ms" }, + "description": "Module load exceeded timeout", + "statusCode": 500 + }, "FUNC_DISABLED": { "data": { "name": "The name of the function" diff --git a/lib/DependencyLoader.js b/lib/DependencyLoader.js index 94e60246..c0a7355c 100644 --- a/lib/DependencyLoader.js +++ b/lib/DependencyLoader.js @@ -116,7 +116,7 @@ class DependencyLoader { */ async loadModule (modName) { if (this.instances[modName]) { - throw new Error('Module already exists') + throw this.app.errors.DEP_ALREADY_LOADED.setData({ module: modName }) } const config = this.configs[modName] @@ -126,19 +126,18 @@ class DependencyLoader { const { default: ModClass } = await import(config.packageName) if (typeof ModClass !== 'function') { - throw new Error('Expected class to be exported') + throw this.app.errors.DEP_INVALID_EXPORT.setData({ module: modName }) } const instance = new ModClass(this.app, config) if (typeof instance.onReady !== 'function') { - throw new Error('Module must define onReady function') + throw this.app.errors.DEP_NO_ONREADY.setData({ module: modName }) } try { - // all essential modules will use hard-coded value, as config won't be loaded yet const timeout = this.app.getConfig('moduleLoadTimeout') ?? 10000 await Promise.race([ instance.onReady(), - new Promise((resolve, reject) => setTimeout(() => reject(new Error(`${modName} load exceeded timeout (${timeout})`)), timeout)) + new Promise((resolve, reject) => setTimeout(() => reject(this.app.errors.DEP_TIMEOUT.setData({ module: modName, timeout })), timeout)) ]) this.instances[modName] = instance await this.moduleLoadedHook.invoke(null, instance) @@ -186,10 +185,10 @@ class DependencyLoader { } if (!modName.startsWith('adapt-authoring-')) modName = `adapt-authoring-${modName}` if (!this.configs[modName]) { - throw new Error(`Missing required module '${modName}'`) + throw this.app.errors.DEP_MISSING.setData({ module: modName }) } if (this.failedModules.includes(modName)) { - throw new Error(`Dependency '${modName}' failed to load`) + throw this.app.errors.DEP_FAILED.setData({ module: modName }) } const instance = this.instances[modName] if (instance) { @@ -197,7 +196,7 @@ class DependencyLoader { } return new Promise((resolve, reject) => { this.moduleLoadedHook.tap((error, instance) => { - if (error) return reject(new Error(`Dependency '${modName}' failed to load`)) + if (error) return reject(this.app.errors.DEP_FAILED.setData({ module: modName })) if (instance?.name === modName) resolve(instance) }) }) From 79150540a22758709179090052157697c1df044c Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 10:42:04 +0000 Subject: [PATCH 35/49] Update: Remove fs-extra, lodash, and minimist dependencies - Replace lodash _.isFunction with typeof check - Replace lodash _.cloneDeep with native structuredClone - Replace minimist with native util.parseArgs - fs-extra was already unused after readJson util was added Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/Hook.js | 5 ++--- lib/utils/getArgs.js | 10 ++++------ package.json | 5 +---- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/Hook.js b/lib/Hook.js index 2dd1c114..65f3d064 100644 --- a/lib/Hook.js +++ b/lib/Hook.js @@ -1,4 +1,3 @@ -import _ from 'lodash' /** * Allows observers to tap into to a specific piece of code, and execute their own arbitrary code * @memberof core @@ -43,7 +42,7 @@ class Hook { * @param {*} scope Sets the scope of the observer */ tap (observer, scope) { - if (_.isFunction(observer)) this._hookObservers.push(observer.bind(scope)) + if (typeof observer === 'function') this._hookObservers.push(observer.bind(scope)) } /** @@ -78,7 +77,7 @@ class Hook { data = await Promise.all(this._hookObservers.map(o => o(...args))) } else { // if not mutable, send a deep copy of the args to avoid any meddling - for (const o of this._hookObservers) data = await o(...this._options.mutable ? args : args.map(a => _.cloneDeep(a))) + for (const o of this._hookObservers) data = await o(...this._options.mutable ? args : args.map(a => structuredClone(a))) } } catch (e) { error = e diff --git a/lib/utils/getArgs.js b/lib/utils/getArgs.js index feeb1d60..605dc9a3 100644 --- a/lib/utils/getArgs.js +++ b/lib/utils/getArgs.js @@ -1,12 +1,10 @@ -import minimist from 'minimist' +import { parseArgs } from 'node:util' /** - * Returns the passed arguments, parsed by minimist for easy access + * Returns the passed arguments, parsed for easy access * @return {Object} The parsed arguments - * @see {@link https://github.com/substack/minimist#readme} */ export function getArgs () { - const args = minimist(process.argv) - args.params = args._.slice(2) - return args + const { values, positionals } = parseArgs({ strict: false, args: process.argv.slice(2) }) + return { ...values, params: positionals } } diff --git a/package.json b/package.json index bc98fae9..6f13f25a 100644 --- a/package.json +++ b/package.json @@ -13,10 +13,7 @@ "dependencies": { "adapt-schemas": "^3.1.0", "chalk": "^5.4.1", - "fs-extra": "11.3.3", - "glob": "^13.0.0", - "lodash": "^4.17.21", - "minimist": "^1.2.8" + "glob": "^13.0.0" }, "devDependencies": { "@adaptlearning/semantic-release-config": "^1.0.0", From adb6d47fac37a4e2cd3a852d169ee30d4be5420b Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 11:28:01 +0000 Subject: [PATCH 36/49] Fix: Update tests and deps for refactored code - Add adapt-authoring-migrations as dependency, static import - Remove tests for removed methods (logError, getConfig, getPhrasesForLang, colourise, getDateStamp, setReady/_isStarting) - Update DependencyLoader tests to use AdaptError codes - Update Config storeUserSettings test for new signature - Update getArgs tests for util.parseArgs shape - Fix lint: remove padded block in DependencyLoader Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/DependencyLoader.js | 1 - package.json | 1 + tests/App.spec.js | 8 --- tests/Config.spec.js | 14 +---- tests/DependencyLoader.spec.js | 100 ++++++++------------------------- tests/Lang.spec.js | 16 ------ tests/Logger.spec.js | 22 -------- tests/utils-getArgs.spec.js | 10 +--- 8 files changed, 27 insertions(+), 145 deletions(-) diff --git a/lib/DependencyLoader.js b/lib/DependencyLoader.js index c0a7355c..19b1718f 100644 --- a/lib/DependencyLoader.js +++ b/lib/DependencyLoader.js @@ -243,7 +243,6 @@ class DependencyLoader { log (level, ...args) { this.app.logger?.log(level, this.name, ...args) } - } export default DependencyLoader diff --git a/package.json b/package.json index 6f13f25a..4331bdf3 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ }, "repository": "github:adapt-security/adapt-authoring-core", "dependencies": { + "adapt-authoring-migrations": "github:adapt-security/adapt-authoring-migrations#feature/boot-phase-migrations", "adapt-schemas": "^3.1.0", "chalk": "^5.4.1", "glob": "^13.0.0" diff --git a/tests/App.spec.js b/tests/App.spec.js index 18c781f7..ff75c366 100644 --- a/tests/App.spec.js +++ b/tests/App.spec.js @@ -149,12 +149,4 @@ describe('App', () => { }) }) - describe('#setReady()', () => { - it('should set _isStarting to false', async () => { - const app = App.instance - app._isStarting = true - await app.setReady() - assert.equal(app._isStarting, false) - }) - }) }) diff --git a/tests/Config.spec.js b/tests/Config.spec.js index 0925e806..0caf850c 100644 --- a/tests/Config.spec.js +++ b/tests/Config.spec.js @@ -117,20 +117,10 @@ describe('Config', () => { }) describe('#storeUserSettings()', () => { - let confDir - - before(async () => { - confDir = path.join(__dirname, 'data', 'config-test', 'conf') - await fs.ensureDir(confDir) - }) - - after(async () => { - await fs.remove(path.join(__dirname, 'data', 'config-test')) - }) - it('should handle missing config file gracefully', async () => { const config = new Config() - config.configFilePath = path.join(confDir, 'nonexistent.config.js') + config.configFilePath = '/nonexistent/path/config.js' + config.logger = { log: () => {} } await assert.doesNotReject(() => config.storeUserSettings()) }) }) diff --git a/tests/DependencyLoader.spec.js b/tests/DependencyLoader.spec.js index 4c1f1b68..c41b5260 100644 --- a/tests/DependencyLoader.spec.js +++ b/tests/DependencyLoader.spec.js @@ -1,5 +1,6 @@ import { describe, it, before, after } from 'node:test' import assert from 'node:assert/strict' +import AdaptError from '../lib/AdaptError.js' import DependencyLoader from '../lib/DependencyLoader.js' import fs from 'fs-extra' import path from 'path' @@ -98,78 +99,6 @@ describe('DependencyLoader', () => { }) }) - describe('#logError()', () => { - it('should call log with error level', () => { - let loggedLevel - const mockApp = { - rootDir: '/test', - logger: { - log: (level) => { loggedLevel = level } - } - } - const loader = new DependencyLoader(mockApp) - - loader.logError('error message') - - assert.equal(loggedLevel, 'error') - }) - - it('should pass all arguments through to log', () => { - let loggedArgs - const mockApp = { - rootDir: '/test', - logger: { - log: (level, name, ...args) => { loggedArgs = args } - } - } - const loader = new DependencyLoader(mockApp) - loader.logError('msg1', 'msg2') - assert.deepEqual(loggedArgs, ['msg1', 'msg2']) - }) - }) - - describe('#getConfig()', () => { - it('should return undefined when config is not available', () => { - const mockApp = { - rootDir: '/test' - } - const loader = new DependencyLoader(mockApp) - - const result = loader.getConfig('someKey') - - assert.equal(result, undefined) - }) - - it('should return config value when config is available', () => { - const mockApp = { - rootDir: '/test', - config: { - get: (key) => { - if (key === 'adapt-authoring-core.testKey') return 'testValue' - } - } - } - const loader = new DependencyLoader(mockApp) - - const result = loader.getConfig('testKey') - - assert.equal(result, 'testValue') - }) - - it('should always use adapt-authoring-core prefix for config keys', () => { - let requestedKey - const mockApp = { - rootDir: '/test', - config: { - get: (key) => { requestedKey = key } - } - } - const loader = new DependencyLoader(mockApp) - loader.getConfig('myKey') - assert.equal(requestedKey, 'adapt-authoring-core.myKey') - }) - }) - describe('#loadConfigs()', () => { let testRootDir @@ -383,13 +312,18 @@ describe('DependencyLoader', () => { describe('#loadModule()', () => { it('should throw when module already exists', async () => { - const mockApp = { rootDir: '/test' } + const mockApp = { + rootDir: '/test', + errors: { + DEP_ALREADY_LOADED: new AdaptError('DEP_ALREADY_LOADED') + } + } const loader = new DependencyLoader(mockApp) loader.instances = { 'existing-module': {} } await assert.rejects( loader.loadModule('existing-module'), - { message: 'Module already exists' } + { code: 'DEP_ALREADY_LOADED' } ) }) @@ -416,19 +350,29 @@ describe('DependencyLoader', () => { describe('#waitForModule()', () => { it('should throw for missing module', async () => { - const mockApp = { rootDir: '/test' } + const mockApp = { + rootDir: '/test', + errors: { + DEP_MISSING: new AdaptError('DEP_MISSING') + } + } const loader = new DependencyLoader(mockApp) loader._configsLoaded = true loader.configs = {} await assert.rejects( loader.waitForModule('adapt-authoring-missing'), - { message: "Missing required module 'adapt-authoring-missing'" } + { code: 'DEP_MISSING' } ) }) it('should throw for failed module', async () => { - const mockApp = { rootDir: '/test' } + const mockApp = { + rootDir: '/test', + errors: { + DEP_FAILED: new AdaptError('DEP_FAILED') + } + } const loader = new DependencyLoader(mockApp) loader._configsLoaded = true loader.configs = { 'adapt-authoring-failed': { name: 'adapt-authoring-failed' } } @@ -436,7 +380,7 @@ describe('DependencyLoader', () => { await assert.rejects( loader.waitForModule('adapt-authoring-failed'), - { message: "Dependency 'adapt-authoring-failed' failed to load" } + { code: 'DEP_FAILED' } ) }) diff --git a/tests/Lang.spec.js b/tests/Lang.spec.js index 54437ae2..4105d4d3 100644 --- a/tests/Lang.spec.js +++ b/tests/Lang.spec.js @@ -54,22 +54,6 @@ describe('Lang', () => { }) }) - describe('#getPhrasesForLang()', () => { - it('should return phrases for a specific language', async () => { - const lang = new Lang() - await lang.loadPhrases({ test: { rootDir: testDir } }, testDir) - const phrases = lang.getPhrasesForLang('en') - assert.ok(phrases) - assert.equal(phrases['app.name'], 'Test App') - }) - - it('should return undefined for unknown language', async () => { - const lang = new Lang() - await lang.loadPhrases({ test: { rootDir: testDir } }, testDir) - assert.equal(lang.getPhrasesForLang('de'), undefined) - }) - }) - describe('.storeStrings()', () => { it('should store a string under lang.key', () => { const phrases = {} diff --git a/tests/Logger.spec.js b/tests/Logger.spec.js index 64538eb2..02017e75 100644 --- a/tests/Logger.spec.js +++ b/tests/Logger.spec.js @@ -75,28 +75,6 @@ describe('Logger', () => { }) }) - describe('.colourise()', () => { - it('should return string unchanged when no colour function', () => { - assert.equal(Logger.colourise('test', undefined), 'test') - }) - - it('should apply colour function', () => { - const result = Logger.colourise('test', s => `[${s}]`) - assert.equal(result, '[test]') - }) - }) - - describe('.getDateStamp()', () => { - it('should return empty string when timestamp disabled', () => { - assert.equal(Logger.getDateStamp({ timestamp: false }), '') - }) - - it('should return ISO format when configured', () => { - const result = Logger.getDateStamp({ timestamp: true, dateFormat: 'iso' }) - assert.ok(result.length > 0) - }) - }) - describe('#log()', () => { it('should not throw when muted via empty levels', () => { const logger = new Logger({ levels: [] }) diff --git a/tests/utils-getArgs.spec.js b/tests/utils-getArgs.spec.js index b72d6fc3..6a3ad997 100644 --- a/tests/utils-getArgs.spec.js +++ b/tests/utils-getArgs.spec.js @@ -7,16 +7,10 @@ describe('getArgs()', () => { it('should return an object with parsed arguments', () => { const args = getArgs() assert.equal(typeof args, 'object') - assert.ok(Array.isArray(args.params)) }) - it('should include the underscore array from minimist', () => { + it('should include params as an array', () => { const args = getArgs() - assert.ok(Array.isArray(args._)) - }) - - it('should derive params by slicing first two entries from _', () => { - const args = getArgs() - assert.deepEqual(args.params, args._.slice(2)) + assert.ok(Array.isArray(args.params)) }) }) From 2823e33ae15b53096b33aebdcc09f728b9a790c3 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 11:28:53 +0000 Subject: [PATCH 37/49] Update: Move App tests to integration test repo All App tests rely on App.instance which triggers a real boot, making them integration tests rather than unit tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/App.spec.js | 152 ---------------------------------------------- 1 file changed, 152 deletions(-) delete mode 100644 tests/App.spec.js diff --git a/tests/App.spec.js b/tests/App.spec.js deleted file mode 100644 index ff75c366..00000000 --- a/tests/App.spec.js +++ /dev/null @@ -1,152 +0,0 @@ -import { describe, it, before, after } from 'node:test' -import assert from 'node:assert/strict' -import fs from 'fs-extra' -import path from 'path' -import { fileURLToPath } from 'url' -import App from '../lib/App.js' - -const __dirname = path.dirname(fileURLToPath(import.meta.url)) - -describe('App', () => { - let testRootDir - let originalRootDir - - before(async () => { - testRootDir = path.join(__dirname, 'data', 'app-test') - await fs.ensureDir(testRootDir) - await fs.writeJson(path.join(testRootDir, 'package.json'), { - name: 'test-app', - version: '1.0.0' - }) - await fs.writeJson(path.join(testRootDir, 'adapt-authoring.json'), { - essentialApis: [] - }) - originalRootDir = process.env.ROOT_DIR - process.env.ROOT_DIR = testRootDir - }) - - after(async () => { - if (originalRootDir !== undefined) { - process.env.ROOT_DIR = originalRootDir - } else { - delete process.env.ROOT_DIR - } - await fs.remove(testRootDir) - }) - - describe('.instance', () => { - it('should return an App instance', () => { - const app = App.instance - assert.ok(app instanceof App) - }) - - it('should return the same instance on subsequent calls (singleton)', () => { - const app1 = App.instance - const app2 = App.instance - assert.equal(app1, app2) - }) - }) - - describe('constructor', () => { - it('should set name to adapt-authoring-core', () => { - const app = App.instance - assert.equal(app.name, 'adapt-authoring-core') - }) - - it('should set rootDir from ROOT_DIR env var', () => { - const app = App.instance - assert.equal(app.rootDir, testRootDir) - }) - - it('should initialize git info', () => { - const app = App.instance - assert.equal(typeof app.git, 'object') - }) - }) - - describe('#dependencies', () => { - it('should return the dependency configs from dependencyloader', () => { - const app = App.instance - assert.equal(typeof app.dependencies, 'object') - assert.equal(app.dependencies, app.dependencyloader.configs) - }) - }) - - describe('#getGitInfo()', () => { - it('should return an object', () => { - const app = App.instance - const info = app.getGitInfo() - assert.equal(typeof info, 'object') - }) - - it('should return empty object when .git directory does not exist', () => { - const app = App.instance - const origRootDir = app.rootDir - app.rootDir = '/nonexistent/path' - const info = app.getGitInfo() - app.rootDir = origRootDir - assert.deepEqual(info, {}) - }) - - it('should return object with branch and commit when .git exists', async () => { - const gitDir = path.join(testRootDir, '.git') - const refsDir = path.join(gitDir, 'refs', 'heads') - await fs.ensureDir(refsDir) - await fs.writeFile(path.join(gitDir, 'HEAD'), 'ref: refs/heads/main\n') - await fs.writeFile(path.join(refsDir, 'main'), 'abc123def456\n') - - const app = App.instance - const origRootDir = app.rootDir - app.rootDir = testRootDir - const info = app.getGitInfo() - app.rootDir = origRootDir - - assert.equal(info.branch, 'main') - assert.equal(info.commit, 'abc123def456') - - await fs.remove(gitDir) - }) - }) - - describe('#waitForModule()', () => { - it('should delegate to dependencyloader.waitForModule', async () => { - const app = App.instance - let calledWith - const origWaitForModule = app.dependencyloader.waitForModule.bind(app.dependencyloader) - app.dependencyloader.waitForModule = async (name) => { - calledWith = name - return { name } - } - const result = await app.waitForModule('test-mod') - app.dependencyloader.waitForModule = origWaitForModule - - assert.equal(calledWith, 'test-mod') - assert.deepEqual(result, { name: 'test-mod' }) - }) - - it('should return array when multiple module names are passed', async () => { - const app = App.instance - const origWaitForModule = app.dependencyloader.waitForModule.bind(app.dependencyloader) - app.dependencyloader.waitForModule = async (name) => ({ name }) - const result = await app.waitForModule('mod-a', 'mod-b') - app.dependencyloader.waitForModule = origWaitForModule - - assert.ok(Array.isArray(result)) - assert.equal(result.length, 2) - assert.deepEqual(result[0], { name: 'mod-a' }) - assert.deepEqual(result[1], { name: 'mod-b' }) - }) - - it('should return single result (not array) for single module', async () => { - const app = App.instance - const origWaitForModule = app.dependencyloader.waitForModule.bind(app.dependencyloader) - app.dependencyloader.waitForModule = async (name) => ({ name }) - const result = await app.waitForModule('single-mod') - app.dependencyloader.waitForModule = origWaitForModule - - assert.ok(!Array.isArray(result)) - assert.deepEqual(result, { name: 'single-mod' }) - }) - }) - -}) From 255892333a028e24296e96ea312edab8e2f829ed Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 12:59:32 +0000 Subject: [PATCH 38/49] Refactor bootstrap libraries for consistency and fix init race condition Move instance variable definitions into init() to fix race condition where super() triggers init() before constructor body runs. Make Config, Errors, and Lang use consistent constructor patterns with shared options object. Convert Errors and Lang to synchronous. Use log function instead of logger instance across all libraries. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 94 ++++++++++++++++++++++++--------------------------- lib/Config.js | 52 +++++++++++++++------------- lib/Errors.js | 33 ++++++++---------- lib/Lang.js | 32 +++++++++++------- 4 files changed, 108 insertions(+), 103 deletions(-) diff --git a/lib/App.js b/lib/App.js index 5a633d01..49800c22 100644 --- a/lib/App.js +++ b/lib/App.js @@ -35,72 +35,67 @@ class App extends AbstractModule { const adaptJson = JSON.parse(fs.readFileSync(path.join(rootDir, metadataFileName))) const packageJson = JSON.parse(fs.readFileSync(path.join(rootDir, packageFileName))) super(null, { ...packageJson, ...adaptJson, name: 'adapt-authoring-core', rootDir }) - /** - * Instance of App instance (required by all AbstractModules) - * @type {App} - */ - this.app = this - /** - * Reference to the passed arguments (parsed for easy reference) - * @type {Object} - */ - this.args = getArgs() - /** - * Reference to the DependencyLoader instance - * @type {DependencyLoader} - */ - this.dependencyloader = new DependencyLoader(this) - this.git = this.getGitInfo() } /** @override */ async init () { - /** - * Reference to the Logger instance - * @type {Logger} - */ - this.logger = new Logger() - try { - await this.dependencyloader.loadConfigs() - - const configFilePath = path.join(this.rootDir, 'conf', `${process.env.NODE_ENV}.config.js`) - - await runMigrations({ - dependencies: this.dependencies, - configFilePath, - rootDir: this.rootDir, - log: (level, ...args) => this.logger.log(level, ...args), - dryRun: this.args['dry-run'] === true - }) - + /** + * Instance of App instance (required by all AbstractModules) + * @type {App} + */ + this.app = this + /** + * Reference to the passed arguments (parsed for easy reference) + * @type {Object} + */ + this.args = getArgs() /** * Reference to the Config instance * @type {Config} */ - this.config = await Config.load({ - rootDir: this.rootDir, - configFilePath, - dependencies: this.dependencies, - appName: this.name, - logger: this.logger - }) - - this.logger = new Logger({ - levels: this.getConfig('logLevels'), - showTimestamp: this.getConfig('showLogTimestamp') - }) + this.config = undefined /** * Reference to the error registry * @type {Errors} */ - this.errors = await Errors.load(this.dependencies, msg => this.logger.log('warn', 'errors', msg)) + this.errors = undefined /** * Reference to the Lang instance * @type {Lang} */ - this.lang = new Lang() - await this.lang.loadPhrases(this.dependencies, this.rootDir, (...args) => this.logger.log('error', 'lang', ...args)) + this.lang = undefined + /** + * Reference to the Logger instance + * @type {Logger} + */ + this.logger = new Logger() + /** + * Reference to the DependencyLoader instance + * @type {DependencyLoader} + */ + this.dependencyloader = new DependencyLoader(this) + /** + * Git metadata for the application (branch and commit hash) + * @type {Object} + */ + this.git = this.getGitInfo() + + await this.dependencyloader.loadConfigs() + + const options = { + dependencies: this.dependencies, + configFilePath: path.join(this.rootDir, 'conf', `${process.env.NODE_ENV}.config.js`), + rootDir: this.rootDir, + log: (...args) => this.logger.log(...args) + } + + await runMigrations({ ...options, dryRun: this.args['dry-run'] === true }) + + this.config = await new Config({ ...options, appName: this.name }).load() + this.logger = new Logger({ levels: this.getConfig('logLevels'), showTimestamp: this.getConfig('showLogTimestamp') }) + this.errors = new Errors(options) + this.lang = new Lang(options) await this.dependencyloader.loadModules() @@ -108,6 +103,7 @@ class App extends AbstractModule { this.log('verbose', 'DIR', 'rootDir', this.rootDir) this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) + } catch (cause) { process.exitCode = 1 throw new Error('Failed to start App', { cause }) diff --git a/lib/Config.js b/lib/Config.js index 6925b655..2203ca52 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -9,43 +9,49 @@ import path from 'path' */ class Config { /** - * Loads configuration from all sources - * @param {String} rootDir Application root directory - * @param {Object} dependencies Key/value map of dependency configs - * @param {String} appName The core module name (for sorting) - * @param {Logger} logger Logger instance for output - * @returns {Promise} + * @param {Object} options + * @param {String} options.rootDir Application root directory + * @param {String} options.configFilePath Path to the user configuration file + * @param {Object} options.dependencies Key/value map of dependency configs + * @param {String} options.appName The core module name (for sorting) + * @param {Function} options.log Logging function (level, id, ...args) */ - static async load ({ rootDir, configFilePath, dependencies, appName, logger }) { - const instance = new Config() - instance.rootDir = rootDir - instance.logger = logger - instance.configFilePath = configFilePath - await instance.storeUserSettings() - instance.storeEnvSettings() - instance.storeSchemaSettings(dependencies, appName) - logger.log('info', 'config', `using config at ${configFilePath}`) - return instance - } - - constructor () { + constructor ({ rootDir, configFilePath, dependencies, appName, log }) { /** @ignore */ this._config = {} /** * Application root directory * @type {String} */ - this.rootDir = undefined + this.rootDir = rootDir /** * Path to the user configuration file * @type {String} */ - this.configFilePath = undefined + this.configFilePath = configFilePath /** * The keys for all attributes marked as public * @type {Array} */ this.publicAttributes = [] + /** @ignore */ + this._dependencies = dependencies + /** @ignore */ + this._appName = appName + /** @ignore */ + this.log = log + } + + /** + * Loads configuration from all sources + * @returns {Promise} + */ + async load () { + await this.storeUserSettings() + this.storeEnvSettings() + this.storeSchemaSettings(this._dependencies, this._appName) + this.log('info', 'config', `using config at ${this.configFilePath}`) + return this } /** @@ -87,7 +93,7 @@ class Config { await fs.readFile(this.configFilePath) config = (await import(this.configFilePath)).default } catch (e) { - this.logger.log('warn', 'config', `Failed to load config at ${this.configFilePath}: ${e}. Will attempt to run with defaults.`) + this.log('warn', 'config', `Failed to load config at ${this.configFilePath}: ${e}. Will attempt to run with defaults.`) return } Object.entries(config).forEach(([name, c]) => { @@ -135,7 +141,7 @@ class Config { } if (errors.length) { errors.forEach(e => { - this.logger.log('error', 'config', `${e.modName ? e.modName + ': ' : ''}${e.message}`) + this.log('error', 'config', `${e.modName ? e.modName + ': ' : ''}${e.message}`) }) throw new Error('Config validation failed') } diff --git a/lib/Errors.js b/lib/Errors.js index ed4ebb54..45527842 100644 --- a/lib/Errors.js +++ b/lib/Errors.js @@ -1,6 +1,6 @@ import AdaptError from './AdaptError.js' -import fs from 'fs/promises' -import { glob } from 'glob' +import fs from 'node:fs' +import { globSync } from 'glob' /** * Loads and stores all error definitions for the application. Errors are accessed via human-readable error codes for better readability when thrown in code. @@ -8,35 +8,33 @@ import { glob } from 'glob' */ class Errors { /** - * Loads all errors defined in Adapt module dependencies - * @param {Object} dependencies Key/value map of dependency configs (each with a rootDir) - * @param {Function} [logWarn] Optional logging function for duplicate warnings - * @returns {Promise} + * @param {Object} options + * @param {Object} options.dependencies Key/value map of dependency configs (each with a rootDir) + * @param {Function} [options.log] Optional logging function (level, id, ...args) */ - static async load (dependencies, logWarn) { - const instance = new Errors() + constructor ({ dependencies, log } = {}) { const errorDefs = {} - await Promise.all(Object.values(dependencies).map(async d => { - const files = await glob('errors/*.json', { cwd: d.rootDir, absolute: true }) - await Promise.all(files.map(async f => { + for (const d of Object.values(dependencies)) { + const files = globSync('errors/*.json', { cwd: d.rootDir, absolute: true }) + for (const f of files) { try { - const contents = JSON.parse(await fs.readFile(f)) + const contents = JSON.parse(fs.readFileSync(f)) Object.entries(contents).forEach(([k, v]) => { if (errorDefs[k]) { - if (logWarn) logWarn(`error code '${k}' already defined`) + log?.('warn', 'errors', `error code '${k}' already defined`) return } errorDefs[k] = v }) } catch (e) { - if (logWarn) logWarn(e.message) + log?.('warn', 'errors', e.message) } - })) - })) + } + } Object.entries(errorDefs) .sort() .forEach(([k, { description, statusCode, isFatal, data }]) => { - Object.defineProperty(instance, k, { + Object.defineProperty(this, k, { get: () => { const metadata = { description } if (isFatal) metadata.isFatal = true @@ -46,7 +44,6 @@ class Errors { enumerable: true }) }) - return instance } } diff --git a/lib/Lang.js b/lib/Lang.js index 95150372..7c5c4b84 100644 --- a/lib/Lang.js +++ b/lib/Lang.js @@ -1,5 +1,5 @@ -import fs from 'node:fs/promises' -import { glob } from 'glob' +import fs from 'node:fs' +import { globSync } from 'glob' import path from 'node:path' /** @@ -7,12 +7,19 @@ import path from 'node:path' * @memberof core */ class Lang { - constructor () { + /** + * @param {Object} options + * @param {Object} options.dependencies Key/value map of dependency configs (each with a rootDir) + * @param {String} options.rootDir The application root directory + * @param {Function} [options.log] Optional logging function (level, id, ...args) + */ + constructor ({ dependencies, rootDir, log } = {}) { /** * The loaded language phrases * @type {Object} */ this.phrases = {} + this.loadPhrases(dependencies, rootDir, log) } /** @@ -27,25 +34,24 @@ class Lang { * Loads and merges all language phrases from dependencies * @param {Object} dependencies Key/value map of dependency configs (each with a rootDir) * @param {String} appRootDir The application root directory - * @param {Function} [logError] Optional logging function for errors - * @returns {Promise} + * @param {Function} [log] Optional logging function (level, id, ...args) */ - async loadPhrases (dependencies, appRootDir, logError) { + loadPhrases (dependencies, appRootDir, log) { const dirs = [ appRootDir, ...Object.values(dependencies).map(d => d.rootDir) ] - await Promise.all(dirs.map(async dir => { - const files = await glob('lang/*.json', { cwd: dir, absolute: true }) - await Promise.all(files.map(async f => { + for (const dir of dirs) { + const files = globSync('lang/*.json', { cwd: dir, absolute: true }) + for (const f of files) { try { - const contents = JSON.parse(await fs.readFile(f, 'utf8')) + const contents = JSON.parse(fs.readFileSync(f, 'utf8')) Object.entries(contents).forEach(([k, v]) => Lang.storeStrings(this.phrases, `${path.basename(f).replace('.json', '')}.${k}`, v)) } catch (e) { - if (logError) logError(e.message, f) + log?.('error', 'lang', e.message, f) } - })) - })) + } + } } /** From fc3a1e0926d86bb05900fb0e1982b2ce5f4b2c47 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 15:38:07 +0000 Subject: [PATCH 39/49] Fix: Set $id on config schemas and log schema errors Config schemas lack $id, causing AJV "already exists" collisions when multiple schemas compile into the same validator instance. Adds a temporary workaround to set $id from the module name until config schemas define their own. Also logs non-file-not-found schema errors instead of swallowing them silently. See https://github.com/adapt-security/adapt-authoring-jsonschema/issues/66 Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/Config.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/Config.js b/lib/Config.js index 2203ca52..578d1327 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -157,8 +157,17 @@ class Config { const schemaPath = path.resolve(pkg.rootDir, 'conf/config.schema.json') let schema try { - schema = schemas.createSchema(schemaPath).build() + // TODO config schemas should define $id, remove this workaround once they do + schema = schemas.createSchema(schemaPath) + schema.raw.$id = pkg.name + schema.build({ compile: false }) + schema.built.$id = pkg.name + schema.compiledWithDefaults = schemas.validatorWithDefaults.compile(schema.built) + schema.compiled = schemas.validator.compile(schema.built) } catch (e) { + if (e.code !== 'SCHEMA_LOAD_FAILED') { + this.log('warn', 'config', `${pkg.name}: ${e.message}`) + } return } const dirKeys = new Set() From 9605f9ef5e3f27de29284a10e5227bb58cf7e4fb Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 17:58:38 +0000 Subject: [PATCH 40/49] Fix: Simplify Lang translate API and restore missing functionality - Remove logWarn param from instance translate/translateError signatures to match how callers actually use them (lang, key, data) - Store defaultLang from config rather than requiring it per-call - Merge translateError into translate (handles Error keys inline) - Extract substituteData for readability - Remove static translate/translateError (instance methods are the API) - Pass defaultLang config value from App Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 2 +- lib/Lang.js | 126 ++++++++++++++++++++------------------------- tests/Lang.spec.js | 42 +++++++-------- 3 files changed, 79 insertions(+), 91 deletions(-) diff --git a/lib/App.js b/lib/App.js index 49800c22..30d024d8 100644 --- a/lib/App.js +++ b/lib/App.js @@ -95,7 +95,7 @@ class App extends AbstractModule { this.config = await new Config({ ...options, appName: this.name }).load() this.logger = new Logger({ levels: this.getConfig('logLevels'), showTimestamp: this.getConfig('showLogTimestamp') }) this.errors = new Errors(options) - this.lang = new Lang(options) + this.lang = new Lang({ ...options, defaultLang: this.getConfig('defaultLang') }) await this.dependencyloader.loadModules() diff --git a/lib/Lang.js b/lib/Lang.js index 7c5c4b84..372c9ea4 100644 --- a/lib/Lang.js +++ b/lib/Lang.js @@ -10,15 +10,26 @@ class Lang { /** * @param {Object} options * @param {Object} options.dependencies Key/value map of dependency configs (each with a rootDir) + * @param {String} options.defaultLang The default language for translations * @param {String} options.rootDir The application root directory * @param {Function} [options.log] Optional logging function (level, id, ...args) */ - constructor ({ dependencies, rootDir, log } = {}) { + constructor ({ dependencies, defaultLang, rootDir, log } = {}) { /** * The loaded language phrases * @type {Object} */ this.phrases = {} + /** + * The default language for translations + * @type {String} + */ + this.defaultLang = defaultLang + /** + * Optional logging function (level, id, ...args) + * @type {Function} + */ + this.log = log this.loadPhrases(dependencies, rootDir, log) } @@ -55,28 +66,59 @@ class Lang { } /** - * Returns translated language string - * @param {String} defaultLang Default language to use when lang is not a string - * @param {Function} logWarn Logging function for missing keys - * @param {String} lang The target language - * @param {String|AdaptError} key The unique string key + * Returns translated language string. If key is an Error, translates using + * the error code as the key and error data for substitution. Non-Error, + * non-string values are returned unchanged. + * @param {String} lang The target language (falls back to defaultLang) + * @param {String|Error} key The unique string key, or an Error to translate * @param {Object} data Dynamic data to be inserted into translated string * @return {String} */ - translate (defaultLang, logWarn, lang, key, data) { - return Lang.translate(this.phrases, defaultLang, logWarn, lang, key, data) + translate (lang, key, data) { + if (typeof lang !== 'string') { + lang = this.defaultLang + } + if (key instanceof Error) { + return this.translate(lang, `error.${key.code}`, key.data ?? key) + } + if (typeof key !== 'string') { + return key + } + const s = this.phrases[lang]?.[key] + if (!s) { + this.log?.('warn', 'lang', `missing key '${lang}.${key}'`) + return key + } + if (!data) { + return s + } + return this.substituteData(s, lang, data) } /** - * Translates an AdaptError - * @param {String} defaultLang Default language to use - * @param {Function} logWarn Logging function for missing keys + * Replaces placeholders in a translated string with data values. + * Supports ${key} for simple substitution, and $map{key:attrs:delim} + * for mapping over array values. + * @param {String} s The translated string * @param {String} lang The target language - * @param {AdaptError} error Error to translate - * @returns The translated error + * @param {Object} data Key/value pairs to substitute + * @return {String} */ - translateError (defaultLang, logWarn, lang, error) { - return Lang.translateError(this.phrases, defaultLang, logWarn, lang, error) + substituteData (s, lang, data) { + for (const [k, v] of Object.entries(data)) { + const resolved = Array.isArray(v) + ? v.map(item => item instanceof Error ? this.translate(lang, item) : item) + : v instanceof Error ? this.translate(lang, v) : v + s = s.replaceAll(`\${${k}}`, resolved) + if (Array.isArray(resolved)) { + for (const [match, expr] of s.matchAll(new RegExp(String.raw`\$map{${k}:(.+)}`, 'g'))) { + const [attrs, delim] = expr.split(':') + const mapped = resolved.map(val => attrs.split(',').map(a => val?.[a] ?? a).join(delim)) + s = s.replace(match, mapped) + } + } + } + return s } /** @@ -91,60 +133,6 @@ class Lang { if (!phrases[lang]) phrases[lang] = {} phrases[lang][key.slice(i + 1)] = value } - - /** - * Returns translated language string - * @param {Object} phrases The phrases dictionary - * @param {String} defaultLang Default language to use when lang is not a string - * @param {Function} logWarn Logging function for missing keys - * @param {String} lang The target language - * @param {String|AdaptError} key The unique string key - * @param {Object} data Dynamic data to be inserted into translated string - * @return {String} - */ - static translate (phrases, defaultLang, logWarn, lang, key, data) { - if (typeof lang !== 'string') { - lang = defaultLang - } - if (key instanceof Error) { - return Lang.translateError(phrases, defaultLang, logWarn, lang, key) - } - const s = phrases[lang]?.[key] - if (!s) { - logWarn(`missing key '${lang}.${key}'`) - return key - } - if (!data) { - return s - } - return Object.entries(data).reduce((s, [k, v]) => { - v = Array.isArray(v) ? v.map(v2 => Lang.translateError(phrases, defaultLang, logWarn, lang, v2)) : Lang.translateError(phrases, defaultLang, logWarn, lang, v) - s = s.replaceAll(`\${${k}}`, v) - if (Array.isArray(v)) { - const matches = [...s.matchAll(new RegExp(String.raw`\$map{${k}:(.+)}`, 'g'))] - matches.forEach(([replace, data]) => { - const [attrs, delim] = data.split(':') - s = s.replace(replace, v.map(val => attrs.split(',').map(a => Object.prototype.hasOwnProperty.call(val, a) ? val[a] : a)).join(delim)) - }) - } - return s - }, s) - } - - /** - * Translates an AdaptError - * @param {Object} phrases The phrases dictionary - * @param {String} defaultLang Default language to use - * @param {Function} logWarn Logging function for missing keys - * @param {String} lang The target language - * @param {AdaptError} error Error to translate - * @returns The translated error - */ - static translateError (phrases, defaultLang, logWarn, lang, error) { - return error instanceof Error - ? Lang.translate(phrases, defaultLang, logWarn, lang, `error.${error.code}`, error.data ?? error) - : error - } } export default Lang diff --git a/tests/Lang.spec.js b/tests/Lang.spec.js index 4105d4d3..238950fa 100644 --- a/tests/Lang.spec.js +++ b/tests/Lang.spec.js @@ -7,6 +7,12 @@ import { fileURLToPath } from 'url' const __dirname = path.dirname(fileURLToPath(import.meta.url)) +function createLang (phrases, defaultLang = 'en') { + const lang = new Lang({ dependencies: {}, defaultLang, rootDir: __dirname }) + lang.phrases = phrases + return lang +} + describe('Lang', () => { let testDir @@ -68,47 +74,41 @@ describe('Lang', () => { }) }) - describe('.translate()', () => { + describe('#translate()', () => { it('should return translated string', () => { - const phrases = { en: { hello: 'Hello' } } - const result = Lang.translate(phrases, 'en', () => {}, 'en', 'hello') - assert.equal(result, 'Hello') + const lang = createLang({ en: { hello: 'Hello' } }) + assert.equal(lang.translate('en', 'hello'), 'Hello') }) it('should substitute data placeholders', () => { // eslint-disable-next-line no-template-curly-in-string - const phrases = { en: { greeting: 'Hello ${name}' } } - const result = Lang.translate(phrases, 'en', () => {}, 'en', 'greeting', { name: 'World' }) - assert.equal(result, 'Hello World') + const lang = createLang({ en: { greeting: 'Hello ${name}' } }) + assert.equal(lang.translate('en', 'greeting', { name: 'World' }), 'Hello World') }) it('should fall back to default lang when lang is not a string', () => { - const phrases = { en: { hello: 'Hello' } } - const result = Lang.translate(phrases, 'en', () => {}, undefined, 'hello') - assert.equal(result, 'Hello') + const lang = createLang({ en: { hello: 'Hello' } }) + assert.equal(lang.translate(undefined, 'hello'), 'Hello') }) it('should return key and warn when key is missing', () => { - const phrases = { en: {} } let warned = false - const result = Lang.translate(phrases, 'en', () => { warned = true }, 'en', 'missing.key') - assert.equal(result, 'missing.key') + const lang = createLang({ en: {} }) + lang.log = () => { warned = true } + assert.equal(lang.translate('en', 'missing.key'), 'missing.key') assert.ok(warned) }) - }) - describe('.translateError()', () => { - it('should return non-error values unchanged', () => { - const result = Lang.translateError({}, 'en', () => {}, 'en', 'plain string') - assert.equal(result, 'plain string') + it('should return non-error, non-string values unchanged', () => { + const lang = createLang({}) + assert.equal(lang.translate('en', 42), 42) }) it('should translate an error using its code', () => { - const phrases = { en: { 'error.TEST': 'Translated error' } } + const lang = createLang({ en: { 'error.TEST': 'Translated error' } }) const error = new Error('TEST') error.code = 'TEST' - const result = Lang.translateError(phrases, 'en', () => {}, 'en', error) - assert.equal(result, 'Translated error') + assert.equal(lang.translate('en', error), 'Translated error') }) }) }) From 5b3dddcf65db8fa130156d4ff02a5b6b2db8ce75 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 18:01:43 +0000 Subject: [PATCH 41/49] Refactor: Inline storeStrings into loadPhrases Only used in one place, no need for a separate static method. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/Lang.js | 17 ++++------------- tests/Lang.spec.js | 14 -------------- 2 files changed, 4 insertions(+), 27 deletions(-) diff --git a/lib/Lang.js b/lib/Lang.js index 372c9ea4..81ea4c64 100644 --- a/lib/Lang.js +++ b/lib/Lang.js @@ -56,8 +56,10 @@ class Lang { const files = globSync('lang/*.json', { cwd: dir, absolute: true }) for (const f of files) { try { + const lang = path.basename(f, '.json') + if (!this.phrases[lang]) this.phrases[lang] = {} const contents = JSON.parse(fs.readFileSync(f, 'utf8')) - Object.entries(contents).forEach(([k, v]) => Lang.storeStrings(this.phrases, `${path.basename(f).replace('.json', '')}.${k}`, v)) + Object.entries(contents).forEach(([k, v]) => { this.phrases[lang][k] = v }) } catch (e) { log?.('error', 'lang', e.message, f) } @@ -121,18 +123,7 @@ class Lang { return s } - /** - * Parses a dotted language key and stores the value in the phrases dictionary - * @param {Object} phrases The phrases dictionary to store into - * @param {String} key Key in the format 'lang.namespace.key' - * @param {String} value The string value to store - */ - static storeStrings (phrases, key, value) { - const i = key.indexOf('.') - const lang = key.slice(0, i) - if (!phrases[lang]) phrases[lang] = {} - phrases[lang][key.slice(i + 1)] = value - } + } export default Lang diff --git a/tests/Lang.spec.js b/tests/Lang.spec.js index 238950fa..e057f7e7 100644 --- a/tests/Lang.spec.js +++ b/tests/Lang.spec.js @@ -60,20 +60,6 @@ describe('Lang', () => { }) }) - describe('.storeStrings()', () => { - it('should store a string under lang.key', () => { - const phrases = {} - Lang.storeStrings(phrases, 'en.hello', 'world') - assert.equal(phrases.en.hello, 'world') - }) - - it('should create lang bucket if missing', () => { - const phrases = {} - Lang.storeStrings(phrases, 'de.greeting', 'Hallo') - assert.ok(phrases.de) - }) - }) - describe('#translate()', () => { it('should return translated string', () => { const lang = createLang({ en: { hello: 'Hello' } }) From 329001e5ea5997c483a8cde2f99d2384973a8add Mon Sep 17 00:00:00 2001 From: Tom Taylor Date: Fri, 27 Mar 2026 18:02:29 +0000 Subject: [PATCH 42/49] Remove unnecessary blank lines in Lang.js --- lib/Lang.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/Lang.js b/lib/Lang.js index 81ea4c64..c5b20e96 100644 --- a/lib/Lang.js +++ b/lib/Lang.js @@ -122,8 +122,6 @@ class Lang { } return s } - - } export default Lang From 34e07d446cb618fb6dd76d06779fc2df378c0355 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 18:04:55 +0000 Subject: [PATCH 43/49] Refactor: Normalise substituteData to avoid array branching Use [v].flat() to handle both scalar and array values in a single code path. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/Lang.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/Lang.js b/lib/Lang.js index c5b20e96..bdb6097f 100644 --- a/lib/Lang.js +++ b/lib/Lang.js @@ -108,16 +108,11 @@ class Lang { */ substituteData (s, lang, data) { for (const [k, v] of Object.entries(data)) { - const resolved = Array.isArray(v) - ? v.map(item => item instanceof Error ? this.translate(lang, item) : item) - : v instanceof Error ? this.translate(lang, v) : v - s = s.replaceAll(`\${${k}}`, resolved) - if (Array.isArray(resolved)) { - for (const [match, expr] of s.matchAll(new RegExp(String.raw`\$map{${k}:(.+)}`, 'g'))) { - const [attrs, delim] = expr.split(':') - const mapped = resolved.map(val => attrs.split(',').map(a => val?.[a] ?? a).join(delim)) - s = s.replace(match, mapped) - } + const items = [v].flat().map(item => item instanceof Error ? this.translate(lang, item) : item) + s = s.replaceAll(`\${${k}}`, items) + for (const [match, expr] of s.matchAll(new RegExp(String.raw`\$map{${k}:(.+)}`, 'g'))) { + const [attrs, delim] = expr.split(':') + s = s.replace(match, items.map(val => attrs.split(',').map(a => val?.[a] ?? a).join(delim))) } } return s From 9ddbd177bc4c5560467b7800a9cf23ff021afd77 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 18:37:34 +0000 Subject: [PATCH 44/49] Fix: Prevent unrelated module failures from cascading via waitForModule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when any module failed to load, the moduleLoadedHook fired with (error, undefined), causing every pending waitForModule promise to reject — even for unrelated modules. This led to misleading errors (e.g. reporting mongodb as failed when ui was the actual failure). Now the failed module name is passed through the hook, and waitForModule only rejects if the specific module being waited for is the one that failed. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/DependencyLoader.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/DependencyLoader.js b/lib/DependencyLoader.js index 19b1718f..41e1e4ac 100644 --- a/lib/DependencyLoader.js +++ b/lib/DependencyLoader.js @@ -143,7 +143,7 @@ class DependencyLoader { await this.moduleLoadedHook.invoke(null, instance) return instance } catch (e) { - await this.moduleLoadedHook.invoke(e) + await this.moduleLoadedHook.invoke(e, { name: modName }) throw e } } @@ -196,8 +196,10 @@ class DependencyLoader { } return new Promise((resolve, reject) => { this.moduleLoadedHook.tap((error, instance) => { - if (error) return reject(this.app.errors.DEP_FAILED.setData({ module: modName })) - if (instance?.name === modName) resolve(instance) + if (instance?.name === modName) return resolve(instance) + if (error && instance?.name === modName) { + return reject(this.app.errors.DEP_FAILED.setData({ module: modName })) + } }) }) } From 5ec967f2733c4de2199c04a2cb9b262e13cacc94 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 19:48:32 +0000 Subject: [PATCH 45/49] Refactor: Use subdirectory structure for lang files Load lang phrases from lang//.json instead of lang/..json. Plain lang/.json files still work for simple cases. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/Lang.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/Lang.js b/lib/Lang.js index bdb6097f..a3535edc 100644 --- a/lib/Lang.js +++ b/lib/Lang.js @@ -53,13 +53,16 @@ class Lang { ...Object.values(dependencies).map(d => d.rootDir) ] for (const dir of dirs) { - const files = globSync('lang/*.json', { cwd: dir, absolute: true }) + const files = globSync('lang/**/*.json', { cwd: dir, absolute: true }) for (const f of files) { try { - const lang = path.basename(f, '.json') + const relative = path.relative(path.join(dir, 'lang'), f) + const parts = relative.replace(/\.json$/, '').split(path.sep) + const lang = parts[0] + const prefix = parts.length > 1 ? parts.slice(1).join('.') + '.' : '' if (!this.phrases[lang]) this.phrases[lang] = {} const contents = JSON.parse(fs.readFileSync(f, 'utf8')) - Object.entries(contents).forEach(([k, v]) => { this.phrases[lang][k] = v }) + Object.entries(contents).forEach(([k, v]) => { this.phrases[lang][`${prefix}${k}`] = v }) } catch (e) { log?.('error', 'lang', e.message, f) } From 9548d74f5e2f4dae56202b7f63aabe7ad06dff12 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Fri, 27 Mar 2026 20:55:50 +0000 Subject: [PATCH 46/49] Fix: Address PR review feedback - Fix waitForModule to only reject for the specific waited-for module - Fix dataToMd recursive accumulator duplication in docs plugin - Add defaults to Config/Lang/Errors constructors for test compatibility - Add .catch() on Logger logHook to prevent unhandled promise rejections - Fix error-handling.md duplicate heading, typo, and outdated references - Update Errors tests to use constructor instead of non-existent .load() - Clean up unused imports in Config tests - Add fs-extra as devDependency - Fix lint issues in App.js Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/error-handling.md | 9 ++++----- docs/plugins/errors.js | 11 ++++++----- lib/App.js | 1 - lib/Config.js | 2 +- lib/DependencyLoader.js | 7 +++---- lib/Lang.js | 4 ++-- lib/Logger.js | 4 +++- package.json | 1 + tests/Config.spec.js | 7 +------ tests/Errors.spec.js | 36 ++++++++++++++++++------------------ 10 files changed, 39 insertions(+), 43 deletions(-) diff --git a/docs/error-handling.md b/docs/error-handling.md index e7c54974..f298009d 100644 --- a/docs/error-handling.md +++ b/docs/error-handling.md @@ -8,7 +8,6 @@ Before going into specifics, it would be useful to discuss application errors in You will need to deal with each category of error differently. Below are some general tips on handling each type of error. -## Initialisation errors ## Initialisation errors Any errors which occur during initialisation should be captured and logged as appropriate. Depending on the type of error, it may or may not be considered fatal to your code. @@ -23,7 +22,7 @@ Some examples: Some examples: - For a database-handler module, disconnecting from the database is an expected error, and can be handled and rectified easily. -## User errors errors +## User errors User errors are any errors which are caused as a direct result of a user performing an action incorrectly. It is even *more* critical with user errors that the error is as specific and descriptive as possible, as the response needs to be informative and instructive to the user that caused the error. Failing to do so will result in an unpleasant user experience. Some examples: @@ -32,14 +31,14 @@ Some examples: ## Defining errors Depending on the kinds of error that you're dealing with in your code, it may be useful to include a set of custom error definitions specific to your code. -Defining useful errors is a critical part of any software system. The ErrorsModule makes it easy to define errors for your own modules, and make use of errors defined in other modules. +Defining useful errors is a critical part of any software system. The error registry makes it easy to define errors for your own modules, and make use of errors defined in other modules. ## Catching errors ## Throwing errors -As mentioned above, it is preferable to catch errors internally in your code and re-throw these errors +As mentioned above, it is preferable to catch errors internally in your code and re-throw these errors. -The ErrorsModule acts as a central store for all errors defined in the system, and errors can be accessed and thrown from here. For convenience, the Errors module is made available directly as a property of the main App instance, or by referencing the module in the usual way: `App#waitForModule('errors)` +The error registry acts as a central store for all errors defined in the system, and errors can be accessed and thrown from here. For convenience, the errors registry is available directly as a property of the main App instance via `app.errors`: ```js try { diff --git a/docs/plugins/errors.js b/docs/plugins/errors.js index 3ef6efc2..e60ab5d4 100644 --- a/docs/plugins/errors.js +++ b/docs/plugins/errors.js @@ -12,10 +12,11 @@ export default class Errors { }, '| Error code | Description | HTTP status code | Supplemental data |\n| - | - | :-: | - |') } - dataToMd (data, s = '') { - if (!data) return s - return Object.entries(data).reduce((s, [k, v]) => { - return `${s}
  • \`${k}\`: ${typeof v === 'object' ? this.dataToMd(v, s) : v}
  • ` - }, s) + dataToMd (data) { + if (!data) return '' + return Object.entries(data).reduce((acc, [k, v]) => { + const nested = typeof v === 'object' ? this.dataToMd(v) : v + return `${acc}
  • \`${k}\`: ${nested}
  • ` + }, '') } } diff --git a/lib/App.js b/lib/App.js index 30d024d8..ba740d16 100644 --- a/lib/App.js +++ b/lib/App.js @@ -103,7 +103,6 @@ class App extends AbstractModule { this.log('verbose', 'DIR', 'rootDir', this.rootDir) this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) - } catch (cause) { process.exitCode = 1 throw new Error('Failed to start App', { cause }) diff --git a/lib/Config.js b/lib/Config.js index 578d1327..053c06c8 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -16,7 +16,7 @@ class Config { * @param {String} options.appName The core module name (for sorting) * @param {Function} options.log Logging function (level, id, ...args) */ - constructor ({ rootDir, configFilePath, dependencies, appName, log }) { + constructor ({ rootDir, configFilePath, dependencies = {}, appName = '', log = () => {} } = {}) { /** @ignore */ this._config = {} /** diff --git a/lib/DependencyLoader.js b/lib/DependencyLoader.js index 41e1e4ac..5c4df053 100644 --- a/lib/DependencyLoader.js +++ b/lib/DependencyLoader.js @@ -196,10 +196,9 @@ class DependencyLoader { } return new Promise((resolve, reject) => { this.moduleLoadedHook.tap((error, instance) => { - if (instance?.name === modName) return resolve(instance) - if (error && instance?.name === modName) { - return reject(this.app.errors.DEP_FAILED.setData({ module: modName })) - } + if (instance?.name !== modName) return + if (error) return reject(this.app.errors.DEP_FAILED.setData({ module: modName })) + resolve(instance) }) }) } diff --git a/lib/Lang.js b/lib/Lang.js index a3535edc..f13ec555 100644 --- a/lib/Lang.js +++ b/lib/Lang.js @@ -47,9 +47,9 @@ class Lang { * @param {String} appRootDir The application root directory * @param {Function} [log] Optional logging function (level, id, ...args) */ - loadPhrases (dependencies, appRootDir, log) { + loadPhrases (dependencies = {}, appRootDir, log) { const dirs = [ - appRootDir, + ...(appRootDir ? [appRootDir] : []), ...Object.values(dependencies).map(d => d.rootDir) ] for (const dir of dirs) { diff --git a/lib/Logger.js b/lib/Logger.js index 58e717ec..5e408099 100644 --- a/lib/Logger.js +++ b/lib/Logger.js @@ -56,7 +56,9 @@ class Logger { const logFunc = console[level] ?? console.log const timestamp = this.config.timestamp ? chalk.dim(`${new Date().toISOString()} `) : '' logFunc(`${timestamp}${colour ? colour(level) : level} ${chalk.magenta(id)}`, ...args) - this.logHook.invoke(new Date(), level, id, ...args) + this.logHook.invoke(new Date(), level, id, ...args).catch((error) => { + console.error('Logger logHook invocation failed:', error) + }) } /** diff --git a/package.json b/package.json index 4331bdf3..e7a8e013 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,7 @@ }, "devDependencies": { "@adaptlearning/semantic-release-config": "^1.0.0", + "fs-extra": "^11.3.4", "standard": "^17.1.0" }, "release": { diff --git a/tests/Config.spec.js b/tests/Config.spec.js index 0caf850c..a0a7beb8 100644 --- a/tests/Config.spec.js +++ b/tests/Config.spec.js @@ -1,11 +1,7 @@ -import { describe, it, before, after } from 'node:test' +import { describe, it } from 'node:test' import assert from 'node:assert/strict' import Config from '../lib/Config.js' -import fs from 'fs-extra' import path from 'path' -import { fileURLToPath } from 'url' - -const __dirname = path.dirname(fileURLToPath(import.meta.url)) describe('Config', () => { describe('constructor', () => { @@ -120,7 +116,6 @@ describe('Config', () => { it('should handle missing config file gracefully', async () => { const config = new Config() config.configFilePath = '/nonexistent/path/config.js' - config.logger = { log: () => {} } await assert.doesNotReject(() => config.storeUserSettings()) }) }) diff --git a/tests/Errors.spec.js b/tests/Errors.spec.js index 12c2135f..1e6dae32 100644 --- a/tests/Errors.spec.js +++ b/tests/Errors.spec.js @@ -33,44 +33,44 @@ describe('Errors', () => { await fs.remove(testDir) }) - describe('.load()', () => { - it('should load error definitions from dependencies', async () => { + describe('constructor', () => { + it('should load error definitions from dependencies', () => { const deps = { test: { name: 'test', rootDir: testDir } } - const errors = await Errors.load(deps) + const errors = new Errors({ dependencies: deps }) assert.ok(errors.TEST_ERROR) assert.ok(errors.FATAL_ERROR) }) - it('should return AdaptError instances', async () => { + it('should return AdaptError instances', () => { const deps = { test: { name: 'test', rootDir: testDir } } - const errors = await Errors.load(deps) + const errors = new Errors({ dependencies: deps }) assert.ok(errors.TEST_ERROR instanceof AdaptError) }) - it('should return fresh instances on each access', async () => { + it('should return fresh instances on each access', () => { const deps = { test: { name: 'test', rootDir: testDir } } - const errors = await Errors.load(deps) + const errors = new Errors({ dependencies: deps }) assert.notEqual(errors.TEST_ERROR, errors.TEST_ERROR) }) - it('should set statusCode from definition', async () => { + it('should set statusCode from definition', () => { const deps = { test: { name: 'test', rootDir: testDir } } - const errors = await Errors.load(deps) + const errors = new Errors({ dependencies: deps }) assert.equal(errors.TEST_ERROR.statusCode, 400) }) - it('should set isFatal from definition', async () => { + it('should set isFatal from definition', () => { const deps = { test: { name: 'test', rootDir: testDir } } - const errors = await Errors.load(deps) + const errors = new Errors({ dependencies: deps }) assert.equal(errors.FATAL_ERROR.isFatal, true) assert.equal(errors.TEST_ERROR.isFatal, false) }) - it('should warn on duplicate error codes', async () => { + it('should warn on duplicate error codes', () => { const dupDir = path.join(__dirname, 'data', 'errors-dup') const errorsDir = path.join(dupDir, 'errors') - await fs.ensureDir(errorsDir) - await fs.writeJson(path.join(errorsDir, 'dup.json'), { + fs.ensureDirSync(errorsDir) + fs.writeJsonSync(path.join(errorsDir, 'dup.json'), { TEST_ERROR: { description: 'duplicate', statusCode: 500 } }) const deps = { @@ -78,13 +78,13 @@ describe('Errors', () => { dup: { name: 'dup', rootDir: dupDir } } let warned = false - await Errors.load(deps, () => { warned = true }) + new Errors({ dependencies: deps, log: () => { warned = true } }) // eslint-disable-line no-new assert.ok(warned) - await fs.remove(dupDir) + fs.removeSync(dupDir) }) - it('should handle empty dependencies', async () => { - const errors = await Errors.load({}) + it('should handle empty dependencies', () => { + const errors = new Errors({ dependencies: {} }) assert.deepEqual(Object.keys(errors), []) }) }) From 1c1d17732166635cf2fdfc24dff58a9fc2af81c7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Mar 2026 20:57:32 +0000 Subject: [PATCH 47/49] Fix: Correct doc issues in error-handling.md and errors.js plugin Agent-Logs-Url: https://github.com/adapt-security/adapt-authoring-core/sessions/772703ac-791f-4bfa-9beb-854b2c913014 Co-authored-by: taylortom <1059083+taylortom@users.noreply.github.com> --- docs/error-handling.md | 7 +++---- docs/plugins/errors.js | 9 +++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/error-handling.md b/docs/error-handling.md index e7c54974..19957be2 100644 --- a/docs/error-handling.md +++ b/docs/error-handling.md @@ -8,7 +8,6 @@ Before going into specifics, it would be useful to discuss application errors in You will need to deal with each category of error differently. Below are some general tips on handling each type of error. -## Initialisation errors ## Initialisation errors Any errors which occur during initialisation should be captured and logged as appropriate. Depending on the type of error, it may or may not be considered fatal to your code. @@ -23,7 +22,7 @@ Some examples: Some examples: - For a database-handler module, disconnecting from the database is an expected error, and can be handled and rectified easily. -## User errors errors +## User errors User errors are any errors which are caused as a direct result of a user performing an action incorrectly. It is even *more* critical with user errors that the error is as specific and descriptive as possible, as the response needs to be informative and instructive to the user that caused the error. Failing to do so will result in an unpleasant user experience. Some examples: @@ -32,14 +31,14 @@ Some examples: ## Defining errors Depending on the kinds of error that you're dealing with in your code, it may be useful to include a set of custom error definitions specific to your code. -Defining useful errors is a critical part of any software system. The ErrorsModule makes it easy to define errors for your own modules, and make use of errors defined in other modules. +Defining useful errors is a critical part of any software system. The Errors library makes it easy to define errors for your own modules, and make use of errors defined in other modules. ## Catching errors ## Throwing errors As mentioned above, it is preferable to catch errors internally in your code and re-throw these errors -The ErrorsModule acts as a central store for all errors defined in the system, and errors can be accessed and thrown from here. For convenience, the Errors module is made available directly as a property of the main App instance, or by referencing the module in the usual way: `App#waitForModule('errors)` +The Errors library acts as a central store for all errors defined in the system, and errors can be accessed and thrown from here. For convenience, the Errors library is made available directly as a property of the main App instance via `app.errors`. ```js try { diff --git a/docs/plugins/errors.js b/docs/plugins/errors.js index 3ef6efc2..8c32d346 100644 --- a/docs/plugins/errors.js +++ b/docs/plugins/errors.js @@ -12,10 +12,11 @@ export default class Errors { }, '| Error code | Description | HTTP status code | Supplemental data |\n| - | - | :-: | - |') } - dataToMd (data, s = '') { - if (!data) return s + dataToMd (data) { + if (!data) return '' return Object.entries(data).reduce((s, [k, v]) => { - return `${s}
  • \`${k}\`: ${typeof v === 'object' ? this.dataToMd(v, s) : v}
  • ` - }, s) + const nested = typeof v === 'object' ? this.dataToMd(v) : v + return `${s}
  • \`${k}\`: ${nested}
  • ` + }, '') } } From 60659e6f1dfe75197176023b9316e508448c93c2 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Sat, 28 Mar 2026 15:41:05 +0000 Subject: [PATCH 48/49] Default NODE_ENV to production in App constructor Moves the NODE_ENV default from bin/start.js into core so all consumers get it automatically. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/App.js b/lib/App.js index ba740d16..6121a170 100644 --- a/lib/App.js +++ b/lib/App.js @@ -31,6 +31,7 @@ class App extends AbstractModule { /** @override */ constructor () { + process.env.NODE_ENV ??= 'production' const rootDir = process.env.ROOT_DIR ?? process.cwd() const adaptJson = JSON.parse(fs.readFileSync(path.join(rootDir, metadataFileName))) const packageJson = JSON.parse(fs.readFileSync(path.join(rootDir, packageFileName))) From decd8099f08aacce6838a985f55517bf215dee55 Mon Sep 17 00:00:00 2001 From: Thomas Taylor Date: Sat, 28 Mar 2026 17:45:51 +0000 Subject: [PATCH 49/49] Handle fatal startup errors with setReady before exit Calls setReady with the error before process.exit(1) so that onReady listeners (e.g. tests) receive the rejection before the process dies. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/App.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/App.js b/lib/App.js index 6121a170..f1d6711a 100644 --- a/lib/App.js +++ b/lib/App.js @@ -105,8 +105,8 @@ class App extends AbstractModule { this.log('verbose', 'DIR', 'dataDir', this.getConfig('dataDir')) this.log('verbose', 'DIR', 'tempDir', this.getConfig('tempDir')) } catch (cause) { - process.exitCode = 1 - throw new Error('Failed to start App', { cause }) + await this.setReady(new Error('Failed to start App', { cause })) + process.exit(1) } const failedMods = this.dependencyloader.failedModules if (failedMods.length) this.log('warn', `${failedMods.length} module${failedMods.length === 1 ? '' : 's'} failed to load: ${failedMods}. See above for details`)