Skip to content

Commit 3afb2d7

Browse files
committed
fix: report a boundary module that throws or fails to load
Sanitizing the 403 / 401 / 404 body removed the only production-visible trace of that failure and put nothing in its place, so a boundary whose module throws or cannot be imported produced a bare heading and no server signal at all, on a request that already returned a 4xx to a real user. Sanitizing a response should move a failure out of the RESPONSE, not out of sight. It reports through the same path the adjacent layout crash does. The docs site gains the rule this commit and the last one established for the boundary's own crash: shown in dev, withheld in prod, reported either way.
1 parent 1fe89dc commit 3afb2d7

3 files changed

Lines changed: 50 additions & 0 deletions

File tree

packages/server/src/ssr/render.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,7 @@ function boundaryErrorKey(err, stage) {
481481
/** Stage labels, which are part of the dedup key (see boundaryErrorKey). */
482482
const STAGE_WALK = 'an error boundary or its layout threw while handling a render error';
483483
const STAGE_GLOBAL_ERROR = 'global-error threw while handling a render error';
484+
const STAGE_BOUNDARY = 'a boundary module threw or failed to load';
484485

485486
/**
486487
* Report a throw from a layout wrapped around a boundary page (#1298) to the
@@ -679,6 +680,12 @@ async function ssrBoundaryHtml(file, heading, opts) {
679680
}
680681
}
681682
} catch (e) {
683+
// The boundary module itself threw or failed to load. REPORT it: with
684+
// the body sanitized below, this is otherwise completely silent in
685+
// production, on a request that already returned a 4xx to a real user.
686+
// Sanitizing without reporting moves a failure out of sight rather than
687+
// out of the response, which is the opposite of the intent.
688+
reportBoundaryLayoutError(e, opts, { what: STAGE_BOUNDARY, overlay: true });
682689
// Dev shows the failure; prod shows only the heading. The 500 path has
683690
// always drawn that line (a thrown error's message is not
684691
// author-controlled and must not reach the client), and these pages were
@@ -737,6 +744,8 @@ async function ssrNotFoundHtml(notFoundFile, opts) {
737744
}
738745
}
739746
} catch (e) {
747+
// Reported for the same reason as in ssrBoundaryHtml above.
748+
reportBoundaryLayoutError(e, opts, { what: STAGE_BOUNDARY, overlay: true });
740749
body = opts.dev
741750
? `<h1>404: Not found</h1><pre>${escapeHtml(safeErrorText(e))}</pre>`
742751
: '<h1>404: Not found</h1>';

test/ssr/ssr.test.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2423,6 +2423,45 @@ test('boundary: an unprintable throw from a 403 boundary degrades, never escapes
24232423
assert.match(await resp.text(), /unprintable value/);
24242424
});
24252425

2426+
test('boundary: a crashing boundary module is REPORTED, so prod is not silent', async () => {
2427+
// Sanitizing the body removed the only production-visible trace of this
2428+
// failure. Sanitizing without reporting moves a failure out of sight rather
2429+
// than out of the response, on a request that already returned a 4xx to a
2430+
// real user.
2431+
const { route, appDir } = makeBoundaryApp({
2432+
files: {
2433+
'admin/forbidden.js': `export default function F() { throw new Error('BOUNDARY_MODULE_BOOM'); }\n`,
2434+
'admin/page.js': `import { forbidden } from ${JSON.stringify(WEBJS_MODULE_URL)};\nexport default function Page() { forbidden(); }\n`,
2435+
},
2436+
page: 'admin/page.js',
2437+
layouts: [],
2438+
forbiddens: ['admin/forbidden.js'],
2439+
});
2440+
const seen = [];
2441+
const resp = await SILENT(() => ssrPage(route, {}, new URL('http://localhost/admin'), {
2442+
dev: false, appDir, onError: (e) => seen.push(e),
2443+
}));
2444+
assert.equal(resp.status, 403);
2445+
assert.ok(!(await resp.text()).includes('BOUNDARY_MODULE_BOOM'), 'the body stays sanitized');
2446+
assert.equal(seen.length, 1, 'but the failure reached the sink');
2447+
assert.match(String(seen[0].message), /BOUNDARY_MODULE_BOOM/);
2448+
});
2449+
2450+
test('boundary: a 404 boundary that fails to LOAD is reported too', async () => {
2451+
// The load failure path, not the render one: a syntax error in not-found.ts.
2452+
const sub = mkdtempSync(join(tmpDir, 'nf-load-'));
2453+
const appDir = join(sub, 'app');
2454+
mkdirSync(appDir, { recursive: true });
2455+
const nf = join(appDir, 'not-found.js');
2456+
writeFileSync(nf, `export default function NF({ x { return; }\n`);
2457+
const seen = [];
2458+
const resp = await SILENT(() => ssrNotFound(nf, {
2459+
dev: false, appDir, url: new URL('http://localhost/gone'), onError: (e) => seen.push(e),
2460+
}));
2461+
assert.equal(resp.status, 404);
2462+
assert.equal(seen.length, 1, 'a boundary that cannot even be imported is not silent either');
2463+
});
2464+
24262465
test('boundary: a boundary response is never storable and never reduced', async () => {
24272466
// Two independent guarantees. The HTML cache refuses a non-200 outright, and
24282467
// the reduced X-Webjs-Have path is structurally unreachable: the boundary

website/app/docs/error-handling/page.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ export default function ErrorHandling() {
4040
4141
<p>A layout that genuinely crashes is reported to your <code>onError</code> hook rather than being swallowed, so it reaches your error tracker. Repeats of the same crash within one request are collapsed to a single report, since one shared layout can fail several boundary attempts. A <code>redirect()</code> or <code>notFound()</code> is never reported, being routing rather than a crash.</p>
4242
43+
<p>The same holds when the <strong>boundary file itself</strong> throws or fails to load. Its response body follows the rule the rest of the framework uses for a thrown error: the failure is shown in development and withheld in production, where the page carries only its status, because a thrown message is not something you control and may name a driver, a path or a connection string. The error still reaches <code>onError</code> and the server log either way, so sanitizing the response never means losing the failure.</p>
44+
4345
<p>Two consequences worth knowing. A layout that fetches data runs that fetch a second time on a boundary response, since the chain is rendered again around the boundary. And a <code>&lt;webjs-suspense&gt;</code> inside a wrapped layout shows its fallback, because a boundary response is buffered so its status and headers are final before the first byte goes out.</p>
4446
4547
<code-block>// app/error.ts: root error boundary

0 commit comments

Comments
 (0)