fix(config): report why the config module failed, not only which file - #3657
Conversation
A reader following the CSS-optimizer hint into a `veryfront.config.ts` writes
the natural first guess:
import { defineConfig } from "veryfront/config";
`veryfront` exports no `./config` subpath -- `defineConfig` is on the package
root -- so Node raises ERR_PACKAGE_PATH_NOT_EXPORTED with a message that says
exactly what is wrong: "Package subpath './config' is not defined by "exports"
in .../veryfront/package.json". Verified against published 0.1.1232, the loader
drops that message and prints:
! Failed to load config file configFile=veryfront.config.ts
✗ [config-parse-error] Failed to parse configuration
Detail: Failed to load veryfront.config.ts
Suggestion: Ensure your configuration file contains valid JavaScript or
TypeScript
The file's syntax was never the problem, so the suggestion sends the reader
looking in the wrong place, and nothing recovers the discarded reason -- `cause`
is attached to the error but nothing between the loader and the terminal reads
it, at any log level, including LOG_LEVEL=debug.
So the four `Failed to load <file>` sites now append a summary of the cause.
It is one line, bounded to 200 characters, and stripped of control characters:
the cause is authored by the project being loaded, and a hosted build log must
not become a paste surface for it.
The unresolvable specifier in the test is `veryfront/not-an-export`, not the
`veryfront/config` from the field report, because this repository's own
deno.json maps `veryfront/config` to src/config/index.ts for internal callers,
so it resolves inside the test process. That asymmetry -- a subpath that is real
in the monorepo and absent from the published exports map -- is why the guess is
natural in the first place.
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fd5b154bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… generic Two review findings on the cause-reporting change. The summary truncated before redacting. `sanitizeUrlCredentials` recognizes userinfo by the trailing `@host`, so cutting to 200 characters first can drop that marker and leave a password prefix in a CONFIG_PARSE_ERROR detail -- and the HTTP boundary strips `detail` only at 5xx, so a 400 carries it to the caller. Redaction now runs over the complete message, the order `sanitizeBoundedDiagnosticText` already documents for the same reason. `readHostedConfigSource` goes back to the generic detail. Everything reaching that catch came out of `adapter.fs.readFile`, so the cause describes the storage backend, not the project's config module. It stays on `cause` for the logs. The new test pins the ordering rather than the presence: padding places the `@` past the bound, so an unredacted cut keeps nine characters of the secret. It fails with redaction moved after the bound. Also casts the three new `assertRejects` results to `VeryfrontError`, matching the rest of the file -- without it `lint:test-typecheck` reported twelve TS18046 errors and its recursive split surfaced five unrelated files as collateral.
1 participant
Second defect found while following #3597's CSS-optimizer hint literally in a scratch project built from published
veryfront@0.1.1232. (The hint itself is fixed in a separate PR.)Reproduction
A reader told to put the extension in
veryfront.config.tswrites the natural first guess:veryfrontexports no./configsubpath —defineConfigis on the package root — so Node raisesERR_PACKAGE_PATH_NOT_EXPORTEDwith a message that says exactly what is wrong:The loader drops it:
The file's syntax was never the problem, so the suggestion sends the reader looking in the wrong place.
causeis attached to the error, but nothing between the loader and the terminal reads it — including atLOG_LEVEL=debug, which adds onlyloadAndMergeConfig calledand then the same bare line.Change
The three
Failed to load <file>sites insrc/config/loader.tsthat wrap evaluation of the project's config module append a summary of the cause. The summary is redacted withsanitizeUrlCredentials, then reduced to one line, bounded to 200 characters, and stripped of control characters: the cause is authored by the project being loaded, and a hosted build log must not become a paste surface for it. Redaction runs before the bound becausesanitizeUrlCredentialsrecognizes userinfo by its trailing@host, and a cut that drops the@hostwould leave a password prefix behind -- the orderingsanitizeBoundedDiagnosticTextalready documents atsrc/errors/diagnostic-policy.ts:63-72.The fourth site, in
readHostedConfigSource, keeps the generic detail. Everything reaching that catch came out ofadapter.fs.readFile, so the cause describes the storage backend rather than the project's config module, andCONFIG_PARSE_ERRORis a 400 whosedetailthe HTTP boundary does not strip (src/errors/middleware/http-error-boundary.ts:113-116). The cause stays attached for the logs.Tests
Red first, three new cases in
src/config/loader.test.ts:The third pins the bound: a cause with
"first line\nsecond line " + "A".repeat(4096)must keep the first line, drop the rest, and leave the report under 512 characters.A fourth case, added after review, pins the redaction order rather than the presence of a call. Its padding is sized so
https://svc:ends at character 190 and the@sits at 209, putting the marker the redactor matches on past the bound. WithsanitizeUrlCredentialsmoved back after the bound it fails:The unresolvable specifier in the test is
veryfront/not-an-export, not theveryfront/configfrom the field report, because this repository's owndeno.jsonmapsveryfront/configtosrc/config/index.tsfor internal callers, so it resolves inside the test process. That asymmetry — a subpath that is real in the monorepo and absent from the published exports map — is why the guess is natural in the first place, and it is called out in the test comment.End-to-end verification
The published 0.1.1232 npm package with only
esm/src/config/replaced by this branch'sdeno task build:npmoutput, same scratch project, same wrong import:Not changed
The
config-parse-errorsuggestion still reads "Ensure your configuration file contains valid JavaScript or TypeScript", which remains wrong for a resolution failure. Narrowing the suggestion per cause class is a larger change to the error registry and is left out of this fix.🤖 Generated with Claude Code