Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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

Large diffs are not rendered by default.

127 changes: 127 additions & 0 deletions src/html/hydration-script-builder/runtime/renderer-modules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,133 @@ describe("hydration-script-builder/runtime/renderer", () => {
]);
});

it("does not probe /index.js when <route>.js reached a module and threw at evaluation", async () => {
// Issue #3667: an extension-style route (pages/vector-a.tsx, no folder) was
// requesting <route>/index.js and 404ing. That only fires as the retry —
// here <route>.js loads and throws a *runtime* error during evaluation
// (the class surfaced by the #3661 O_NOFOLLOW adapter crash). The module
// exists and is the served file, so the /index.js probe can only 404 and
// add noise; the real error must surface untouched.
const requested: string[] = [];
const evaluationError = new TypeError(
"Cannot read properties of undefined (reading 'O_NOFOLLOW')",
);

const thrown = await captureRejection(
loadPageModuleWithIndexFallback(
"http://modules/pages/vector-a",
"vector-a",
null,
(url) => {
requested.push(url);
return Promise.reject(
url.endsWith("/vector-a.js") ? evaluationError : notFound(url),
);
},
),
);

assertEquals(thrown, evaluationError);
assertEquals(requested, ["http://modules/pages/vector-a.js"]);
});

it('retries at /index.js when Safari reports only "Load failed"', async () => {
// Safari surfaces a failed dynamic import as a TypeError reading exactly
// "Load failed", naming no module. That is the same shape as the
// evaluation error pinned above, so the retry cannot be decided on the
// error's type — only its wording distinguishes them. Without this the
// <route>/index.js fallback never fired in Safari and the page stayed
// blank for every <route>/index.tsx route.
const requested: string[] = [];
const pageModule: ModuleNamespace = { default: "docs-index" };

const loaded = await loadPageModuleWithIndexFallback(
"http://modules/pages/docs",
"docs",
null,
(url) => {
requested.push(url);
if (url.endsWith("/docs.js")) {
return Promise.reject(new TypeError("Load failed"));
}
return Promise.resolve(pageModule);
},
);

assertEquals(loaded, pageModule);
assertEquals(requested.length, 2);
});

it("refuses the retry for a module that throws a non-Error value", async () => {
// Only module code produces a non-Error rejection; the loader's own
// failures are TypeError or SyntaxError. Probing /index.js here can only
// 404 and bury what the module threw.
const requested: string[] = [];
const thrown = await captureRejection(
loadPageModuleWithIndexFallback(
"http://modules/pages/vector-a",
"vector-a",
null,
(url) => {
requested.push(url);
return Promise.reject(url.endsWith("/vector-a.js") ? "boom" : notFound(url));
},
),
);

assertEquals(thrown, "boom");
assertEquals(requested, ["http://modules/pages/vector-a.js"]);
});

it('refuses the retry for a trailing-period "Load failed."', async () => {
// Safari's message has no period. Allowing one widened the match past the
// engine wording for no gain, and an application error is free to end in
// a full stop.
const requested: string[] = [];
const evaluationError = new TypeError("Load failed.");

const thrown = await captureRejection(
loadPageModuleWithIndexFallback(
"http://modules/pages/vector-a",
"vector-a",
null,
(url) => {
requested.push(url);
return Promise.reject(
url.endsWith("/vector-a.js") ? evaluationError : notFound(url),
);
},
),
);

assertEquals(thrown, evaluationError);
assertEquals(requested, ["http://modules/pages/vector-a.js"]);
});

it("still refuses the retry for an evaluation TypeError that merely mentions loading", async () => {
// The Safari match is exact, so an application error that happens to
// contain the words does not reopen the noise #3667 removed.
const requested: string[] = [];
const evaluationError = new TypeError("Image load failed for /hero.png");

const thrown = await captureRejection(
loadPageModuleWithIndexFallback(
"http://modules/pages/vector-a",
"vector-a",
null,
(url) => {
requested.push(url);
return Promise.reject(
url.endsWith("/vector-a.js") ? evaluationError : notFound(url),
);
},
),
);

assertEquals(thrown, evaluationError);
assertEquals(requested, ["http://modules/pages/vector-a.js"]);
});

it("retries at /index.js for rejections the classifier does not recognize", async () => {
// A proxy that rewrites a module miss into an HTML shell surfaces as a
// SyntaxError. Gating the retry on error wording turned that into a blank
Expand Down
42 changes: 39 additions & 3 deletions src/html/hydration-script-builder/runtime/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ export function isModuleNotFoundError(error: unknown): boolean {
if (!error) return false;
if (error instanceof SyntaxError) return false;
const message = String((error as Error).message || error);
// Safari reports a failed dynamic import as a bare "Load failed", with no
// wording that names modules at all. Matched exactly, with no trailing
// punctuation allowed, rather than as a
// substring: the phrase is short enough that a loose match would swallow an
// application error like "Image load failed" and retry a module that had in
// fact evaluated. The other engines name the module in the message.
if (/^load failed$/i.test(message.trim())) return true;
return /(?:dynamically imported module|Importing a module script failed|Failed to load module script)/i
.test(message);
}
Expand All @@ -52,12 +59,37 @@ export function preferReachedModuleError(earlier: unknown, later: unknown): unkn
return earlier;
}

/**
* True when `error` proves `<route>.js` loaded as a module and then threw while
* *evaluating* — a runtime error from the module's own code.
*
* A `SyntaxError` means the module never linked (a missing export, or an HTML
* shell a proxy returned for a miss) and a fetch failure (see
* {@link isModuleNotFoundError}) means it was never served — both are cases the
* `<route>/index.js` retry exists to recover. Anything else is code that ran
* inside a module that *did* load, which proves `<route>.js` is the real served
* file: retrying a sibling `<route>/index.js` can only 404.
*/
function isReachedModuleEvaluationError(error: unknown): boolean {
// A rejection that is not an `Error` can only have come from module code that
// ran and threw it: the loader's own failures reject with `TypeError` or
// `SyntaxError`. Treating a thrown string or `null` as "not reached" sent the
// loader after `<route>/index.js`, which can only 404 and bury the throw.
if (!(error instanceof Error)) return true;
if (error instanceof SyntaxError) return false;
return !isModuleNotFoundError(error);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/**
* Loads a Pages Router page, retrying at `<route>/index.js` because both
* pages/about.tsx and pages/about/index.tsx are valid sources for the same
* route. The retry is unconditional: gating it on the wording of the first
* rejection turned any unrecognized wording into a blank page. Error selection,
* not the retry, is what must stay precise.
* route. The retry stays unconditional for missing-module and proxy-shell
* rejections — gating it on the wording of those turned any unrecognized
* wording into a blank page. It is skipped only when the first attempt reached
* a module and threw at evaluation: `<route>.js` then exists, so probing
* `<route>/index.js` can only 404 and bury the real error under a misleading
* "module not found" (issue #3667, as surfaced by the #3661 adapter crash).
* Error selection, not the retry, is what must stay precise.
*/
export async function loadPageModuleWithIndexFallback(
basePath: string,
Expand All @@ -74,6 +106,10 @@ export async function loadPageModuleWithIndexFallback(
// ask for <route>/index/index.js.
if (pageSlug === "index" || pageSlug.endsWith("/index")) throw routeError;

// <route>.js loaded and threw at evaluation — it is the served file, so the
// retry is guaranteed useless. Surface the real error, not a sibling 404.
if (isReachedModuleEvaluationError(error)) throw routeError;
Comment thread
kojiwakayama marked this conversation as resolved.

try {
return await importModule(basePath + "/index.js");
} catch (indexError) {
Expand Down