Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions src/config/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,127 @@ 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,
);

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,
);

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,
);

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("bounds distinct concurrent loads and recovers capacity after they drain", async () => {
const adapter = setup();
const gate = Promise.withResolvers<void>();
Expand Down
52 changes: 48 additions & 4 deletions src/config/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ async function readHostedConfigSource(
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),
Comment thread
kojiwakayama marked this conversation as resolved.
Outdated
cause: error,
context: { configFile },
});
Expand Down Expand Up @@ -1488,6 +1488,50 @@ 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. */
function summarizeConfigLoadCause(error: unknown): string | undefined {
const message = error instanceof Error
? error.message
: typeof error === "string"
? error
: undefined;
if (message === undefined) return undefined;
const firstLine = message.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;
Comment thread
kojiwakayama marked this conversation as resolved.
}

/**
* 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,
Expand Down Expand Up @@ -2145,7 +2189,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 },
});
Expand Down Expand Up @@ -2215,7 +2259,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 },
});
Expand Down Expand Up @@ -2345,7 +2389,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 },
});
Expand Down
Loading