fix: record the boundary failure phase instead of inferring it

Inferring the phase by re-rendering is unsound when BOTH a wrapped layout
and the boundary tree are broken: the standalone attempt throws too, so the
tree error was discarded and the LAYOUT error was reported under the
boundary label. Two failures, one report, wrong name, in the code whose
whole purpose is that a boundary crash is never lost. The phase is known
exactly where it happens, so renderBoundaryInChain marks it and the catch
reads it. Both errors are reported now, each under its own label, and the
dev body shows the tree error, which is the one that defeated the fallback.

The unrouted 404's dev frame also stamped a bare pathname. The overlay gate
compares against location.pathname + location.search, so the frame was
refused (and then dropped) for any url carrying a query, and for every url
on a base-path deploy, where the ingress strip removed the prefix from url
while the browser still has it. It stamps what the matched-page branch
stamps. That hook is also dropped for a speculative prefetch now, per the
same #1047 rule: hovering a link to a broken url must not raise an overlay
on the page the user is actually on.

Author: vivek7405

packages/server/src/dev/serve.js

@@ -509,7 +509,26 @@ export async function handleCore(req, ctx) {
     req,
     url,
     onError: reportError ? (e) => reportError(e, req, 'ssr') : undefined,
-    onDevError: dev ? (e) => reportDevError(e, { kind: 'render', url: url.pathname }) : undefined,
+    // The frame's url must be stamped exactly as the matched-page branch
+    // stamps it, or the overlay's scope gate refuses it: the browser compares
+    // against `location.pathname + location.search`, so dropping the query
+    // suppresses the overlay on any unrouted url that carries one, and
+    // dropping the base path suppresses it on EVERY url of a sub-path deploy
+    // (the ingress strip already removed the prefix from `url`, while the
+    // browser's location still has it). A refused frame goes to the pending
+    // slot and is then discarded, so the overlay simply never paints.
+    //
+    // Dropped for a speculative prefetch, on the same #1047 rule the
+    // matched-page branch follows: hovering a link to a broken page must not
+    // raise an overlay on the page you are actually on, nor become
+    // `state.lastDevError` for the SSE to replay into a fresh tab. Prefetch is
+    // on by default and fetches unrouted hrefs too, so this is reachable.
+    onDevError: dev && req.headers.get('x-webjs-prefetch') !== '1'
+      ? (e) => reportDevError(e, {
+        kind: 'render',
+        url: withBasePath(url.pathname, basePath()) + url.search,
+      })
+      : undefined,
   });
 }
 

packages/server/src/ssr/render.js

@@ -339,7 +339,18 @@ async function renderBoundaryInChain(tree, route, boundaryFile, ctx, dev) {
   const pageSeg = pageSegmentPath(route.file);
   const layoutSegs = new Set((route.layouts || []).map(layoutSegmentPath));
   if (!layoutSegs.has(pageSeg)) tree = wrapWithChildrenMarker(tree, pageSeg, params);
-  const chain = await wrapLayoutChain(tree, wrapLayouts, ctx, dev, params, null);
+  // MARK a layout-phase failure. The caller has to tell "a wrapped layout
+  // threw" apart from "the boundary's own tree is broken", and it cannot infer
+  // it by re-rendering: when BOTH are broken the standalone attempt throws too,
+  // and an inference would report the layout error under the boundary's label
+  // and lose the tree's entirely. The phase is known exactly here, so it is
+  // recorded rather than guessed.
+  let chain;
+  try {
+    chain = await wrapLayoutChain(tree, wrapLayouts, ctx, dev, params, null);
+  } catch (e) {
+    throw markLayoutPhase(e);
+  }
   return renderToString(chain.tree, { ssr: true, dev });
 }
 
@@ -414,6 +425,28 @@ async function renderChain(route, ctx, dev, suspenseCtx, have, pageModule) {
   return { html: body + (await loadingTemplates(route, ctx, dev)), reduced: chain.reduced };
 }
 
