Skip to content

Commit 05991db

Browse files
committed
fix: report a boundary-path failure once, and never over the root cause
Three defects in the reporting, plus the docs that described it wrongly. The report sat inside the error-boundary walk, so one throwing layout was reported once per boundary in the chain. The convergence argument written beside it was wrong: layoutsForBoundary selects by segment ancestry, not by boundary depth, so two boundaries can resolve to the same layout set and a root layout that throws fails every attempt. The loop still terminates, on its bounded index rather than on a shrinking set. Deduplicating by identity would not work either, since a layout that constructs its error yields a fresh object per attempt, so the 500 path carries one latch across the walk and the global-error attempt. The dev overlay keeps ONE retained frame per url and its slot is last-write wins, so pushing a secondary failure after the page error replaced the root cause with a symptom, and the replacement survived a reconnect. The 500 path no longer sends secondary failures to the overlay; the page error owns that frame. The 404 / 403 / 401 paths still do, because nothing else claims it there: their trigger is a sentinel, which is never reported. A broken global-error, the app's last-resort boundary, still reached no sink at all. It reports through the same latch now. The docs said a throwing layout falls through to the next boundary out, which is true only on the 500 path. The other three render one nearest boundary and degrade to a chrome-less standalone render, and discard a control-flow throw from a wrapped layout rather than honouring it. Both surfaces say so now.
1 parent 7b26242 commit 05991db

4 files changed

Lines changed: 133 additions & 9 deletions

File tree

