fix: key boundary-report dedup on the stage, and make the key total

The fingerprint was wrong on three counts and unsafe on a fourth.

It never saw the original error, so the commonest layout crash of all was
reported twice: when a layout is what threw, it is both the error that
produced the 500 and the thing re-run around every boundary it wraps. The
set is seeded with it now.

Keying on the whole stack cannot work, and neither can keying on the first
frame alone. The frames below the construction site record how the throw was
reached, so the same layout re-rendered around a different boundary never
matches itself; the construction site alone, meanwhile, is shared by two
genuinely different failures that go through one helper. Those two shapes are
indistinguishable by stack, so the key carries the STAGE instead. One layout
failing repeatedly inside the walk collapses to one report, while a shared
helper failing in both the walk and global-error reports twice.

The key could also throw. String(Object.create(null)) throws, and this runs
inside the catch that keeps the response alive, ahead of the guarded sinks,
so a null-prototype throw escaped ssrPage and took the 500 page with it. It
is total now and returns null when no safe key exists, which means report
rather than risk dropping. That also covers a non-Error throw, which used to
collapse every plain object to one key and silently drop the second.

Author: vivek7405

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

@@ -217,7 +217,7 @@ Refusals worth knowing: `formaction=${fn}` is supported on a `<button>` anywhere
 - 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.
 - 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.
 
-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. Repeats of the SAME cause within one request collapse to a single report, keyed on the error's name, message and throw site, because one shared layout can fail several boundary attempts; two DIFFERENT failures are both reported, including `global-error`'s own. A control-flow sentinel never is, since it is routing rather than a crash.
+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. Repeats of the SAME cause within one request collapse to a single report, because one shared layout can fail every boundary attempt (and, when the layout is what threw, arrives again as the error that produced the 500). The key is the STAGE plus the error's name, message and construction site: the stack below that site records how the throw was reached and differs on every re-render, so it cannot be part of the key, and the stage is what keeps `global-error`'s own crash from being swallowed by a boundary that failed through the same helper. Two DIFFERENT failures are both reported, and anything whose key cannot be derived safely (a non-Error throw) is always reported rather than risking a drop. A control-flow sentinel never is, since it is routing rather than a crash.
 
 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.
 - 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.

packages/server/src/ssr/render.js