+/**
+ * Marker for a throw that came from the LAYOUT-wrapping phase of a boundary
+ * render, as opposed to the boundary's own tree (#1298). Non-enumerable, so it
+ * never shows up in a serialized error, and best-effort: a frozen or primitive
+ * throw simply goes unmarked and is treated as a tree failure, which reports it
+ * once under the boundary label rather than dropping it.
+ */
+const LAYOUT_PHASE = Symbol('webjs.boundaryLayoutPhase');
+
+/** @param {unknown} err @returns {unknown} */
+function markLayoutPhase(err) {
+  if (err && typeof err === 'object') {
+    try { Object.defineProperty(err, LAYOUT_PHASE, { value: true, enumerable: false }); } catch { /* frozen */ }
+  }
+  return err;
+}
+
+/** @param {unknown} err @returns {boolean} */
+function isLayoutPhase(err) {
+  return !!(err && typeof err === 'object' && /** @type {any} */ (err)[LAYOUT_PHASE]);
+}
+
 /**
  * Render a thrown value as text for a DEV error page, safely.
  *
@@ -668,25 +701,26 @@ async function ssrBoundaryHtml(file, heading, opts) {
             body = await renderToString(tree, { ssr: true, dev: opts.dev });
           }
         } catch (layoutErr) {
-          // Degrade to the standalone render this has always produced, and to
-          // its empty boot set with it.
-          //
-          // The standalone attempt is also what tells us WHAT failed, so it
-          // runs before anything is reported. If it succeeds, the fault was in
-          // the layout chain: report it, because these paths execute layout
-          // modules for the first time since #1298 and a genuine layout crash
-          // would otherwise vanish, leaving a developer looking at a
-          // chrome-less boundary page with nothing saying why. If it throws
-          // too, the boundary's OWN tree is what is broken, so rethrow and let
-          // the outer catch report it once, under the right label. Reporting
-          // here first would report a tree failure twice and call it a layout.
+          // A TREE failure is not this catch's business: the standalone
+          // fallback would re-render the same tree and fail the same way, so
+          // rethrow and let the outer catch report it ONCE under the boundary
+          // label. Reporting here as well would double-report it and call it a
+          // layout crash.
+          if (!isLayoutPhase(layoutErr)) throw layoutErr;
+          // A wrapped layout threw. Degrade to the standalone render this has
+          // always produced, and to its empty boot set with it. Report it
+          // either way: these paths execute layout modules for the first time
+          // since #1298, so a genuine layout crash would otherwise vanish.
+          reportBoundaryLayoutError(layoutErr, opts, { overlay: true });
           try {
             body = await renderToString(tree, { ssr: true, dev: opts.dev });
             moduleUrls = [];
-          } catch {
-            throw layoutErr;
+          } catch (treeErr) {
+            // BOTH are broken. The layout crash is already reported above, so
+            // hand the outer catch the TREE error: it is a second, distinct
+            // failure, and it is the one whose text the dev body should show.
+            throw treeErr;
           }
-          reportBoundaryLayoutError(layoutErr, opts, { overlay: true });
         }
       }
     } catch (e) {
@@ -747,15 +781,15 @@ async function ssrNotFoundHtml(notFoundFile, opts) {
             body = await renderToString(tree, { ssr: true, dev: opts.dev });
           }
         } catch (layoutErr) {
-          // Same degradation, and the same report-only-if-the-fault-was-the-
-          // chain rule, as ssrBoundaryHtml above.
+          // Same phase rule as ssrBoundaryHtml above.
+          if (!isLayoutPhase(layoutErr)) throw layoutErr;
+          reportBoundaryLayoutError(layoutErr, opts, { overlay: true });
           try {
             body = await renderToString(tree, { ssr: true, dev: opts.dev });
             moduleUrls = [];
-          } catch {
-            throw layoutErr;
+          } catch (treeErr) {
+            throw treeErr;
           }
-          reportBoundaryLayoutError(layoutErr, opts, { overlay: true });
         }
       }
     } catch (e) {

packages/server/test/routing/global-boundaries.test.js

@@ -138,3 +138,52 @@ test('an unrouted 404 whose not-found THROWS reaches the onError sink', async ()
   assert.equal(captured.length, 1, 'the crash reached the APM sink through the real path');
   assert.match(String(captured[0].message), /ROOT_NF_BOOM/);
 });
+
+test('an unrouted 404 stamps its dev frame the way the overlay gate expects', async () => {
+  // The browser compares the frame url against `location.pathname +
+  // location.search`. A stamp missing the query is refused for any unrouted
+  // url carrying one, and the frame is then dropped, so the overlay never
+  // paints. This asserts the stamp, which is the half a render test cannot see.
+  const appDir = makeApp({
+    'package.json': pkg,
+    'app/page.js': page('export default function H() { return html`<main>home</main>`; }'),
+    'app/not-found.js':
+      `export default function NF() { throw new Error('NF_FRAME_BOOM'); }\n`,
+  });
+  const frames = [];
+  const app = await createRequestHandler({
+    appDir, dev: true, onDevError: (frame) => frames.push(frame),
+  });
+  const prev = console.error;
+  console.error = () => {};
+  try {
+    await app.handle(new Request('http://x/missing?page=2'));
+  } finally { console.error = prev; }
+  assert.equal(frames.length, 1, 'a frame was pushed');
+  assert.equal(frames[0].url, '/missing?page=2', 'and it carries the search, as the gate requires');
+});
+
+test('a PREFETCH of an unrouted url raises no dev overlay frame (#1047)', async () => {
+  // Hovering a link to a broken url must not raise an overlay on the page the
+  // user is actually looking at, nor become the frame the SSE replays.
+  const appDir = makeApp({
+    'package.json': pkg,
+    'app/page.js': page('export default function H() { return html`<main>home</main>`; }'),
+    'app/not-found.js':
+      `export default function NF() { throw new Error('NF_PREFETCH_BOOM'); }\n`,
+  });
+  const frames = [];
+  const errors = [];
+  const app = await createRequestHandler({
+    appDir, dev: true,
+    onDevError: (frame) => frames.push(frame),
+    onError: (e) => errors.push(e),
+  });
+  const prev = console.error;
+  console.error = () => {};
+  try {
+    await app.handle(new Request('http://x/missing', { headers: { 'x-webjs-prefetch': '1' } }));
+  } finally { console.error = prev; }
+  assert.equal(frames.length, 0, 'no overlay frame from a speculative fetch');
+  assert.equal(errors.length, 1, 'but the APM sink still hears about it, since the render really threw');
+});

test/ssr/ssr.test.js

@@ -2495,6 +2495,36 @@ test('boundary: a boundary whose own TREE fails is reported ONCE, and not as a l
   assert.equal(hits.length, 1, 'reported once, not once per render attempt');
 });
 
+test('boundary: when the layout AND the tree are broken, BOTH are reported', async () => {
+  // The previous shape inferred the phase by re-rendering, so with both broken
+  // the standalone attempt threw too, the tree error was discarded, and the
+  // LAYOUT error was reported under the boundary label. Two failures, one
+  // report, wrong name. The phase is recorded at its source now.
+  const { route, appDir } = makeBoundaryApp({
+    files: {
+      'layout.js': `export default function Root() { throw new Error('LAYOUT_HALF_BOOM'); }\n`,
+      'admin/forbidden.js': HTML_IMPORT + `export default function F() {
+        return html\`<p>\${Promise.reject(new Error('TREE_HALF_BOOM'))}</p>\`;
+      }\n`,
+      'admin/page.js': `import { forbidden } from ${JSON.stringify(WEBJS_MODULE_URL)};\nexport default function Page() { forbidden(); }\n`,
+    },
+    page: 'admin/page.js',
+    layouts: ['layout.js'],
+    forbiddens: ['admin/forbidden.js'],
+  });
+  const seen = [];
+  const resp = await SILENT(() => ssrPage(route, {}, new URL('http://localhost/admin'), {
+    dev: true, appDir, onError: (e) => seen.push(e),
+  }));
+  assert.equal(resp.status, 403, 'it still answers with the boundary status');
+  const messages = seen.map((e) => String(e && e.message));
+  assert.ok(messages.includes('LAYOUT_HALF_BOOM'), 'the layout crash is reported');
+  assert.ok(messages.includes('TREE_HALF_BOOM'), 'and the tree crash is not lost behind it');
+  // The dev body shows the TREE error, since that is the one that defeated the
+  // fallback and left the page with nothing to render.
+  assert.match(await resp.text(), /TREE_HALF_BOOM/);
+});
+
 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