.agents/skills/webjs/references/routing-and-pages.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,14 @@ Refusals worth knowing: `formaction=${fn}` is supported on a `<button>` anywhere
212212
- `error.ts` default-exports `({ error, ...ctx }) => TemplateResult`; catches sibling-page and deeper render errors, innermost wins (prod sends only `error.message`).
213213
- `loading.ts` wraps the sibling page in `Suspense` with an immediately-flushed fallback.
214214
- `not-found.ts` / `forbidden.ts` / `unauthorized.ts` render the nearest matching boundary for the thrown control-flow signal, and receive the same ctx a page does (`params`, `searchParams`, `url`).
215-
- **Every one of those boundaries renders INSIDE the layouts at and above its own segment** (#1298), so it carries the keyed `wj:children` pairs and a client-router navigation into a failing page stays a SOFT navigation with the surrounding chrome and its hydrated state intact. A layout deeper than the boundary is not rendered (it never rendered on the way in), and its module is not in the boundary's boot script; the boundary's own module IS. Since a boundary sits inside its own segment's layout, it cannot catch that layout: a throwing layout is handled by the next boundary OUT, matching Next's `layout -> error -> page` hierarchy. Two consequences: a layout that fetches runs its fetch again on a boundary response, and a `<webjs-suspense>` inside a wrapped layout shows its fallback, because a boundary response is buffered so its status is final before the first byte. A 404 for a URL that matched NO route has no chain to wrap in and stays a bare document.
215+
- **Every one of those boundaries renders INSIDE the layouts at and above its own segment** (#1298), so it carries the keyed `wj:children` pairs and a client-router navigation into a failing page stays a SOFT navigation with the surrounding chrome and its hydrated state intact. A layout deeper than the boundary is not rendered (it never rendered on the way in), and its module is not in the boundary's boot script; the boundary's own module IS. Since a boundary sits inside its own segment's layout, it cannot catch that layout, matching Next's `layout -> error -> page` hierarchy. What happens next differs by path, and the difference is worth knowing:
216+
217+
- On the **500 path**, a throwing layout is handled by the next `error.{js,ts}` OUT: the walk tries each boundary in the chain, innermost first, and a layout that throws fails every attempt whose wrapped set contains it, so control ends at `global-error` (or the default 500 page) when they are exhausted.
218+
- On the **404 / 403 / 401 paths** there is NO outward walk. Each renders the one nearest boundary, so a throwing layout degrades that response to a chrome-less standalone render of the boundary with no boot script, keeping its status. A control-flow throw from a wrapped layout there (an auth-gate layout calling `redirect('/login')`) is discarded rather than honoured, deliberately: the status is already decided and the boundary page is the answer to that request.
219+
220+
A genuine layout crash is reported to `onError` (and to the dev overlay on the 404 / 403 / 401 paths, where nothing else claimed the frame) rather than being swallowed; a control-flow sentinel never is, since it is routing rather than a crash.
221+
222+
Two further consequences: a layout that fetches runs its fetch again on a boundary response, and a `<webjs-suspense>` inside a wrapped layout shows its fallback, because a boundary response is buffered so its status is final before the first byte. A 404 for a URL that matched NO route has no chain to wrap in and stays a bare document.
216223
- Root-only (in `app/` exactly): `global-error.ts` is the app-wide catch-all after nested `error` boundaries are exhausted and renders its OWN `<!doctype><html><body>` (returned verbatim, so keep it static HTML with no components or hydration). That verbatim document is exactly why it is the one boundary left UNWRAPPED: a second shell would nest inside the root layout's, wrapping it would re-run the code that just threw, and with no boot script it could not soft-swap anyway. `global-not-found.ts` renders for an unmatched-anywhere URL when no `not-found` matches.
217224

218225
Metadata routes (`sitemap.ts`, `robots.ts`, `manifest.ts`, `icon.ts`, `apple-icon.ts`, `opengraph-image.ts`, `twitter-image.ts`) live at app root or static segments and default-export a possibly-async function; `sitemap()` / `sitemapIndex()` from `@webjsdev/server` serialize spec-valid XML.

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ Default export receives `{ children, params, searchParams, url }`, must embed `c
340340

341341
### Error / loading / metadata routes
342342

343-
`error.{js,ts}` default-exports `({ error, ...ctx }) => TemplateResult` (catches sibling-page / deeper render errors, innermost wins, prod sends only `error.message`). **A boundary renders INSIDE the layouts at and above its own segment** (#1298), so it carries the keyed `wj:children` pairs and navigating into a failing page is a SOFT navigation with the site chrome and its hydrated state intact; the same holds for `not-found` / `forbidden` / `unauthorized`, which now receive a page-shaped ctx too. A layout deeper than the boundary is not rendered and is not in the boot script, which ships the boundary module instead of the page's. A boundary cannot catch its OWN segment's layout (it renders inside it, Next's `layout -> error -> page` order), so a throwing layout falls through to the next boundary out. `loading.{js,ts}` wraps the sibling page in `Suspense` with an immediately-flushed fallback. `forbidden.{js,ts}` / `unauthorized.{js,ts}` render the nearest 403 / 401 boundary for a thrown `forbidden()` / `unauthorized()` (#848). Two **root-only** boundaries (`app/` root exactly): `global-error.{js,ts}` is the app-wide catch-all tried after the nested `error` boundaries are exhausted, and it renders its **own** `<!doctype><html><body>` document (returned verbatim, since a root-layout failure is when it fires). It is the sole boundary that is NOT wrapped in layouts: a second shell cannot nest inside the root layout's (invariant 8), wrapping it would re-run the code that just threw, and it ships no boot script anyway. Because it is returned verbatim (no framework `<head>` splice), it ships **no importmap or boot script**, so keep it **static HTML with no components/hydration** (a last-resort page must not depend on the module system that may have just failed); under an opt-in CSP, an inline `<script>` in it must carry the nonce via `cspNonce()` (an inline `<style>` needs one only if you tighten `style-src`, since the default allows inline style outright). `global-not-found.{js,ts}` renders for an unmatched-anywhere URL when no `not-found` matches. `not-found` is nearest-wins from the throwing page's chain (#848 fixed the prior root-only behavior). Metadata routes (`sitemap`, `robots`, `manifest`, `icon`, `apple-icon`, `opengraph-image`, `twitter-image`) live at app root or static segments only and default-export a possibly-async function; `sitemap(entries)` / `sitemapIndex(sitemaps)` from `@webjsdev/server` serialize spec-valid XML. **`icon` / `apple-icon` are auto-LINKED** into the head (`<link rel="icon" href="/icon">`, `<link rel="apple-touch-icon" href="/apple-icon">`, base-path prefixed, no asserted `type` / `sizes` since the route picks its content type at request time), so writing the file is the whole wiring. A declared `metadata.icons` **suppresses** them rather than merging, matching Next's precedence for its static icon files, so an app that outgrows a placeholder route names its real icons instead of deleting it. `opengraph-image` / `twitter-image` are NOT auto-linked (a preview image is a per-page editorial choice): point `metadata.openGraph.images` / `metadata.twitter.images` at them. Declare a favicon through `metadata.icons` or a metadata route, never a hand-written `<link rel="icon">`, since only the root layout may write a shell at all (invariant 8). See `references/routing-and-pages.md`.
343+
`error.{js,ts}` default-exports `({ error, ...ctx }) => TemplateResult` (catches sibling-page / deeper render errors, innermost wins, prod sends only `error.message`). **A boundary renders INSIDE the layouts at and above its own segment** (#1298), so it carries the keyed `wj:children` pairs and navigating into a failing page is a SOFT navigation with the site chrome and its hydrated state intact; the same holds for `not-found` / `forbidden` / `unauthorized`, which now receive a page-shaped ctx too. A layout deeper than the boundary is not rendered and is not in the boot script, which ships the boundary module instead of the page's. A boundary cannot catch its OWN segment's layout (it renders inside it, Next's `layout -> error -> page` order). On the 500 path a throwing layout falls through to the next `error` boundary OUT, and to `global-error` when the chain is exhausted; the 404 / 403 / 401 paths have no outward walk, so there a throwing layout degrades that one response to a chrome-less standalone render of the boundary (status preserved, no boot script) and a control-flow throw from a wrapped layout is discarded rather than honoured, since the status is already decided. A real layout crash is reported to `onError` instead of vanishing; a sentinel never is. `loading.{js,ts}` wraps the sibling page in `Suspense` with an immediately-flushed fallback. `forbidden.{js,ts}` / `unauthorized.{js,ts}` render the nearest 403 / 401 boundary for a thrown `forbidden()` / `unauthorized()` (#848). Two **root-only** boundaries (`app/` root exactly): `global-error.{js,ts}` is the app-wide catch-all tried after the nested `error` boundaries are exhausted, and it renders its **own** `<!doctype><html><body>` document (returned verbatim, since a root-layout failure is when it fires). It is the sole boundary that is NOT wrapped in layouts: a second shell cannot nest inside the root layout's (invariant 8), wrapping it would re-run the code that just threw, and it ships no boot script anyway. Because it is returned verbatim (no framework `<head>` splice), it ships **no importmap or boot script**, so keep it **static HTML with no components/hydration** (a last-resort page must not depend on the module system that may have just failed); under an opt-in CSP, an inline `<script>` in it must carry the nonce via `cspNonce()` (an inline `<style>` needs one only if you tighten `style-src`, since the default allows inline style outright). `global-not-found.{js,ts}` renders for an unmatched-anywhere URL when no `not-found` matches. `not-found` is nearest-wins from the throwing page's chain (#848 fixed the prior root-only behavior). Metadata routes (`sitemap`, `robots`, `manifest`, `icon`, `apple-icon`, `opengraph-image`, `twitter-image`) live at app root or static segments only and default-export a possibly-async function; `sitemap(entries)` / `sitemapIndex(sitemaps)` from `@webjsdev/server` serialize spec-valid XML. **`icon` / `apple-icon` are auto-LINKED** into the head (`<link rel="icon" href="/icon">`, `<link rel="apple-touch-icon" href="/apple-icon">`, base-path prefixed, no asserted `type` / `sizes` since the route picks its content type at request time), so writing the file is the whole wiring. A declared `metadata.icons` **suppresses** them rather than merging, matching Next's precedence for its static icon files, so an app that outgrows a placeholder route names its real icons instead of deleting it. `opengraph-image` / `twitter-image` are NOT auto-linked (a preview image is a per-page editorial choice): point `metadata.openGraph.images` / `metadata.twitter.images` at them. Declare a favicon through `metadata.icons` or a metadata route, never a hand-written `<link rel="icon">`, since only the root layout may write a shell at all (invariant 8). See `references/routing-and-pages.md`.
344344

345345
### Route handlers (`app/**/route.{js,ts}`)
346346

packages/server/src/ssr/render.js

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -435,16 +435,39 @@ async function renderChain(route, ctx, dev, suspenseCtx, have, pageModule) {
435435
* standalone boundary body, exactly as it does for a real layout crash, and
436436
* the status the boundary carries is preserved.
437437
*
438+
* REPORTED AT MOST ONCE PER REQUEST, via the caller's `once` latch. The 500
439+
* path tries each boundary in the chain, and a throwing layout that is an
440+
* ancestor of several of them fails EVERY attempt: `layoutsForBoundary` selects
441+
* by segment ancestry rather than by boundary depth, so two boundaries at
442+
* different segments can resolve to the same layout set. Without the latch a
443+
* root layout that throws is reported once per boundary in the chain. Identity
444+
* deduplication would not do, because a layout that constructs its error
445+
* (`throw new Error(...)`) yields a fresh object per attempt.
446+
*
447+
* `overlay` says whether this failure may claim the DEV OVERLAY. The overlay
448+
* holds one retained frame per URL and the dev handler's slot is last-write
449+
* wins, so a secondary failure pushed after the original page error would
450+
* replace the root cause with a symptom, and the replacement would survive a
451+
* reconnect. So the 500 path passes false (the page error already claimed the
452+
* slot and is what the developer needs to see), while the 403 / 401 / 404
453+
* paths pass true, since nothing else claimed it there: their trigger is a
454+
* control-flow sentinel, which is never reported.
455+
*
438456
* @param {unknown} err
439457
* @param {{ onError?: (e: unknown) => void, onDevError?: (e: unknown) => void }} opts
440-
* @param {string} what Where the throw came from, for the log line.
458+
* @param {{ what?: string, once?: { reported: boolean }, overlay?: boolean }} [cfg]
441459
*/
442-
function reportBoundaryLayoutError(err, opts, what = 'a layout threw while wrapping a boundary page') {
460+
function reportBoundaryLayoutError(err, opts, cfg = {}) {
443461
if (isRedirect(err) || isNotFound(err) || isForbidden(err) || isUnauthorized(err)) return;
462+
if (cfg.once) {
463+
if (cfg.once.reported) return;
464+
cfg.once.reported = true;
465+
}
466+
const what = cfg.what || 'a layout threw while wrapping a boundary page';
444467
if (typeof opts.onError === 'function') {
445468
try { opts.onError(err); } catch { /* a throwing sink must not affect the response */ }
446469
}
447-
if (typeof opts.onDevError === 'function') {
470+
if (cfg.overlay !== false && typeof opts.onDevError === 'function') {
448471
try { opts.onDevError(err); } catch { /* a throwing sink must not affect the response */ }
449472
}
450473
console.error(`[webjs] ${what}:`, err);
@@ -570,7 +593,7 @@ async function ssrBoundaryHtml(file, heading, opts) {
570593
// #1298, so a genuine layout crash would otherwise vanish, leaving a
571594
// developer looking at a chrome-less boundary page with nothing
572595
// saying why, and an APM sink that never heard about it.
573-
reportBoundaryLayoutError(layoutErr, opts);
596+
reportBoundaryLayoutError(layoutErr, opts, { overlay: true });
574597
body = await renderToString(tree, { ssr: true, dev: opts.dev });
575598
moduleUrls = [];
576599
}
@@ -622,7 +645,7 @@ async function ssrNotFoundHtml(notFoundFile, opts) {
622645
}
623646
} catch (layoutErr) {
624647
// Same degradation, and the same reporting, as ssrBoundaryHtml above.
625-
reportBoundaryLayoutError(layoutErr, opts);
648+
reportBoundaryLayoutError(layoutErr, opts, { overlay: true });
626649
body = await renderToString(tree, { ssr: true, dev: opts.dev });
627650
moduleUrls = [];
628651
}
@@ -1047,6 +1070,9 @@ export async function ssrPage(route, params, url, opts) {
10471070
// error page's boot scripts (when moduleUrls is non-empty) and
10481071
// the meta csp-nonce tag both pass strict-CSP enforcement.
10491072
const errNonce = opts.req ? getNonce(opts.req) : undefined;
1073+
// One latch for the whole 500 path: the walk below, and the global-error
1074+
// attempt after it, report at most ONE secondary failure between them.
1075+
const secondary = { reported: false };
10501076
// Try nearest error.js (innermost → outermost).
10511077
for (let i = route.errors.length - 1; i >= 0; i--) {
10521078
try {
@@ -1097,7 +1123,11 @@ export async function ssrPage(route, params, url, opts) {
10971123
// handling it would vanish completely. A control-flow sentinel is
10981124
// filtered out by the reporter, since a boundary throwing notFound() is
10991125
// a routing decision rather than a crash.
1100-
reportBoundaryLayoutError(nested, opts, 'an error boundary or its layout threw while handling a render error');
1126+
reportBoundaryLayoutError(nested, opts, {
1127+
what: 'an error boundary or its layout threw while handling a render error',
1128+
once: secondary,
1129+
overlay: false,
1130+
});
11011131
}
11021132
}
11031133
// Root global-error.{js,ts} (#848): the app-wide catch-all, tried after the
@@ -1119,7 +1149,16 @@ export async function ssrPage(route, params, url, opts) {
11191149
return htmlResponse(body, 500, opts.req, url);
11201150
}
11211151
} catch (nested) {
1122-
// fall through to the default 500 page
1152+
// Fall through to the default 500 page, but do not lose this. A broken
1153+
// global-error.{js,ts} is the app's LAST-RESORT boundary failing, and
1154+
// the developer would otherwise see only the generic default page and
1155+
// the original error, with nothing naming the boundary as the thing
1156+
// that failed.
1157+
reportBoundaryLayoutError(nested, opts, {
1158+
what: 'global-error threw while handling a render error',
1159+
once: secondary,
1160+
overlay: false,
1161+
});
11231162
}
11241163
}
11251164
// Default: dev shows stack, prod shows a terse message (no stack trace leaks).

0 commit comments

Comments
 (0)