@@ -416,21 +416,49 @@ async function renderChain(route, ctx, dev, suspenseCtx, have, pageModule) {
 
 /**
  * A per-request dedup key for a secondary boundary failure: the error's name,
- * message and throw site. Identity cannot be used, because a layout that
+ * message and FULL stack. Identity cannot be used, because a layout that
  * constructs its error yields a fresh object each time it is re-run, and the
  * whole point is to collapse exactly those repeats while letting a genuinely
- * different failure through. The first stack frame is the throw site, so two
- * different layouts raising the same message still read as distinct causes.
+ * different failure through.
+ *
+ * The CONSTRUCTION SITE (the first stack frame), not the whole stack. The
+ * frames below it record how the throw was reached, and the same layout
+ * re-rendered around a different boundary is reached differently every time,
+ * so a full-stack key would never collapse the repeats it exists for.
+ *
+ * That leaves the construction site shared by two genuinely different
+ * failures that go through one helper, which is why the caller's STAGE is part
+ * of the key rather than the error alone: the boundary walk and the
+ * `global-error` attempt are different stages, so a shared helper failing in
+ * both is reported twice, while one layout failing repeatedly INSIDE the walk
+ * is reported once. The stage is what distinguishes them; the stack cannot.
+ *
+ * Returns null when no safe key can be derived, which means DO NOT DEDUPE.
+ * Failing open is the only acceptable direction here: a duplicate report is
+ * noise, a dropped one is a crash nobody hears about. That covers a non-Error
+ * throw (two unrelated plain objects would otherwise share `[object Object]`)
+ * and anything whose property access or stringification throws, which is a
+ * real shape: `String(Object.create(null))` throws, and this runs INSIDE the
+ * catch that is supposed to keep the response alive, so a throw here would
+ * escape `ssrPage` and take the 500 page with it.
  *
  * @param {unknown} err
- * @returns {string}
+ * @returns {string | null}
  */
-function boundaryErrorKey(err) {
-  if (!(err instanceof Error)) return `raw:${String(err)}`;
-  const site = String(err.stack || '').split('\n')[1] || '';
-  return `${err.name}:${err.message}:${site.trim()}`;
+function boundaryErrorKey(err, stage) {
+  if (!(err instanceof Error)) return null;
+  try {
+    const site = String(err.stack || '').split('\n')[1] || '';
+    return `${stage}\u0000${String(err.name)}:${String(err.message)}:${site.trim()}`;
+  } catch {
+    return null;
+  }
 }
 
+/** Stage labels, which are part of the dedup key (see boundaryErrorKey). */
+const STAGE_WALK = 'an error boundary or its layout threw while handling a render error';
+const STAGE_GLOBAL_ERROR = 'global-error threw while handling a render error';
+
 /**
  * Report a throw from a layout wrapped around a boundary page (#1298) to the
  * same sinks a page-render error reaches, then let the caller degrade to the
@@ -484,9 +512,13 @@ function boundaryErrorKey(err) {
 function reportBoundaryLayoutError(err, opts, cfg = {}) {
   if (isRedirect(err) || isNotFound(err) || isForbidden(err) || isUnauthorized(err)) return;
   if (cfg.seen) {
-    const key = boundaryErrorKey(err);
-    if (cfg.seen.has(key)) return;
-    cfg.seen.add(key);
+    const key = boundaryErrorKey(err, cfg.what || '');
+    // A null key means no safe key could be derived, so report rather than
+    // risk dropping. See boundaryErrorKey.
+    if (key !== null) {
+      if (cfg.seen.has(key)) return;
+      cfg.seen.add(key);
+    }
   }
   const what = cfg.what || 'a layout threw while wrapping a boundary page';
   if (typeof opts.onError === 'function') {
@@ -1099,7 +1131,17 @@ export async function ssrPage(route, params, url, opts) {
     // global-error attempt after it collapse REPEATS of the same cause (a
     // shared layout that fails every attempt) while still reporting a
     // genuinely different failure, including global-error's own.
+    //
+    // SEEDED with the original error, which was just reported above. When a
+    // LAYOUT is what threw, the walk re-runs that same layout around each
+    // boundary it wraps (the root layout wraps every boundary), so the cause
+    // that produced this 500 is about to arrive again as a secondary failure.
+    // Without the seed the commonest layout crash of all is reported twice.
     const secondary = new Set();
+    {
+      const originalKey = boundaryErrorKey(err, STAGE_WALK);
+      if (originalKey !== null) secondary.add(originalKey);
+    }
     // Try nearest error.js (innermost → outermost).
     for (let i = route.errors.length - 1; i >= 0; i--) {
       try {
@@ -1151,7 +1193,7 @@ export async function ssrPage(route, params, url, opts) {
         // filtered out by the reporter, since a boundary throwing notFound() is
         // a routing decision rather than a crash.
         reportBoundaryLayoutError(nested, opts, {
-          what: 'an error boundary or its layout threw while handling a render error',
+          what: STAGE_WALK,
           seen: secondary,
           overlay: false,
         });
@@ -1182,7 +1224,7 @@ export async function ssrPage(route, params, url, opts) {
         // the original error, with nothing naming the boundary as the thing
         // that failed.
         reportBoundaryLayoutError(nested, opts, {
-          what: 'global-error threw while handling a render error',
+          what: STAGE_GLOBAL_ERROR,
           seen: secondary,
           overlay: false,
         });

test/ssr/ssr.test.js

@@ -2186,7 +2186,7 @@ test('boundary: one throwing layout is reported ONCE, not once per boundary', as
   // The walk tries each boundary in the chain, and layoutsForBoundary selects
   // by segment ancestry rather than boundary depth, so a root layout that
   // throws fails EVERY attempt. Identity dedup would not help: a layout that
-  // constructs its error yields a fresh object per attempt. Hence the latch.
+  // constructs its error yields a fresh object per attempt, so the key is its name, message and stack.
   const { route, appDir } = makeBoundaryApp({
     files: {
       'layout.js': `export default function Root() { throw new Error('root-layout-boom'); }\n`,
@@ -2291,6 +2291,72 @@ test('boundary: dedup collapses REPEATS of one cause, not distinct failures', as
   assert.ok(messages.includes('shared-layout-boom'), 'and the LATER, unrelated layout crash is not swallowed by it');
 });
 
+test('boundary: a LAYOUT crash that produced the 500 is reported once, not twice', async () => {
+  // The commonest shape of all, and the one the dedup set has to be SEEDED for.
+  // When the layout is what threw, it becomes the original error AND is re-run
+  // around every boundary it wraps, so the same cause arrives twice.
+  const { route, appDir } = makeBoundaryApp({
+    files: {
+      'layout.js': `export default function Root() { throw new Error('only-layout-boom'); }\n`,
+      'error.js': HTML_IMPORT + `export default function Err() { return html\`<p id="b">boundary</p>\`; }\n`,
+      // The page renders fine: the LAYOUT is the sole failure.
+      'page.js': HTML_IMPORT + `export default function Page() { return html\`<p>ok</p>\`; }\n`,
+    },
+    page: 'page.js',
+    layouts: ['layout.js'],
+    errors: ['error.js'],
+  });
+  const seen = [];
+  const resp = await SILENT(() => ssrPage(route, {}, new URL('http://localhost/'), {
+    dev: false, appDir, onError: (e) => seen.push(e),
+  }));
+  assert.equal(resp.status, 500);
+  const hits = seen.filter((e) => /only-layout-boom/.test(String(e && e.message)));
+  assert.equal(hits.length, 1, 'reported once, not once as the original and again as a secondary');
+});
+
+test('boundary: a throw whose value cannot be stringified still degrades, never escapes', async () => {
+  // The dedup key runs INSIDE the catch that keeps the response alive, so it
+  // must be total. `String(Object.create(null))` throws, and a key computation
+  // that throws would take ssrPage's 500 page down with it.
+  const { route, appDir } = makeBoundaryApp({
+    files: {
+      'error.js': `export default function Err() { throw Object.create(null); }\n`,
+      'page.js': `export default function Page() { throw Object.create(null); }\n`,
+    },
+    page: 'page.js',
+    layouts: [],
+    errors: ['error.js'],
+  });
+  const resp = await SILENT(() => ssrPage(route, {}, new URL('http://localhost/'), { dev: false, appDir }));
+  assert.equal(resp.status, 500, 'it degraded to the default 500 rather than throwing out of ssrPage');
+  assert.ok((await resp.text()).includes('Something went wrong'));
+});
+
+test('boundary: two boundaries failing through ONE shared helper are both reported', async () => {
+  // The key is the whole stack, not just the frame where the Error was
+  // constructed. Keying on that one frame would make these two collide, and
+  // global-error's own crash would be swallowed by the earlier boundary's.
+  const { route, appDir } = makeBoundaryApp({
+    files: {
+      'boom.js': `export function boom() { throw new Error('shared-helper-boom'); }\n`,
+      'error.js': `import { boom } from './boom.js';\nexport default function Err() { boom(); }\n`,
+      'global-error.js': `import { boom } from './boom.js';\nexport default function GE() { boom(); }\n`,
+      'page.js': `export default function Page() { throw new Error('page-boom'); }\n`,
+    },
+    page: 'page.js',
+    layouts: [],
+    errors: ['error.js'],
+  });
+  const seen = [];
+  const resp = await SILENT(() => ssrPage(route, {}, new URL('http://localhost/'), {
+    dev: false, appDir, globalError: join(appDir, 'global-error.js'), onError: (e) => seen.push(e),
+  }));
+  assert.equal(resp.status, 500);
+  const hits = seen.filter((e) => /shared-helper-boom/.test(String(e && e.message)));
+  assert.equal(hits.length, 2, 'the boundary and global-error each report, despite one construction site');
+});
+
 test('boundary: a boundary response is never storable and never reduced', async () => {
   // Two independent guarantees. The HTML cache refuses a non-200 outright, and
   // the reduced X-Webjs-Have path is structurally unreachable: the boundary