fix: do not report a control-flow throw as a crash, and stop losing the 500 one

The new boundary-layout reporter treated every throw as an error, so a layout
throwing redirect() while wrapping a 403 reached the app's APM sink and painted
a false dev error overlay over the page the user was looking at. That is the
classification the page path already makes before it reports, and it matters
more here because of what the overlay lands on top of. Sentinels are filtered
out now. They are still not HONOURED, which is deliberate and written down: the
status is decided by the time a boundary renders and the boundary page is the
answer to the request, so a layout redirect cannot replace a 403 the app asked
for with a bounce it did not.

The 500 path kept swallowing the same throw the commit before this one exists
to stop losing. Its fallthrough to the next boundary out is right, but the
sinks only ever heard the original page error, so a boundary or layout that
crashed while handling it vanished. It reports on the way past now.

The non-empty guard on the instrumentation prepend was unreachable and its
comment described a state the function cannot be in: the boundary file is
always pushed, since only pages and layouts are fed to the elision analysis.

Author: vivek7405

packages/server/src/ssr/render.js

@@ -415,22 +415,39 @@ async function renderChain(route, ctx, dev, suspenseCtx, have, pageModule) {
 }
 
 /**
- * Report a throw from a layout wrapped around a 403 / 401 / 404 boundary
- * (#1298) to the same sinks a page-render error reaches, then let the caller
- * degrade to the standalone render. Best-effort on both sinks: a throwing sink
- * must never affect the response.
+ * 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
+ * standalone render. Best-effort on both sinks: a throwing sink must never
+ * affect the response.
+ *
+ * A CONTROL-FLOW sentinel is not an error and is never reported. This is the
+ * same classification the page path makes before it calls `onError` (a
+ * `redirect()` / `notFound()` / `forbidden()` / `unauthorized()` is a routing
+ * decision, not a crash), and it matters more here: reporting one would fire
+ * the app's APM sink and paint a false dev error overlay OVER the 403 the user
+ * is looking at.
+ *
+ * The sentinel is not HONOURED either, and that is deliberate. The status is
+ * already decided by the time a boundary renders, and the boundary page IS the
+ * answer to this request; letting a layout redirect out of it would replace a
+ * 403 the app asked for with a bounce the app did not, on a path where the
+ * redirect target could itself be forbidden. So the render degrades to the
+ * standalone boundary body, exactly as it does for a real layout crash, and
+ * the status the boundary carries is preserved.
  *
  * @param {unknown} err
  * @param {{ onError?: (e: unknown) => void, onDevError?: (e: unknown) => void }} opts
+ * @param {string} what  Where the throw came from, for the log line.
  */
