fix(config): catch failing config/*.cfm includes at boot instead of masked 500 - #3070
Conversation
…asked 500 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
…mpatibility Top-level `var di = injector();` in config/services.cfm is a compile error on Adobe CF (the file is included inside $includeConfig(), a function context). Change all config/services.cfm code-block examples across guides and CLAUDE.md to use `local.di = injector(); local.di.map(...)` — valid on all engines. Refs #3063 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Docs updatedAdded a doc commit to this PR:
|
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR wraps $includeConfig()'s savecontent/include in a try/catch so a config/*.cfm file that fails to compile or run at boot is logged by name and skipped, instead of cascading out of onApplicationStart into the masked app-wide 500 from #3063. The implementation matches the issue's proposed direction exactly, the new spec (vendor/wheels/tests/specs/global/includeConfigSpec.cfc) follows established prior art (reloadGlobalsSpec.cfc's _tmp fixture + var ctx shared-struct patterns), the changelog fragment and commit message conform, and CI is green. I found no correctness, cross-engine, or security issues — verdict: comment (two non-blocking nits below).
Things I explicitly verified during review:
- No fail-open security regression from skipping
settings.cfm. An emptyreloadPassworddisables URL-based reload and environment switching rather than allowing them (vendor/wheels/events/onapplicationstart.cfc:371-376), so a swallowedsettings.cfmfailure fails closed on that surface. - Log-and-continue is the issue-sanctioned design, not a unilateral trade-off: #3063's acceptance criteria are "a log line naming the file, not an opaque app-wide 500", and the log text at
vendor/wheels/Global.cfc:254interpolatesarguments.template, satisfying that. - Catch body avoids the documented cross-engine traps: no
local.X-in-catchpersistence reads (invariant 11), no loops infinally(invariant 12), no bare tag-in-script statements (invariant 13), engine-uniform constructs only. The inner best-efforttry/catcharoundwriteLogmirrors the existing output-leak branch atGlobal.cfc:268-282. - Spec fixture paths are deliberately all-lowercase to survive
LCase()+ case-sensitive filesystems, and$assert.notThrowshas prior art in four other specs.
Docs
-
Stale docblock on
$includeConfig—vendor/wheels/Global.cfc:210-232. The docblock still describes only the output-capture behavior, and line 223 narrows the no-throw guarantee to output:The function never throws on output; missing-wrapper mistakes are recoverable…
The function's contract is now broader: it never throws on a template compile/runtime failure either. The rationale lives only in the inline catch comment (lines 241-249), which a caller reading the docblock won't see. Suggest adding one sentence to the docblock, e.g. "The function also never throws on a template compile/runtime failure — the failure is logged with the file name and the file is skipped (issue #3063)."
-
Behavior change worth a line in the user-facing guide: any exception a config file throws at boot — including an intentional fail-fast guard like
throw("missing API key")inconfig/settings.cfm— is now downgraded to a log entry and the app boots without that file's registrations. The PR delegates doc updates tobot-update-docs.ymlfor the DI snippet; please make sure that pass (or a follow-up) also mentions that config-file errors are non-fatal at boot as of this change, since the changelog fragment is the only user-facing record of it right now.
Tests
- The targeted engine wasn't exercised by this PR's CI. The checks that ran are Lucee 7 + SQLite plus Lucee 7 smoke; the bug is an Adobe-CF-specific compile failure, and the spec substitutes a portable runtime throw (a reasonable stand-in, clearly justified in the spec comments). Standard CFML semantics say an include-site compile error is catchable at the
includestatement, so I expect this to hold — but since CLAUDE.md asks for Adobe verification of Adobe-targeted fixes before relying on CI, a one-offtools/test-matrix.sh adobe2023 mysqlrun (or replaying the originalvar di = injector();repro from #3063 against adobe2023) before merge would close the loop. Non-blocking.
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: Re-review at head f0198fe66, which adds one commit on top of the previously reviewed framework fix (b564dce8b, reviewed with state comment — that review's analysis of the $includeConfig() try/catch and includeConfigSpec.cfc still stands and is not repeated here). The new commit switches every config/services.cfm snippet in the guides and CLAUDE.md from var di = injector(); to local.di = injector();. The substitution is correct and the right fix for the documented Adobe CF cliff: config files are included inside $includeConfig() (a function context), where local. assignment is valid on Lucee, Adobe, and BoxLang, while top-level var is an Adobe compile error (#3063). The guides' {test:compile} verification passed (Deploy guides green), commitlint and DCO pass for both commits, and I found no correctness, cross-engine, or security issues. Verdict: comment — the only findings are non-blocking docs nits, including two carried forward unaddressed from the previous review.
What I verified on the delta:
- Scope correctness of
local.di—vendor/wheels/Global.cfc:237includes the template inside the$includeConfigfunction body, solocalresolves to the function-local scope on every engine; the snippet change cannot collide with the function's own locals (local.$wheelsConfigOutput,local.preview, …). The tutorial variant (06-authentication.mdx) keeps its<cfscript>wrapper, and the surrounding prose that quoted the old snippet (the leak description and the "service not found" troubleshooting item) was updated consistently — not just the code blocks. - All five touched guide pages + CLAUDE.md now show the same canonical shape; no mixed old/new snippets within a page.
- Commit message (
docs: use local.di in config/services.cfm DI snippets for Adobe CF compatibility) conforms tocommitlint.config.js(valid type, header ≤ 100 chars, body explains the why) and CI's "Validate Commit Messages" passed.
Docs
-
Published blog post still teaches the Adobe-breaking snippet —
web/content/blog/posts/from-wirebox-to-wheelsdi.md:49-59has a code block explicitly labeled// config/services.cfm — in-house DI, familiar surfacestarting withvar di = injector();(same in its source skeleton,docs/releases/blog-skeletons/08-wirebox-to-wheelsdi.md:55). The commit deliberately scoped itself to "guides and CLAUDE.md", but the blog ships to users too, and a reader copying that block intoconfig/services.cfmon Adobe gets exactly the #3063 failure (now downgraded to a logged skip, but their registrations still silently don't run). Suggest updating it tolocal.diin this PR or a follow-up. Relatedly trivial: the missing-wrapper warning text atvendor/wheels/Global.cfc:276still usesvar di = injector();as its example of leaked text, while the updated tutorial now tells users they'd seelocal.di = injector();leak — harmless, but worth aligning if you touch that string again. -
Carried forward (previous review, unaddressed): stale
$includeConfigdocblock —vendor/wheels/Global.cfc:223-225still says "The function never throws on output; missing-wrapper mistakes are recoverable…", describing only the output-capture contract. Sinceb564dce8bthe contract is broader: a template compile/runtime failure is also caught, logged, and skipped. One added sentence in the docblock would save a future maintainer from having to find the rationale in the inline catch comment. -
Carried forward (previous review, unaddressed): no user-facing record that config-file errors are non-fatal at boot — this commit fixes the snippets but adds no guide text noting that, as of this change, an exception thrown by a
config/*.cfmfile at boot (including an intentional fail-fastthrow) results in a logged skip rather than a failed start. The changelog fragment remains the only user-facing record. A sentence oncore-concepts/dependency-injection.mdx(or the configuration page) would close this. -
PR body checklist is now stale — the Feature Completeness Checklist still shows Framework Docs / AI Reference Docs / CLAUDE.md as unchecked, "handled by
bot-update-docs.yml", butf0198fe66performs the guides + CLAUDE.md updates in this PR. Worth ticking those boxes so the record matches the diff.
…nfigIncludeFailed (fail-closed) (#3135) Reworks the #3070 shape per maintainer decision on #3063: $includeConfig() no longer logs-and-continues when a config template fails to compile or run. It still logs to the wheels log, then rethrows a named Wheels.ConfigIncludeFailed error carrying the failing template path and the original engine message (original type/detail preserved in detail). The throw is unconditional — no environment branching, no swallowed path. A production app with a broken config/settings.cfm previously booted fail-open on framework defaults and served traffic; now application start aborts with a clear, located, Rails-style boot error. Composes with the #3108 onError unmasking so the named error renders on the development error page. Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
Hardens config-file loading so a
config/*.cfmtemplate that fails to compile or run during application start no longer takes the whole app down at boot.vendor/wheels/Global.cfc::$includeConfig()now wraps thesavecontent { include … }in atry/catch: on failure it logs an error naming the offending template and continues, instead of letting the exception cascade out ofonApplicationStartinto a masked, application-wide HTTP 500 whose secondaryonErrorfailure (Element WHEELS.ENGINEADAPTER is undefined …) hid the real cause.The canonical trigger is the documented DI snippet
var di = injector();inconfig/services.cfm, which is a compile error on Adobe CF (top-levelvarin an included template) while Lucee accepts it — so the documented pattern booted on Lucee and bricked every request on Adobe. This change makes that failure debuggable; the docs snippet itself (changingvar→local./unscoped in the dependency-injection guide and the CLAUDE.md DI block) is the second half of the issue and is handled separately bybot-update-docs.yml, which is why this usesRefsrather thanFixes.Refs #3063
Related Issue
Refs #3063
Type of Change
Feature Completeness Checklist
Signed-off-by:vendor/wheels/tests/specs/global/includeConfigSpec.cfc(failing → passing)bot-update-docs.yml(MDX dependency-injection guide snippet)bot-update-docs.ymlbot-update-docs.yml(DI Container Quick Reference snippet)changelog.d/3063-includeconfig-compile-guard.fixed.mdTest Plan
New spec
includeConfigSpec.cfc(3 cases): a config template that fails at runtime is caught (does not propagate), a healthy template still executes its body after the guard, and a later config include still runs after an earlier one fails. A runtime throw (undefined-variable reference) stands in for Adobe's engine-specific compile error so the regression is portable to the Lucee CI runner.does not propagate a failure thrown by a config templateandrecovers and keeps loading later config after one file failsboth failed — the exception propagated out of$includeConfig.globalspec layer (wheels.tests.specs.global) — 4 passed, 0 failed, 0 errors.provides sets controller class dataformat-list pollution;should fallback to template for ajax request with no layout specified); both were present before this change and are outside config loading.Cross-engine note: the
try/catchis uniform across Lucee/Adobe/BoxLang (no engine-specific branches), and the catch body avoidslocal.X-in-catch persistence and other CLAUDE.md cross-engine traps.Screenshots / Output
n/a