diff --git a/src/config/loader.test.ts b/src/config/loader.test.ts index 24a9e05287..f5291341de 100644 --- a/src/config/loader.test.ts +++ b/src/config/loader.test.ts @@ -683,6 +683,170 @@ export default config as const; } }); + it("names why the config module failed, not just which file failed", async () => { + // Reproduced against published 0.1.1232 in a `veryfront init --template + // minimal` scaffold. A reader following the CSS-optimizer hint writes + // the natural first guess, `import { defineConfig } from + // "veryfront/config"` -- a subpath the package does not export. The + // build reports: + // + // ! 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 runtime said exactly what was wrong -- "Package subpath './config' + // is not defined by exports" -- and the loader dropped it, then advised + // checking syntax that was never the problem. `cause` is attached but + // nothing on the way to the terminal reads it, at any log level. + const adapter = setup(); + const projectDir = await Deno.makeTempDir({ + prefix: "vf-config-load-cause-", + }); + const configPath = `${projectDir}/veryfront.config.js`; + // Not the literal `veryfront/config` from the field report: this + // repository's own deno.json maps that specifier to src/config/index.ts + // for internal callers, so inside the test process it resolves. That + // asymmetry is why the guess is natural in the first place -- the + // subpath is real in the monorepo and absent from the package's exports + // map. A specifier no map claims reproduces the same class of failure + // and needs no network. + const source = 'import { defineConfig } from "veryfront/not-an-export";\n' + + "export default defineConfig({});\n"; + + try { + await Deno.writeTextFile(configPath, source); + adapter.fs.files.set(configPath, source); + + const error = await assertRejects( + () => getConfig(projectDir, adapter), + VeryfrontError, + ) as VeryfrontError; + + assert( + error.message.includes("veryfront.config.js"), + `error must still name the file, got: ${error.message}`, + ); + // Both runtimes name the subpath rather than the joined specifier: + // Deno says "Unknown export './not-an-export' for 'veryfront'", Node + // says "Package subpath './config' is not defined by exports". + assert( + error.message.includes("not-an-export"), + `error must name the subpath that failed to resolve, got: ${error.message}`, + ); + } finally { + await Deno.remove(projectDir, { recursive: true }); + } + }); + + it("carries a thrown config's own message through to the reader", async () => { + const adapter = setup(); + const projectDir = await Deno.makeTempDir({ + prefix: "vf-config-throw-cause-", + }); + const configPath = `${projectDir}/veryfront.config.js`; + const source = 'throw new Error("DATABASE_URL is required");\n'; + + try { + await Deno.writeTextFile(configPath, source); + adapter.fs.files.set(configPath, source); + + const error = await assertRejects( + () => getConfig(projectDir, adapter), + VeryfrontError, + ) as VeryfrontError; + + assert( + error.message.includes("DATABASE_URL is required"), + `error must repeat what the config threw, got: ${error.message}`, + ); + } finally { + await Deno.remove(projectDir, { recursive: true }); + } + }); + + it("bounds a hostile cause instead of pasting it into the report", async () => { + // The cause is authored by the project being loaded. A hosted build log + // must not become a paste surface for an arbitrarily long, arbitrarily + // formatted string, so the summary is one line and bounded. + const adapter = setup(); + const projectDir = await Deno.makeTempDir({ + prefix: "vf-config-cause-bound-", + }); + const configPath = `${projectDir}/veryfront.config.js`; + const noise = "A".repeat(4096); + const source = `throw new Error("first line\\nsecond line ${noise}");\n`; + + try { + await Deno.writeTextFile(configPath, source); + adapter.fs.files.set(configPath, source); + + const error = await assertRejects( + () => getConfig(projectDir, adapter), + VeryfrontError, + ) as VeryfrontError; + + assert( + error.message.includes("first line"), + `error must keep the first line, got: ${error.message}`, + ); + assert( + !error.message.includes("second line"), + `error must stop at the first line, got: ${error.message}`, + ); + assert( + error.message.length < 512, + `error must stay bounded, got ${error.message.length} characters`, + ); + } finally { + await Deno.remove(projectDir, { recursive: true }); + } + }); + + it("redacts a credential the bound would otherwise cut in half", async () => { + // Order matters, not just presence: the redactor recognizes userinfo by + // the trailing `@host`. Cutting the message to 200 characters first can + // drop that `@host` and leave the password prefix behind in a detail + // that a 400 response carries all the way to the caller, so redaction + // has to see the complete message. The padding is sized so the password + // straddles the 200-character bound: `https://svc:` ends at character + // 190 and the `@` sits at 209, so an unredacted cut keeps nine + // characters of the secret and loses the marker that identifies it. + const adapter = setup(); + const projectDir = await Deno.makeTempDir({ + prefix: "vf-config-cause-credential-", + }); + const configPath = `${projectDir}/veryfront.config.js`; + const padding = "B".repeat(160); + const password = "sup3rsecretpassword"; + const source = + `throw new Error("upstream refused ${padding} https://svc:${password}@registry.internal/pkg");\n`; + + try { + await Deno.writeTextFile(configPath, source); + adapter.fs.files.set(configPath, source); + + const error = await assertRejects( + () => getConfig(projectDir, adapter), + VeryfrontError, + ) as VeryfrontError; + + assert( + !error.message.includes(password), + `error must not carry the password, got: ${error.message}`, + ); + // A prefix of the secret is still the secret: pin the exact cut the + // 200-character bound produces when redaction runs too late. + assert( + !error.message.includes(password.slice(0, 9)), + `error must not carry a prefix of the password, got: ${error.message}`, + ); + } finally { + await Deno.remove(projectDir, { recursive: true }); + } + }); + it("bounds distinct concurrent loads and recovers capacity after they drain", async () => { const adapter = setup(); const gate = Promise.withResolvers(); diff --git a/src/config/loader.ts b/src/config/loader.ts index d5c7acf5ec..eeeea0d2b9 100644 --- a/src/config/loader.ts +++ b/src/config/loader.ts @@ -9,6 +9,7 @@ import { import { isBun, isDenoCompiled } from "#veryfront/platform/compat/runtime.ts"; import { ESBUILD_WASM_URL } from "#veryfront/platform/compat/esbuild-shared.ts"; import { serverLogger } from "#veryfront/utils/logger/logger.ts"; +import { sanitizeUrlCredentials } from "#veryfront/utils/logger/redact.ts"; import { getReactImportMap, REACT_DEFAULT_VERSION } from "#veryfront/utils/constants/cdn.ts"; import { DEFAULT_CACHE_DIR } from "#veryfront/utils/constants/server.ts"; import { buildConfigCacheKey, type VirtualConfigSourceContext } from "#veryfront/cache/keys.ts"; @@ -767,6 +768,13 @@ async function readHostedConfigSource( } if (isPreservedConfigLoadError(error)) throw error; logger.warn("Failed to load config file", { configFile }); + // Deliberately generic, unlike the three evaluation sites. Everything + // reaching here came out of `adapter.fs.readFile`, so the cause describes + // the storage backend -- internal hostnames, paths, account identifiers -- + // not the project's own config module. CONFIG_PARSE_ERROR is a 400, and + // the HTTP boundary strips `detail` only at 5xx + // (src/errors/middleware/http-error-boundary.ts:113-116), so a cause + // repeated here would reach the tenant. It stays on `cause` for the logs. throw CONFIG_PARSE_ERROR.create({ detail: `Failed to load ${configFile}`, cause: error, @@ -1488,6 +1496,59 @@ function isPreservedConfigLoadError(error: unknown): boolean { return error instanceof VeryfrontError; } +/** + * How much of a config module's own failure the report repeats. + * + * The cause is authored by the project being loaded, so a hosted build log must + * not become a paste surface for it. One line, bounded, control characters + * removed. + */ +const MAX_CONFIG_LOAD_CAUSE_CHARACTERS = 200; + +// deno-lint-ignore no-control-regex +const CONTROL_CHARACTERS = /[\u0000-\u001F\u007F-\u009F]/g; + +/** + * Return the one-line summary of `error`, or `undefined` when it has none. + * + * Redaction runs over the complete message before anything is cut away, the + * order `sanitizeBoundedDiagnosticText` documents: taking the first line or the + * first 200 characters can split `scheme://user:password@host` before the + * trailing `@host` the redactor matches on, which would leave the password + * prefix in a status-400 detail. + */ +function summarizeConfigLoadCause(error: unknown): string | undefined { + const message = error instanceof Error + ? error.message + : typeof error === "string" + ? error + : undefined; + if (message === undefined) return undefined; + const redacted = sanitizeUrlCredentials(message); + const firstLine = redacted.split("\n", 1)[0]?.replace(CONTROL_CHARACTERS, " ").trim() ?? ""; + if (firstLine.length === 0) return undefined; + return firstLine.length > MAX_CONFIG_LOAD_CAUSE_CHARACTERS + ? `${firstLine.slice(0, MAX_CONFIG_LOAD_CAUSE_CHARACTERS - 1)}…` + : firstLine; +} + +/** + * Report why the config module failed, not only which file did. + * + * `cause` is attached to the error, but nothing between here and the terminal + * reads it, at any log level. A reader whose config imports a subpath the + * package does not export got "Failed to load veryfront.config.ts" and a + * suggestion to check their syntax -- while the runtime had already said + * "Package subpath './config' is not defined by exports". Repeating that line + * is the difference between a build the reader can fix and one they cannot. + */ +function configLoadFailureDetail(configFile: string, error: unknown): string { + const summary = summarizeConfigLoadCause(error); + return summary === undefined + ? `Failed to load ${configFile}` + : `Failed to load ${configFile}: ${summary}`; +} + async function loadConfigFromTempFile( source: string, configPath: string, @@ -2145,7 +2206,7 @@ function getConfigInternal( if (isPreservedConfigLoadError(error)) throw error; logger.warn("Failed to load config file", { configFile }); throw CONFIG_PARSE_ERROR.create({ - detail: `Failed to load ${configFile}`, + detail: configLoadFailureDetail(configFile, error), cause: error, context: { configFile }, }); @@ -2215,7 +2276,7 @@ function getConfigInternal( if (isPreservedConfigLoadError(error)) throw error; logger.warn("Failed to load config file", { configFile }); throw CONFIG_PARSE_ERROR.create({ - detail: `Failed to load ${configFile}`, + detail: configLoadFailureDetail(configFile, error), cause: error, context: { configFile }, }); @@ -2345,7 +2406,7 @@ export async function evaluateHostedConfigSource( } if (isPreservedConfigLoadError(error)) throw error; throw CONFIG_PARSE_ERROR.create({ - detail: `Failed to load ${options.source.fileName}`, + detail: configLoadFailureDetail(options.source.fileName, error), cause: error, context: { configFile: options.source.fileName }, });