Skip to content

Commit fdf0e4c

Browse files
committed
fix: do not treat a failed prefetch as a recovery, or re-render a chainless boundary
The retained-frame clear infers that a render succeeded from the frame it captured still being current, and that inference holds only when a sink was installed to write a new one. A prefetch installs none, so a prefetch of a url whose not-found throws AGAIN looked exactly like a recovery and wiped a frame that was still current, which is the gap the retention exists to close. The matched-page branch gates its whole supersede closure on !isPrefetch for this reason; this one was gated only on dev. The standalone fallback also ran when no route was supplied, where there is no layout chain and the try body already ran that exact renderToString. It re-executed the boundary tree to learn nothing, and a tree that is not idempotent under re-render could succeed on the retry and be reported as a layout crash that never happened.
1 parent 641dca1 commit fdf0e4c

3 files changed

Lines changed: 50 additions & 3 deletions

File tree

packages/server/src/dev/serve.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,15 @@ export async function handleCore(req, ctx) {
510510
// page that has since recovered. Scoped to this url, so a good render here
511511
// never erases an error still current for a page the user is looking at.
512512
const devErrorUrl = withBasePath(url.pathname, basePath()) + url.search;
513-
const before = dev ? state.lastDevError : null;
513+
// Gated on !isPrefetch as well as dev, exactly as the matched-page branch
514+
// gates its whole supersede closure. The clear below infers "this render
515+
// succeeded" from "the retained frame is still the one I captured", and that
516+
// inference is only valid when a sink was installed to write a new one. A
517+
// prefetch installs none, so a prefetch of a url that throws AGAIN would
518+
// look like a recovery and wipe a frame that is still current, which is the
519+
// #893 gap the retention exists to close.
520+
const isPrefetch = req.headers.get('x-webjs-prefetch') === '1';
521+
const before = dev && !isPrefetch ? state.lastDevError : null;
514522
const resp = await ssrNotFound(state.routeTable.notFound || state.routeTable.globalNotFound, {
515523
dev,
516524
appDir,
@@ -531,7 +539,7 @@ export async function handleCore(req, ctx) {
531539
// raise an overlay on the page you are actually on, nor become
532540
// `state.lastDevError` for the SSE to replay into a fresh tab. Prefetch is
533541
// on by default and fetches unrouted hrefs too, so this is reachable.
534-
onDevError: dev && req.headers.get('x-webjs-prefetch') !== '1'
542+
onDevError: dev && !isPrefetch
535543
? (e) => reportDevError(e, { kind: 'render', url: devErrorUrl })
536544
: undefined,
537545
});

packages/server/src/ssr/render.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,14 @@ async function ssrBoundaryHtml(file, heading, opts) {
701701
body = await renderToString(tree, { ssr: true, dev: opts.dev });
702702
}
703703
} catch (layoutErr) {
704+
// With no route there was no layout chain: the try body already ran
705+
// this exact `renderToString(tree)`, so re-running it would execute
706+
// the boundary tree a second time to learn nothing, and a tree that
707+
// is not idempotent under re-render (a hole holding a one-shot async
708+
// iterable, say) could even SUCCEED on the retry and be reported as
709+
// a layout crash that never happened. Rethrow: the outer catch
710+
// reports it once, under the boundary label.
711+
if (!opts.route) throw layoutErr;
704712
// Always attempt the standalone render, which is the degradation
705713
// this path has always produced. Its OUTCOME is half the diagnosis
706714
// and the phase marker is the other half; neither alone is enough.
@@ -793,7 +801,9 @@ async function ssrNotFoundHtml(notFoundFile, opts) {
793801
body = await renderToString(tree, { ssr: true, dev: opts.dev });
794802
}
795803
} catch (layoutErr) {
796-
// Same diagnosis rule as ssrBoundaryHtml above.
804+
// Same diagnosis rule as ssrBoundaryHtml above, including the
805+
// no-route short-circuit.
806+
if (!opts.route) throw layoutErr;
797807
let treeErr = null;
798808
try {
799809
body = await renderToString(tree, { ssr: true, dev: opts.dev });

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,3 +222,32 @@ export default function NF() {
222222
const held = app.getLastDevError();
223223
assert.equal(held, null, 'no stale frame retained for a url that recovered');
224224
});
225+
226+
test('a PREFETCH that fails again does not wipe a still-current overlay frame', async () => {
227+
// The clear infers "this render succeeded" from "the retained frame is still
228+
// the one I captured". A prefetch installs no sink, so a prefetch of a url
229+
// that throws AGAIN writes no frame and would look exactly like a recovery.
230+
// The url is still broken; wiping the frame is the #893 gap the retention
231+
// exists to close.
232+
const appDir = makeApp({
233+
'package.json': pkg,
234+
'app/page.js': page('export default function H() { return html`<main>home</main>`; }'),
235+
'app/not-found.js':
236+
`export default function NF() { throw new Error('STILL_BROKEN_NF'); }\n`,
237+
});
238+
const app = await createRequestHandler({ appDir, dev: true });
239+
const prev = console.error;
240+
console.error = () => {};
241+
try {
242+
await app.handle(new Request('http://x/gone'));
243+
const held = app.getLastDevError();
244+
assert.ok(held, 'the first, real navigation retained a frame');
245+
assert.match(String(held.url), /\/gone$/);
246+
247+
// Now hover a link to it. Same url, still broken, speculative.
248+
await app.handle(new Request('http://x/gone', { headers: { 'x-webjs-prefetch': '1' } }));
249+
} finally { console.error = prev; }
250+
const after = app.getLastDevError();
251+
assert.ok(after, 'the frame survives a prefetch that failed again');
252+
assert.match(String(after.url), /\/gone$/);
253+
});

0 commit comments

Comments
 (0)