fix(config): fail closed with named Wheels.ConfigIncludeFailed when a config include fails at boot - #3135
Conversation
…nfigIncludeFailed (fail-closed) 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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR reworks #3070's log-and-continue into the maintainer-decided fail-closed contract from #3063: $includeConfig() logs the failing config/*.cfm template, then rethrows a named Wheels.ConfigIncludeFailed error carrying the template path and original engine message, with the original type/detail preserved in detail. The implementation is correct, cross-engine-safe, well-tested with a documented RED/GREEN cycle, and conventions-compliant. Verdict: approve.
What I verified (no findings — evidence below)
Correctness
- The throw is unconditional with no environment branching (
vendor/wheels/Global.cfc:267-275), and logging stays best-effort inside its own try/catch so a logging failure can't mask the rethrow (Global.cfc:256-266). - All four call sites are bare —
vendor/wheels/events/onapplicationstart.cfc:325,:327,:381,:385. I checked every try/catch in that file (lines 106, 194, 218, 226, 373): all are localized blocks that close before the includes, andpublic/Application.cfc:107(onApplicationStart) has no try/catch — so the named error genuinely propagates out of application start, which is the point of the design. - The output-leak warning branch (missing-cfscript-wrapper detection,
Global.cfc:277-296) is unchanged and unreachable on the throw path, as documented. e.detailis guarded withStructKeyExists(e, "detail") && Len(e.detail)(Global.cfc:271) before concatenation.
Cross-engine
- The catch body assigns no
local.X(invariant 11 — BoxLang catch-scope discard); it only callswriteLogandThrow, both with all-named arguments. - The spec's type/message assertions capture catch state into a shared struct (
includeConfigSpec.cfc:72-80), correctly applying invariant 11. - The spec's
toInclude("undefinedConfigVarXyz")assertion (includeConfigSpec.cfc:87) is safe against Adobe's uppercased variable names in engine messages:vendor/wheels/wheelstest/system/Expectation.cfc:579showstoIncludedelegates to the case-insensitiveassert.includes(the case-sensitive variant istoIncludeWithCase). Every supported engine names the undefined variable in its message. - No bare
cfabort, no bracket-notation calls, no inline-closure constructor args, noLeft(str, 0)exposure.
Tests
- Four BDD specs extending
wheels.WheelsTest(not RocketUnit) covering: named-type rethrow, message/detail content (#3063 acceptance), happy path unchanged, and no cross-call state poisoning. Error path AND happy path both exercised. The PR documents a genuine RED/GREEN cycle (1 pass / 3 fail against #3070's code → 4/0 against this change) plus a live cold-boot proof, and honestly discloses the 12 pre-existingtestClientSpecfailures reproducing on unmodified develop.
Docs
- Changelog fragment
changelog.d/3063-includeconfig-compile-guard.fixed.mdupdated in place — correct use of the fragment system, no directCHANGELOG.mdedit. No stale references to the old skip behavior remain under.ai/(grep for "was skipped" /includeConfigcame back clean). - The docblock's claim that the error "renders on the development error page now that onError no longer masks application-start errors" is accurate: #3108 (
5af724852) is merged.
Commits
- Single commit
fix(config): rethrow failing config/*.cfm includes as named Wheels.ConfigIncludeFailed (fail-closed)— valid type/scope, header exactly 100 chars (within the ≤100 limit), not ALL-CAPS, body explains the why (fail-open production boot risk), DCO sign-off present.
Security
- Fail-closed is the security-positive direction here: a production app with broken config no longer boots on framework defaults (potentially default security posture) and serves traffic. The enriched error detail surfaces on the development error page; production requests still get the generic error path via
onError.
One observation, not a finding: only Lucee 7 + SQLite was exercised locally. The catch-path code deliberately avoids the BoxLang/Adobe catch-scope pitfalls, and the compat matrix will exercise the rest — nothing in the diff pattern-matches a known cross-engine trap.
Summary
Reworks the just-merged #3070 per the maintainer's design decision on #3063: replace log-and-continue-booting with a named rethrow — fail-closed everywhere, Rails-style located error.
#3070's shape caught
config/*.cfminclude failures and continued boot in all environments. Review found the problem with that: a production app with a brokenconfig/settings.cfmwould boot fail-open on framework defaults and serve traffic (no DI registrations, default settings, possibly default security posture). The maintainer chose instead: catch → log → rethrow a namedWheels.ConfigIncludeFailederror carrying the failing template path + the original engine message (original type/detail preserved indetail). Fail-closed in every environment, no environment branching, no swallowed path. This composes with the #3108 unmasking (onError no longer hides app-start errors) so the named error actually renders on the development error page — step one toward Rails-style "what broke, where, why" boot errors.$includeConfig()still logs the failure to thewheelslog before rethrowing, and the output-leak warning branch (missing-cfscript-wrapper detection) is unchanged. All 4 call sites (/config/settings.cfm,/config/<env>/settings.cfm,/config/services.cfm,/config/<env>/services.cfminonapplicationstart.cfc) are bare, so the throw propagates out of application start — which is the point.Fixes #3063
Type of Change
Feature Completeness Checklist
Signed-off-by:vendor/wheels/tests/specs/global/includeConfigSpec.cfcreworked (RED → GREEN, see Test Plan)changelog.d/3063-includeconfig-compile-guard.fixed.mdreworded to the rethrow contractbot-update-docs.yml(per fix(config): catch failing config/*.cfm includes at boot instead of masked 500 #3070's split)Test Plan
Lucee 7 + SQLite Docker harness (dir-only worktree mount, CI-equivalent image).
Spec-level RED/GREEN (
wheels.tests.specs.global.includeConfigSpec):var di = injector();) crashes the entire app at boot on Adobe CF #3063 acceptance — a clear error, not the old masked one), that the original type survives intodetail, and that the happy path still runs. The old "boot continues after a failure" spec is inverted to the throw contract.Live cold-boot proof (deliberate
syntax error here;;appended toconfig/settings.cfm):wheels.log:Wheels: /config/settings.cfm failed to compile or run during onApplicationStart — application start was aborted (fail-closed). Error: Missing [;] or [line feed] after expression; Failed at [/wheels-test-suite/config/settings.cfm:33]— a genuine engine compile error caught and named.$engineAdapter()hunks applied locally:Lucee 7.0.1.100 Error (Wheels.ConfigIncludeFailed)/ Message:Failed to include config template '/config/settings.cfm': Missing [;] or [line feed] after expression; Failed at [...settings.cfm:33]/ Detail:Original exception type: template. Application start was aborted because this config file could not be loaded — fix the file and restart...— exactly the located error Documented config/services.cfm snippet (var di = injector();) crashes the entire app at boot on Adobe CF #3063 asked for.key [ENGINEADAPTER] doesn't existsecondary failure inonError— which is precisely what fix(config): onError no longer masks app-start exceptions in dev #3108 fixes. The two PRs compose; neither depends on the other to be correct, only to be pretty.Full core suite (Lucee 7 + SQLite): 4414 pass / 12 fail / 0 error. All 12 failures are in
wheels.tests.specs.internal.testClientSpec(HTTP self-request specs) and reproduce identically on unmodified develop in the same harness — pre-existing/environmental, zero new failures from this change.Cross-engine notes: the catch body only throws/logs (no
local.X-persistence dependence, invariant #11);e.detailread is guarded withStructKeyExistsfor engine portability; the spec captures catch state into a shared struct per invariant #11.Screenshots / Output
n/a (error-page text quoted above)
🤖 Generated with Claude Code