-function reportBoundaryLayoutError(err, opts) {
+function reportBoundaryLayoutError(err, opts, what = 'a layout threw while wrapping a boundary page') {
+  if (isRedirect(err) || isNotFound(err) || isForbidden(err) || isUnauthorized(err)) return;
   if (typeof opts.onError === 'function') {
     try { opts.onError(err); } catch { /* a throwing sink must not affect the response */ }
   }
   if (typeof opts.onDevError === 'function') {
     try { opts.onDevError(err); } catch { /* a throwing sink must not affect the response */ }
   }
-  console.error('[webjs] a layout threw while wrapping a boundary page:', err);
+  console.error(`[webjs] ${what}:`, err);
 }
 
 /**
@@ -475,9 +492,11 @@ function boundaryModuleUrls(boundaryFile, wrapLayouts, opts) {
   // it has on the happy path: it runs before app modules so the app's client
   // error reporting is installed before anything can throw. A boundary page is
   // where that matters most, so it must not be the one place it is missing.
-  // Only when the set is non-empty: an empty set means no boot script at all,
-  // and instrumentation alone is not a reason to start emitting one.
-  if (opts.instrumentationClient && urls.length) {
+  // The set is never empty by the time we get here (the boundary file is
+  // always pushed above, since only pages and layouts are fed to the elision
+  // analysis, so a boundary is never inert and never import-only), which is
+  // why this needs no non-empty guard.
+  if (opts.instrumentationClient) {
     const u = toUrlPath(opts.instrumentationClient, opts.appDir);
     const i = urls.indexOf(u);
     if (i !== -1) urls.splice(i, 1);
@@ -1070,6 +1089,15 @@ export async function ssrPage(route, params, url, opts) {
         // absorbs a throw from a WRAPPED LAYOUT, which is the case that makes a
         // throwing layout render the boundary outside it (Next parity: a
         // boundary sits inside its own segment's layout, so it cannot catch it).
+        //
+        // Report it on the way past. The fallthrough is the right BEHAVIOUR,
+        // but it is not a reason to lose the error: whatever this response ends
+        // up being, the sinks only ever hear about the ORIGINAL page error
+        // (reported above), so a boundary or a layout that crashed while
+        // handling it would vanish completely. A control-flow sentinel is
+        // filtered out by the reporter, since a boundary throwing notFound() is
+        // a routing decision rather than a crash.
+        reportBoundaryLayoutError(nested, opts, 'an error boundary or its layout threw while handling a render error');
       }
     }
     // Root global-error.{js,ts} (#848): the app-wide catch-all, tried after the

test/ssr/ssr.test.js

@@ -2127,6 +2127,61 @@ test('boundary: a layout that throws while wrapping a 403 is REPORTED, not swall
   assert.match(String(seen[0].message), /layout-boom-403/);
 });
 
+test('boundary: a layout control-flow throw is NOT reported as an error', async () => {
+  // A redirect() from a layout is a routing decision, not a crash. Reporting it
+  // would fire the app's APM sink and paint a false dev error overlay OVER the
+  // 403 the user is looking at. It is not honoured either: the status is
+  // already decided and the boundary page IS the answer to this request.
+  const { route, appDir } = makeBoundaryApp({
+    files: {
+      'layout.js': `import { redirect } from ${JSON.stringify(WEBJS_MODULE_URL)};\nexport default function Root() { redirect('/login'); }\n`,
+      'admin/forbidden.js': HTML_IMPORT + `export default function F() { return html\`<p id="fb">no</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 devSeen = [];
+  const resp = await SILENT(() => ssrPage(route, {}, new URL('http://localhost/admin'), {
+    dev: true, appDir, onError: (e) => seen.push(e), onDevError: (e) => devSeen.push(e),
+  }));
+  assert.equal(resp.status, 403, 'the boundary still answers, and its status is preserved');
+  const body = await resp.text();
+  assert.ok(body.includes('id="fb"'), 'the boundary rendered');
+  assert.deepEqual(seen, [], 'a control-flow sentinel never reaches the APM sink');
+  assert.deepEqual(devSeen, [], 'nor the dev error overlay');
+});
+
+test('boundary: a layout that throws on the 500 path is reported, not lost', async () => {
+  // The fallthrough to the next boundary out is the right behaviour, but the
+  // sinks otherwise only ever hear the ORIGINAL page error, so a boundary or
+  // layout that crashed while handling it would vanish completely.
+  const { route, appDir } = makeBoundaryApp({
+    files: {
+      'layout.js': HTML_IMPORT + `export default function Root({ children }) { return html\`<div id="root-chrome">\${children}</div>\`; }\n`,
+      'error.js': HTML_IMPORT + `export default function Err() { return html\`<p id="root-boundary">outer</p>\`; }\n`,
+      'docs/layout.js': `export default function Docs() { throw new Error('layout-boom-500'); }\n`,
+      'docs/error.js': HTML_IMPORT + `export default function Err() { return html\`<p id="docs-boundary">inner</p>\`; }\n`,
+      'docs/page.js': `export default function Page() { throw new Error('page-boom'); }\n`,
+    },
+    page: 'docs/page.js',
+    layouts: ['layout.js', 'docs/layout.js'],
+    errors: ['error.js', 'docs/error.js'],
+  });
+  const seen = [];
+  const resp = await SILENT(() => ssrPage(route, {}, new URL('http://localhost/docs'), {
+    dev: false, appDir, onError: (e) => seen.push(e),
+  }));
+  assert.equal(resp.status, 500);
+  const body = await resp.text();
+  assert.ok(body.includes('id="root-boundary"'), 'it still falls through to the boundary outside the throwing layout');
+  const messages = seen.map((e) => String(e && e.message));
+  assert.ok(messages.includes('page-boom'), 'the original page error is still reported');
+  assert.ok(messages.includes('layout-boom-500'), 'and so is the layout crash that used to vanish');
+});
+
 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