Skip to content

Commit 6c94972

Browse files
DavertMikclaude
andcommitted
refactor(config): for-of hook loops, swallow per-hook errors via applyHook
Three changes asked for on the previous hook refactor: - Replace the for-i loops in Config.create and Config.runPendingHooks with for-of. Both rely on Array iterator semantics — newly pushed hooks (e.g. a hook that registers another hook) are still picked up in the same pass because the iterator re-checks length on each step. - Centralize hook execution in a single applyHook() helper that wraps the call in try/finally. A broken hook now logs through globalThis.codeceptjs.output (or stderr when the runner isn't up yet) and is marked ran=true regardless, so a poison entry can't block the rest of the pass or get retried by runPendingHooks. Callers don't touch hook.ran themselves. - Tighten the comment over the globalThis.codeceptjs block in lib/codecept.js. The previous wording implied end-user projects didn't need it — they do, because @codeceptjs/configure is typically imported at the top of a user's codecept.conf.js and registers hooks via the in-process registry before Config.load even returns. setup.mjs handles the same contract for mocha tests via .mocharc require. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ee59b2d commit 6c94972

2 files changed

Lines changed: 37 additions & 22 deletions

File tree

lib/codecept.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ import globalRetryListener from './listener/globalRetry.js'
3333
import exitListener from './listener/exit.js'
3434
import emptyRunListener from './listener/emptyRun.js'
3535

36-
// Register an in-process handle so companion packages (@codeceptjs/configure,
37-
// @codeceptjs/expect-helper, @codeceptjs/helper) can reach the running
38-
// framework without doing a top-level `import 'codeceptjs'` — that bare
39-
// specifier doesn't resolve in this repo's own checkout (the project IS the
40-
// codeceptjs package). End-user projects don't need this either way; npm
41-
// resolves `codeceptjs` from their node_modules and the bare import works.
36+
// Companion packages (@codeceptjs/configure, @codeceptjs/expect-helper,
37+
// @codeceptjs/helper) read framework singletons through globalThis.codeceptjs.
38+
// `@codeceptjs/configure` in particular runs at the top of a user's
39+
// codecept.conf.js, before `Config.load` returns — so the registry has to
40+
// exist by the time this module is imported, not later during init.
41+
// In tests we install the same handle from .mocharc → test/support/setup.mjs.
4242
if (!globalThis.codeceptjs) {
4343
globalThis.codeceptjs = { config: Config, container, event, output, recorder, Helper }
4444
}

lib/config.js

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,27 @@ const defaultConfig = {
3232
],
3333
}
3434

35-
// Array<{ fn: (cfg) => void, ran: boolean }>
35+
// Array<{ fn: (cfg) => void, ran: boolean, error?: Error }>
3636
let hooks = []
3737
let config = {}
3838

39+
// Apply a single hook against `cfg`, swallowing errors so one broken hook
40+
// can't take down the whole run. The failure is logged through the
41+
// framework's own output module (when available) so it shows up in test
42+
// reports; the hook is still marked ran so it doesn't get retried.
43+
function applyHook(hook, cfg) {
44+
try {
45+
hook.fn(cfg)
46+
} catch (err) {
47+
hook.error = err
48+
const out = globalThis.codeceptjs?.output
49+
if (out && typeof out.error === 'function') out.error(`config hook failed: ${err.message}`)
50+
else console.error('config hook failed:', err)
51+
} finally {
52+
hook.ran = true
53+
}
54+
}
55+
3956
const configFileNames = ['codecept.config.js', 'codecept.conf.js', 'codecept.js', 'codecept.config.cjs', 'codecept.conf.cjs', 'codecept.config.ts', 'codecept.conf.ts']
4057

4158
/**
@@ -50,12 +67,11 @@ class Config {
5067
*/
5168
static create(newConfig) {
5269
config = deepMerge(deepClone(defaultConfig), newConfig)
53-
// Re-apply every hook against the freshly built config and mark them ran;
54-
// hooks added later (e.g. from plugin boot) stay pending until runPendingHooks.
55-
for (let i = 0; i < hooks.length; i++) {
56-
hooks[i].fn(config)
57-
hooks[i].ran = true
58-
}
70+
// Re-apply every hook against the freshly built config; hooks added later
71+
// (e.g. from plugin boot) stay pending until runPendingHooks. Array
72+
// iterators re-check length on each step, so hooks pushed during a hook
73+
// execution are visited in this same pass.
74+
for (const hook of hooks) applyHook(hook, config)
5975
return config
6076
}
6177

@@ -132,23 +148,22 @@ class Config {
132148

133149
/**
134150
* Run every hook that hasn't been applied to the current config yet.
135-
* Hooks added after `Config.create()` (e.g. by a plugin's boot code) stay
151+
* Hooks added after `Config.create()` (e.g. from plugin boot code) stay
136152
* pending until this is called; once it runs, they're marked applied so
137-
* subsequent calls are no-ops.
153+
* subsequent calls are no-ops. Hooks added while pending hooks are running
154+
* are picked up in the same pass (the array iterator re-checks length).
138155
*
139-
* Hooks registered while pending hooks are running are picked up too
140-
* the loop re-checks length on each iteration.
156+
* Failures are logged through `output.error` and don't abort the loop
157+
* a broken hook can't poison the run, but its error is visible.
141158
*
142159
* @param {Object<string, *>} [cfg] target config (defaults to the live singleton)
143160
* @return {boolean} true if any hook ran
144161
*/
145162
static runPendingHooks(cfg = config) {
146163
let ran = false
147-
for (let i = 0; i < hooks.length; i++) {
148-
const h = hooks[i]
149-
if (h.ran) continue
150-
h.fn(cfg)
151-
h.ran = true
164+
for (const hook of hooks) {
165+
if (hook.ran) continue
166+
applyHook(hook, cfg)
152167
ran = true
153168
}
154169
return ran

0 commit comments

Comments
 (0)