fix(app-router): hard-navigate to browser URL on non-ok RSC fetch

[NathanDrake2406]

What this changes

When a client-side RSC navigation fetch returns a non-ok response (404, 500, or any other HTTP error), the client now immediately hard-navigates to the browser-facing URL instead of trying to parse the error body as an RSC stream.

The same guard is added to the initial hydration fallback fetch in readInitialRscStream: a non-ok response triggers window.location.reload() so the server can render the correct error page as HTML.

Why

Without this fix, a non-ok RSC fetch (e.g. a server returning an HTML error page with status 500) is passed directly to createFromFetch. The RSC parser tries to read HTML as an RSC stream, throws a cryptic "Connection closed" error, and the outer catch logs "[vinext] RSC navigation error: ..." then hard-navigates to the same URL, which can loop.

The fix matches Next.js behavior from packages/next/src/client/components/router-reducer/fetch-server-response.ts:211:

if (!isFlightResponse || !res.ok || !res.body) {
  return doMpaNavigation(responseUrl.toString())
}

Approach

In the navigation path (inside navigateRsc): added !navResponse.ok check after the stale-navigation guard. On non-ok: settle pending router state, clear pending pathname, and window.location.href = href (the browser URL without .rsc suffix). The stale-navigation check at navId !== activeNavigationId runs first so superseded navigations never trigger a hard-nav to a stale URL.

In readInitialRscStream: added !rscResponse.ok check after the fallback fetch. On non-ok: window.location.reload() and return a never-resolving stream (so the caller does not proceed into createFromReadableStream during the brief window before the reload takes effect).

No changes to the prefetch path -- prefetchRscResponse already discards non-ok responses before storing snapshots.

Validation

New E2E tests in tests/e2e/app-router/rsc-fetch-errors.spec.ts:

• RSC navigation to a non-existent route (returns 404 RSC, but with !ok) hard-navs to the non-.rsc URL
• RSC navigation with a Playwright route intercept returning HTTP 500 HTML hard-navs without logging a stream-parse error
• The final URL after hard-nav does not contain .rsc

All 3 new tests pass. Existing navigation, regression, and error-handling E2E test suites pass (1 pre-existing flaky test on retry, unrelated to this change).

Risks / follow-ups

The readInitialRscStream non-ok path uses window.location.reload(), which will loop if the server consistently returns non-ok for the page's RSC endpoint. This is the same behavior as a hard browser refresh on a broken server and is not worse than the pre-fix behavior (opaque parse failure).

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@875

commit: 46454b5

[Copilot]

Pull request overview

Updates the App Router client to immediately fall back to an MPA navigation when an RSC fetch returns a non-OK HTTP status, avoiding attempts to parse non-RSC (e.g. HTML error pages) as an RSC stream.

Changes:

• Add a !navResponse.ok guard in client-side RSC navigation to hard-navigate to the browser URL on non-OK responses.
• Add a !rscResponse.ok guard during initial hydration fallback RSC fetch to reload the page on non-OK responses.
• Add Playwright E2E coverage for 404/500 non-OK RSC fetch scenarios to ensure the final URL is the non-.rsc destination and no stream-parse error is logged.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/e2e/app-router/rsc-fetch-errors.spec.ts	Adds E2E tests validating hard-navigation behavior on non-OK RSC navigation fetches.
packages/vinext/src/server/app-browser-entry.ts	Adds non-OK response guards for navigation RSC fetches and initial hydration fallback RSC fetches.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test title says “without looping”, but the assertions only verify the final URL and absence of specific console errors. This could still pass even if the page immediately reloads/redirects again after landing on /about. Consider adding an explicit assertion that the URL remains stable for a short period (or that the hard navigation only happens once) to actually cover the “no loop” behavior.

[NathanDrake2406]

Addressed the review comment. Two changes:

1. Tightened the 500-route test to actually verify "no looping". After the hard navigation settles, the test waits 1.5s, then asserts the URL is stable and the /about.rsc intercept fired at most 3 times. A runaway reload loop would fail the hit-count assertion immediately.

2. Fixed a real bug the tightened test exposed. In the prior patch, readInitialRscStream would window.location.reload() on a non-ok RSC response. That assumes the next HTTP response will differ, which is not guaranteed: if the RSC endpoint is persistently broken while the HTML document is served successfully (upstream gating, mismatched handler, intercepted requests in tests), the reload refetches the same HTML, the post-hydration RSC fetch gets the same non-ok, and the page reloads forever. Added a sessionStorage-keyed attempt guard: first non-ok sets the key and reloads, second non-ok for the same path clears the key and throws, letting the outer bootstrap catch surface the error instead of triggering another reload. A successful RSC response clears the key.

Both changes in 6071901.

[james-elicx]

/bigbonk review

[ask-bonk]

Nice fix, and good match with Next.js' fetch-server-response.ts behavior (!isFlightResponse || !res.ok || !res.body → doMpaNavigation). The follow-up commit tightening the 500-route test and adding the reload-loop guard in readInitialRscStream is a solid improvement over v1.

A few things worth addressing before merge:

1. Behavioral parity: does the RSC content-type still need to be checked? Next.js guards on !isFlightResponse || !res.ok || !res.body. This PR only adds !res.ok on the nav path. That's defensible (HTML error bodies are usually !ok), but an RSC response with content-type text/html and status 200 (e.g. a proxy strips/rewrites the content-type) would still be fed into createFromFetch and throw the cryptic parse error this PR was meant to fix. Worth at least a follow-up issue to also check content-type.startsWith('text/x-component') and navResponse.body != null before parsing.

2. sessionStorage can throw. In strict-mode iframes, Safari private browsing on older versions, or with storage blocked, sessionStorage.getItem/setItem throws SecurityError. The initial-hydration path is the one place you really don't want to throw before the app boots. Wrap the storage access in try/catch so storage-denied environments degrade to "reload once per fetch and hope for the best", not "crash hydration".

3. Stale reload-guard key across paths. The guard is cleared on line 870 only on a successful RSC fetch for the current path. If the reload happens but the user navigates elsewhere (back button, link in the inline HTML) before hydration re-runs readInitialRscStream, the key remains set for the original path. Re-visiting that path later will trip the "after reload" branch on the very first failure and throw instead of attempting a recovery reload. Minor, but you could either clear the key on any successful readInitialRscStream entry (embed path included) or include a timestamp so stale keys expire.

4. Unhandled rejection on the "after reload" throw. main() is called via void main() with no top-level catch. Throwing from readInitialRscStream produces an unhandledrejection that surfaces as a bare red error in devtools and may trigger window.onerror / reporting hooks. The comment says "the outer bootstrap can fall back to showing the server-rendered HTML" — in practice there is no bootstrap catch; the SSR'd HTML stays visible by accident (hydration just never runs). Either add a .catch(err => console.error(...)) to the void main() call, or console.error then return instead of throwing. The latter is closer to what the comment describes.

5. Test: the 500-route spec is good, but aboutRscHits <= 3 is loose. With the new guard, the expected trajectory is exactly 2 hits (one from the client nav, one from the post-reload initial fetch which then aborts). <= 3 tolerates an extra reload that shouldn't happen. Consider tightening to <= 2 so a future regression that adds one unnecessary reload is caught.

6. Test leaks console listener across tests. page.on('console', ...) is registered per-test but page is per-test in Playwright, so this is fine in practice. Ignore.

Overall looks good — happy to approve once (2) is addressed; the rest are follow-ups.

[ask-bonk]

sessionStorage.getItem / setItem can throw SecurityError (strict-mode iframes, storage-disabled browsers, some Safari private-browsing configurations). If that happens here the initial hydration crashes before the app boots, which is worse than the bug this PR fixes.

Suggest wrapping the storage calls:

Suggested change

	return new ReadableStream<Uint8Array>();
	if (!rscResponse.ok) {
	const reloadKey = "__vinext_rsc_initial_reload__";
	const currentPath = window.location.pathname + window.location.search;
	let priorAttempt: string | null = null;
	try {
	priorAttempt = sessionStorage.getItem(reloadKey);
	} catch {
	// sessionStorage can throw in strict iframes / private-mode Safari.
	// Fall through to the "no prior attempt" branch.
	}
	if (priorAttempt === currentPath) {
	try {
	sessionStorage.removeItem(reloadKey);
	} catch {}
	throw new Error(
	`[vinext] Initial RSC fetch returned ${rscResponse.status} after reload; aborting hydration`,
	);
	}
	try {
	sessionStorage.setItem(reloadKey, currentPath);
	} catch {}
	window.location.reload();
	// Return a never-resolving stream so the caller does not proceed into
	// createFromReadableStream before the reload takes effect.
	return new ReadableStream<Uint8Array>();
	}
	// Clear the reload guard on success so a subsequent navigation to the same
	// path is not wrongly flagged as a looped reload.
	try {
	sessionStorage.removeItem("__vinext_rsc_initial_reload__");
	} catch {}

[ask-bonk]

Throwing here becomes an unhandled promise rejection because main() is invoked as void main() at line 1326 with no top-level catch. The resulting red error in devtools plus any window.onerror/error reporting hook fire even though the comment says "the outer bootstrap can fall back to showing the server-rendered HTML" — in practice there is no bootstrap catch and the SSR'd HTML stays visible only by accident (hydration just never starts).

Consider one of:

• Replace throw with console.error(...) + return new ReadableStream() so the failure is reported without triggering unhandledrejection.
• Add a .catch(err => console.error('[vinext] hydration aborted:', err)) to the void main() call at line 1326.

The first option matches the intent of the comment most closely.

[ask-bonk]

Parity note (non-blocking): Next.js' equivalent guard (fetch-server-response.ts in canary) is !isFlightResponse || !res.ok || !res.body. This PR only covers !res.ok.

The !res.body case is mostly theoretical, but !isFlightResponse covers a real scenario: a 200 response from a proxy/CDN that stripped or rewrote the Content-Type header (e.g. returned text/html instead of text/x-component). That would still be passed to createFromFetch and throw the exact stream-parse error this PR was added to fix.

Worth a follow-up to also check the content type before parsing:

const ct = navResponse.headers.get("content-type") ?? "";
const isFlight = ct.startsWith("text/x-component");
if (!navResponse.ok || !isFlight || !navResponse.body) {
  window.location.href = currentHref;
  return;
}

[ask-bonk]

Stale-key edge case: this only clears the key on the success path for the current path. If the post-reload hydration navigates elsewhere before re-entering readInitialRscStream (e.g. the SSR'd HTML contains a link the user clicks, or the back button), the key remains set for the original path. A later return to that path will skip the recovery-reload on the first failure and throw immediately.

Consider clearing the key unconditionally on any successful entry into readInitialRscStream (including the embedded-RSC branch at line 807), or storing a timestamp alongside the path so stale keys expire after e.g. 60 seconds.

[ask-bonk]

With the guard in place the expected hit count is exactly 2 (one from the client RSC nav fetch that triggers the hard-nav, one from the post-reload initial RSC fetch that then aborts via the sessionStorage guard). <= 3 leaves room for one extra reload that shouldn't happen. Tightening to <= 2 would catch future regressions that add one unnecessary reload.

Suggested change

	msg.includes("RSC navigation error") ||
	expect(aboutRscHits).toBeLessThanOrEqual(2);

[ask-bonk]

Review posted successfully at #875 (review).

Summary

Posted a batched review with 5 inline comments covering:

1. (blocking) sessionStorage can throw in strict iframes / Safari private mode — crashes hydration before the app boots. Suggested try/catch wrapper.
2. (follow-up) Unhandled rejection on the "after reload" throw — void main() has no top-level catch, so the thrown error becomes an unhandledrejection. Suggested console.error + return instead.
3. (follow-up) Missing content-type check — Next.js guards on !isFlightResponse || !res.ok || !res.body; this PR only covers !res.ok. A 200 response from a proxy that strips the RSC content-type would still hit the parse error.
4. (nit) Stale reload-guard key across paths — key only clears on success for the current path, not cleared if the user navigates elsewhere first.
5. (nit) Test aboutRscHits <= 3 is loose — expected is exactly 2; tightening catches future regressions.

Overall the fix correctly matches Next.js' fetch-server-response.ts behavior (confirmed by reading canary source), and the follow-up commit adding the reload-loop guard is a meaningful improvement. Main blocker is the sessionStorage try/catch.

github run

[NathanDrake2406]

All five review points addressed in 97d2942.

1. sessionStorage SecurityError (line 866). Wrapped every sessionStorage.getItem / setItem / removeItem in a try { ... } catch { } via three small helpers (readReloadFlag / writeReloadFlag / clearReloadFlag). A throw in strict-mode iframes or Safari private mode now falls through to the "no prior attempt" branch instead of crashing hydration.

2. Unhandled promise rejection (line 860). Replaced the throw after a looped reload with console.error plus a never-resolving ReadableStream. void main() has no outer catch, so the throw would have surfaced as unhandledrejection and fired window.onerror hooks. The never-resolving stream halts hydration cleanly — SSR'd HTML stays visible, client components simply don't hydrate, matching what the comment originally promised.

3. Content-type parity with Next.js (line 1142). Next.js guards on !isFlightResponse || !res.ok || !res.body. The previous commit only covered !res.ok. Added Content-Type starts-with-text/x-component check on both the client-side nav path (line 1181 area) and the initial-hydration path (readInitialRscStream). A proxy/CDN that returns 200 with a rewritten text/html content-type now triggers the hard-nav / recovery reload instead of reaching createFromFetch and throwing the exact stream-parse error these guards exist to prevent.

4. Stale reload key (line 870). Added clearReloadFlag() at the top of the embedded-RSC branch in readInitialRscStream. The embed branch only runs when the server successfully rendered the page, so any prior flag is stale. Without this clear, the flag could survive a client-side navigation and a later hard-reload of the same path would skip the one legitimate recovery attempt.

5. Hit count tightening (line 119). <= 3 → <= 2. Expected sequence is exactly two hits (triggering client RSC nav fetch + one post-reload fetch that aborts); any extra hit now fails the test.

Also extracted the recovery logic into a recoverFromBadInitialRscResponse(reason) helper so the !ok and !isFlight branches share one code path with descriptive logging.

[james-elicx]

/bigbonk review

[ask-bonk]

The fix matches Next.js' !isFlightResponse || !res.ok || !res.body guard from fetch-server-response.ts, and the follow-ups from the prior review are all addressed — sessionStorage is wrapped in try/catch, the content-type check is added on both paths, the reload flag is cleared on the embedded-RSC branch, the throw-after-reload is replaced with console.error + never-resolving stream, and the test hit count is tightened to <= 2.

One substantive concern about the "after reload" abort path below — after recoverFromBadInitialRscResponse returns a never-resolving stream, main() continues to hydrateRoot(...) with a suspended promise, which still sets window.__VINEXT_RSC_ROOT__, window.__VINEXT_HYDRATED_AT, and window.__VINEXT_RSC_NAVIGATE__. To any external observer (including waitForAppRouterHydration and user interactions), the app looks hydrated, but every router click will suspend forever. Worth considering whether main() should bail before hydrateRoot on this path.

The rest are small nits / test-comment cleanups, not blockers.

[ask-bonk]

Subtle side effect worth flagging: recoverFromBadInitialRscResponse returns a never-resolving stream, so main() will resolve past await readInitialRscStream() and continue on to hydrateRoot(document, ..., { initialElements: <promise that never resolves> }) at line 1023. React will suspend hydration indefinitely (which is fine — SSR'd HTML stays visible, matching the comment), but main() still:

• assigns window.__VINEXT_RSC_ROOT__
• sets window.__VINEXT_HYDRATED_AT = performance.now()
• registers window.__VINEXT_RSC_NAVIGATE__

So to external probes (waitForAppRouterHydration, analytics, Sentry's hydration-timing hooks, user clicks on Links), the page looks hydrated. Any subsequent __VINEXT_RSC_NAVIGATE__ call will then fire an RSC fetch that succeeds but renders into a root that is still suspended on the initial elements — navigation will also hang.

Consider early-returning from main() on this path so __VINEXT_RSC_NAVIGATE__ is never registered and the page is an obvious no-interaction fallback:

const rscStream = await readInitialRscStream();
if (rscStream === HYDRATION_ABORTED_SENTINEL) return;

or have recoverFromBadInitialRscResponse resolve a sentinel that the caller checks. This keeps the "SSR'd HTML stays visible" guarantee without leaving a half-hydrated global surface that misleads observers.

[ask-bonk]

Nit: the first-attempt branch silently calls reload() with no diagnostic. When this fires in production it will look like a spurious page reload with no clue in the console about why. A one-line console.warn before writeReloadFlag(...) would make the behavior traceable without being noisy (it only happens once per broken path):

Suggested change

	window.location.reload();
	writeReloadFlag(currentPath);
	console.warn(
	`[vinext] Initial RSC fetch ${reason}; reloading once to let the server render the HTML error page`,
	);
	window.location.reload();
	// Never-resolving stream so the caller does not proceed into
	// createFromReadableStream before the reload takes effect.
	return new ReadableStream<Uint8Array>();

[ask-bonk]

Minor: currentHref is the pre-redirect URL captured at navigation start. If a server-side redirect has already updated history via replaceHistoryStateWithoutNotify earlier in this while (true) loop (the redirect branch at ~L1194-1210), the redirect loop writes history to the new path but the outer currentHref variable has also been updated on L1205, so this should be correct. Flagging it only so a future refactor that breaks the redirect-loop invariant (e.g. not updating currentHref after a redirect) doesn't silently send users to the wrong hard-nav destination. A comment pinning the invariant ("currentHref is kept in sync with history across redirect hops above, so hard-nav on !ok is safe") would help.

[ask-bonk]

The comment says "exactly two hits" but the assertion is <= 2, and the actual count is either 1 or 2 depending on whether the home-page <Link href="/about"> prefetch races with the __VINEXT_RSC_NAVIGATE__ call. The current assertion is correct; it's the comment that needs a tweak.

Suggested change

	// fetch that triggered the hard-nav, and one from the post-reload
	// Expected trajectory: up to two hits — one from the home-page Link
	// prefetch of /about.rsc (which is discarded on !ok), and one from the
	// client RSC nav fetch that triggers the hard-nav. If hydration timing
	// races, the prefetch may not fire and the count is 1. After the hard
	// navigation to /about, the embedded-RSC branch in readInitialRscStream
	// handles hydration without a fallback .rsc fetch, so no post-reload
	// hits occur. A runaway reload loop would produce many more.
	expect(aboutRscHits).toBeLessThanOrEqual(2);

[ask-bonk]

Nit: "Unexpected token" is broad enough to match any JSON.parse failure anywhere in the page (including unrelated third-party code, e.g. a prefetched fixture that logs a JSON parse diagnostic). Tightening to co-occurrence with an RSC-specific marker avoids false positives:

Suggested change

	);
	function isRscStreamParseError(msg: string): boolean {
	return (
	msg.includes("createFromFetch") ||
	msg.includes("createFromReadableStream") ||
	msg.includes("Failed to parse RSC") ||
	(msg.includes("Connection closed") && msg.includes("RSC")) ||
	(msg.includes("Unexpected token") && msg.includes("RSC"))
	);
	}

Given the PR's pre-fix failure mode is [vinext] RSC navigation error: Error: Connection closed, the Connection closed strand is the important one — and that already contains RSC via the [vinext] prefix context in the real error path.

[ask-bonk]

Review posted successfully at #875 (review).

Summary

Posted a batched review with 5 inline comments acknowledging that the previous round's feedback was all addressed (sessionStorage try/catch, content-type parity, reload-loop guard, throw → console.error, tightened hit count), and raising:

1. (substantive) Half-hydrated global surface after reload abort — After recoverFromBadInitialRscResponse returns a never-resolving stream, main() still runs past await readInitialRscStream() and registers __VINEXT_RSC_ROOT__, __VINEXT_HYDRATED_AT, and __VINEXT_RSC_NAVIGATE__. To observers the page looks hydrated, but any nav click suspends forever. Suggested an early-return sentinel.

2. (nit) Silent first-attempt reload — The "try once" branch calls reload() with no diagnostic. A one-line console.warn makes production behavior traceable.

3. (nit) Redirect-loop / currentHref invariant — The new !ok guard reads currentHref, which is kept in sync with history across redirect hops. Suggested a pinning comment so a future refactor doesn't break the invariant.

4. (test comment) "Exactly two hits" mismatch with <= 2 — Actual count is 1 or 2 depending on prefetch timing; suggested rewording.

5. (test nit) "Unexpected token" is too broad — Could false-positive on unrelated JSON parse errors; suggested tightening the filter to co-occur with an RSC-specific marker.

No blockers — the fix correctly mirrors Next.js' fetch-server-response.ts behavior.

github run

[NathanDrake2406]

All five new points addressed in 23796b6.

1. Half-hydrated-state side effect (line 849 / abort path). Right — the never-resolving stream let main() continue past the await and still assign __VINEXT_RSC_ROOT__, __VINEXT_HYDRATED_AT, and __VINEXT_RSC_NAVIGATE__. External probes thought the page was hydrated, user clicks on Links wired up to __VINEXT_RSC_NAVIGATE__, and every subsequent navigation rendered into a root that was suspended on a never-resolving initial-elements promise — so every nav hung. Fix: recoverFromBadInitialRscResponse now returns null on the abort path (second attempt). readInitialRscStream's return type changed to ReadableStream | null, and main() early-returns on null — no hydration globals get registered, SSR'd HTML stays the definitive state.

2. Silent reload diagnostic (line 846). Added a one-shot console.warn before window.location.reload() naming the failure reason. The sessionStorage flag keeps it from repeating — one line in the console per broken path, not noisy.

3. currentHref redirect-loop invariant (line 1189). Added a comment pinning the invariant: currentHref is the browser-facing URL without .rsc, kept in sync with history across redirect hops by the redirect branch below (which reassigns currentHref = destinationPath before continue). Flagged so a future refactor that breaks it doesn't silently misroute users.

4. Test comment vs <= assertion (line 123). Rewrote the comment to match the actual bound: up to two hits (home-page Link prefetch of /about.rsc + the client nav fetch), possibly 1 if hydration timing races the prefetch. After the hard-nav the embedded-RSC branch handles hydration without a fallback fetch, so no post-reload hits occur.

5. "Unexpected token" too broad (line 41). Tightened: generic strings (Connection closed, Unexpected token) now require an RSC-context co-marker ("RSC" or "vinext" in the message). createFromFetch / createFromReadableStream / Failed to parse RSC stay unconditional since they are already specific. A benign third-party JSON.parse diagnostic can no longer false-positive.

[james-elicx]

/bigbonk review

[ask-bonk]

The fix is correct and matches Next.js' !isFlightResponse || !res.ok || !res.body guard from fetch-server-response.ts (verified against canary). The review iteration has been thorough — sessionStorage is guarded, content-type parity is covered on both the nav and the initial-hydration paths, the reload-loop guard in readInitialRscStream is bounded and clears the flag on every successful embed-path entry, and the null-sentinel early return from main() cleanly avoids the half-hydrated global surface. Prefetch path is also safe: prefetchRscResponse only snapshots when response.ok, and snapshotRscResponse preserves the original content-type, so a 200-with-text/html response that is later consumed via consumePrefetchResponse will still be rejected by the new content-type check at the nav site.

A few non-blocking follow-ups worth considering — none of them need to hold this PR:

1. First-attempt still registers hydration globals before reload() fires. When recoverFromBadInitialRscResponse returns a never-resolving stream (first attempt), main() falls through to assign window.__VINEXT_RSC_ROOT__, __VINEXT_HYDRATED_AT, and __VINEXT_RSC_NAVIGATE__ in the tiny window before window.location.reload() unloads the page. In practice this is harmless (the page is about to unload and hydrateRoot suspends on the never-resolving initial-elements promise), but the comment on the null branch explicitly says "we must not assign __VINEXT_RSC_ROOT__ / __VINEXT_RSC_NAVIGATE__" as the rationale — that invariant only holds on the second attempt today. If you want symmetry, the first-attempt branch could return null too and let main() early-exit before the reload fires from within the helper (the reload is scheduled before the return either way).

2. isPageUnloading parity with Next.js. Next.js tracks a pagehide/pageshow-gated isPageUnloading flag and suppresses console.error for fetch errors during unload (fetch-server-response.ts:345). Vinext's outer catch at line 1310 (console.error("[vinext] RSC navigation error:", error)) will fire during the unloading interval after this PR's hard-nav on line 1206 if any other in-flight RSC fetch is aborted by the browser. Not introduced by this PR, but now more noticeable because hard-nav is the happy path for a broken response. Worth filing as a follow-up.

3. Test: the third test is largely redundant. "navigation to non-existent route does not land on the .rsc URL" duplicates the URL-suffix assertion from the first test. Could be folded in, or kept as a focused smoke-check — minor.

4. Test flakiness: waitForTimeout(1500) is a timer-based stability check. On a slow CI runner, a legitimate slow-path reload could land outside the 1.5s window and still fail the test on count. If you see flakes, consider replacing with page.waitForLoadState('networkidle') after the hard-nav plus a short settle, which is less sensitive to absolute timing.

Approving the approach — nothing above is a blocker.

[ask-bonk]

Symmetry nit: the comment on the null branch (line 1018-1023) says we must not assign the __VINEXT_RSC_* globals because doing so leaves a half-hydrated surface. That rationale also applies during the brief window between window.location.reload() firing here and the page actually unloading. In practice it is harmless (the never-resolving stream keeps hydrateRoot suspended, no user can click anything before unload), but for invariant symmetry both branches could return null and rely on main()'s early exit:

Suggested change

	return new ReadableStream<Uint8Array>();
	writeReloadFlag(currentPath);
	// One-shot diagnostic so a production reload is traceable. Only fires once
	// per broken path thanks to the sessionStorage flag above; not noisy.
	console.warn(
	`[vinext] Initial RSC fetch ${reason}; reloading once to let the server render the HTML error page`,
	);
	window.location.reload();
	// Return null so main() early-exits immediately rather than registering
	// hydration globals in the tiny window before the reload unloads the page.
	// hydrateRoot on a never-resolving stream would suspend anyway, so no
	// interaction is possible, but bypassing the globals keeps the invariant
	// ("broken RSC response => no __VINEXT_RSC_* registered") uniform across
	// both attempts.
	return null;

Non-blocking — pointing it out only because the comment on line 1018-1023 describes this as an invariant.

[ask-bonk]

Follow-up suggestion (not for this PR): Next.js' equivalent call site at fetch-server-response.ts tracks a pagehide/pageshow-gated isPageUnloading flag and suppresses console.error for fetch errors during unload. After this PR, hard-nav via window.location.href = currentHref is the expected happy path for non-ok/non-RSC responses, which increases the surface where in-flight RSC fetches can be aborted by the browser during unload and surface as errors through the outer catch at line 1310.

A future change could adopt the same isPageUnloading pattern around that console.error to avoid noisy diagnostics in the expected path. Not a blocker — calling it out so it can be filed as a follow-up.

[ask-bonk]

Minor: waitForTimeout(1500) is a fixed-duration stability check. On a slow CI runner, a legitimate late reload could land just outside the 1.5s window and satisfy the URL assertion while still representing broken behavior, or a transient pause in Playwright could make a healthy hit arrive after the timer. Consider replacing with page.waitForLoadState('networkidle') (optionally with a short waitForTimeout(200) after) — that tracks actual request activity instead of a wall-clock window and is less sensitive to CI timing.

Non-blocking; leaving it in is fine and the aboutRscHits <= 2 assertion provides the real bound.

[ask-bonk]

Nit: this third test is largely redundant with the first (page.url() already equals /this-route-does-not-exist there, which transitively satisfies "no .rsc suffix"). Could be folded into the first test as an additional assertion, or deleted, to keep the suite tight. Not a blocker.

[ask-bonk]

Review posted successfully at #875 (review).

Summary

Posted a batched review with 4 inline comments. Overall assessment: the fix is correct and matches Next.js' !isFlightResponse || !res.ok || !res.body guard from fetch-server-response.ts (verified against canary source). Prior review rounds were thorough and all blocking concerns have been addressed.

Non-blocking observations raised:

1. Invariant symmetry on the first-attempt path — recoverFromBadInitialRscResponse returning a never-resolving stream still lets main() register __VINEXT_RSC_ROOT__ / __VINEXT_RSC_NAVIGATE__ in the tiny pre-unload window. Practically harmless (hydration suspends on the never-resolving stream, no interaction possible before unload), but the null-branch comment explicitly calls this out as an invariant. Returning null from both branches would unify behavior.
2. isPageUnloading parity — Next.js suppresses console.error on fetch errors during unload. Vinext's outer catch at line 1310 will fire for aborted in-flight RSC fetches after the hard-nav. Not introduced by this PR, but more visible now. Good follow-up candidate.
3. Test redundancy — third test duplicates the first test's URL assertion.
4. Test timing — waitForTimeout(1500) is a wall-clock stability check that could be tightened with waitForLoadState('networkidle').

Confirmed prefetch safety: prefetchRscResponse only snapshots when response.ok, and snapshotRscResponse preserves content-type, so a 200-with-text/html cached prefetch will still be rejected by the new nav-site content-type check.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Fix is correct and matches Next.js' fetch-server-response.ts guard (!isFlightResponse || !res.ok || !res.body → doMpaNavigation). The iteration across prior review rounds covered the hard concerns: sessionStorage is guarded against SecurityError, the content-type check is mirrored on both the nav and initial-hydration paths, the reload-loop guard is bounded and cleared on every successful embed-path entry, and main() early-exits on null so no hydration globals leak onto the window in the unload window.

One substantive finding worth addressing before merge, plus two small nits:

1. (substantive) isPageUnloading is never reset on pageshow — diverges from Next.js and silences real errors after bfcache restore. Next.js' equivalent registers handlers on both pagehide and pageshow; vinext only has pagehide. When the user navigates away and returns via the back/forward cache, the JS execution context is restored with isPageUnloading === true, so the outer catch at line 1317 silently swallows every subsequent RSC navigation error for the lifetime of that tab. Fix is one listener.

2. (nit) First-attempt branch in recoverFromBadInitialRscResponse leaves hydration globals in place for the pre-unload window. Not a new concern — acknowledged in the PR thread — but since I was pointing out #1 anyway, reiterating here with a suggestion that unifies the branches.

3. (nit) Test comment vs assertion bound. Minor readability.

Prefetch path is confirmed safe: prefetchRscResponse only snapshots when response.ok, and snapshotRscResponse preserves content-type, so a cached 200-with-text/html response is still rejected by the new nav-site content-type guard.

[ask-bonk]

Substantive: isPageUnloading is set on pagehide but never reset on pageshow. When the browser restores this document from bfcache (back/forward cache), the JS execution context is resumed with isPageUnloading === true, and the outer navigation catch at line 1317 will silently drop every subsequent error — real errors, not just unload-aborts. Next.js explicitly pairs the two handlers for this exact reason (fetch-server-response.ts, canary):

window.addEventListener('pagehide', () => { isPageUnloading = true })
window.addEventListener('pageshow', () => { isPageUnloading = false })

vinext also documents the pattern on line 1316 as "Mirrors Next.js' isPageUnloading pattern", so the divergence is not intentional. Fix:

Suggested change

	});
	window.addEventListener("pagehide", () => {
	isPageUnloading = true;
	});
	// Reset on pageshow so a bfcache-restored document does not resume with
	// the flag stuck at true, which would silently swallow every subsequent
	// RSC navigation error for the lifetime of that tab. Matches Next.js'
	// fetch-server-response.ts handler pair.
	window.addEventListener("pageshow", () => {
	isPageUnloading = false;
	});
	void main();

User-visible symptom without this: after a user navigates away from a vinext app and hits Back, any genuine RSC navigation failure (stream-parse error, 500 from backend, etc.) produces a silent hard-nav with no [vinext] RSC navigation error: diagnostic in the console, making production debugging significantly harder.

[ask-bonk]

Invariant-symmetry nit (non-blocking): the comment on lines 836-839 says both branches return null "so main() aborts the hydration bootstrap without registering __VINEXT_RSC_* globals — including during the brief window between reload() firing and the page actually unloading". Today that is strictly true: both branches do return null and main() exits on line 1026, so no globals are registered in either case. Good.

However, the current code ordering (reload() then return null) relies on main()'s synchronous early-return to protect the invariant. If a future refactor inlines or restructures recoverFromBadInitialRscResponse — e.g. a caller that checks for a sentinel and does fallback rendering instead of early-exit — the first-attempt branch silently becomes half-hydrated during the unload window, whereas the second-attempt branch remains safe.

Calling the reload from inside the helper before the return already gives the right behavior; one way to make the invariant locally obvious would be to factor the reload schedule and the return null together so both branches read identically at the call site. Current code is fine — flagging only because the comment calls this out as a load-bearing invariant.

[ask-bonk]

Nit: the console listener is registered after page.goto(/about) and waitForAppRouterHydration, so any RSC stream-parse error fired during the initial /about hydration would not be captured. For this test the navigation target is /this-route-does-not-exist, so it does not matter in practice, but the symmetry with the second test (which has the same pattern) is a bit accidental. Consider registering the listener before page.goto and filtering by page.url() if you want to scope — or leave as-is and add a note that initial-page errors are out of scope for this spec.

[ask-bonk]

Nit: the comment above says "up to two hits" and the assertion is <= 2, which is correct. But it's worth noting that with the current recoverFromBadInitialRscResponse behavior, the post-reload initial-fetch is also intercepted (500), triggering the after reload; aborting hydration branch — which does not fire another .rsc request. So the bound is genuinely tight.

However, if a future refactor moves the initial RSC fetch behind any retry or revalidation logic, the <= 2 bound could silently stop catching the regression this test was designed for. Consider adding a lower bound too:

Suggested change

	expect(aboutRscHits).toBeLessThanOrEqual(2);
	// Lower bound: at minimum, the client nav fetch that triggers the
	// hard-nav must have fired. A value of 0 would mean the navigation
	// skipped the RSC fetch entirely and the test is no longer exercising
	// the !ok-guard path.
	expect(aboutRscHits).toBeGreaterThanOrEqual(1);
	expect(aboutRscHits).toBeLessThanOrEqual(2);

[ask-bonk]

Review posted successfully at #875 (review).

Summary

The fix is correct and matches Next.js's !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts (verified against canary source). Prior review rounds were thorough — sessionStorage guarded, content-type parity on both paths, bounded reload loop, null-sentinel early return.

Posted a batched review with 4 inline comments:

1. (substantive, blocking-ish) Missing pageshow handler for isPageUnloading. The code adds isPageUnloading = true on pagehide but never resets it. Next.js pairs pagehide/pageshow specifically to handle bfcache restore. Without the reset, after a user navigates away and hits Back, the flag stays true and every subsequent RSC navigation error is silently swallowed for the rest of that tab's lifetime. The code comment even says "Mirrors Next.js' isPageUnloading pattern", so this is an unintentional divergence. Verified against packages/next/src/client/components/router-reducer/fetch-server-response.ts on canary.

2. (nit) Invariant-symmetry comment on recoverFromBadInitialRscResponse: the "no globals registered" guarantee holds today but relies on main()'s synchronous early-return pattern.

3. (nit) Test console listener registered after page.goto, misses errors during initial page hydration. Not actually broken for these tests but pattern-worth-flagging.

4. (nit) Suggest adding a >= 1 lower bound to the hit-count assertion so a future regression that skips the RSC fetch entirely still fails the test.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Reviewed the current state after all iterations. The fix is correct and matches Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts. Prior review rounds covered the major concerns thoroughly:

• sessionStorage access guarded against SecurityError
• Content-type parity (text/x-component) on both nav and initial-hydration paths
• Bounded reload-loop guard with fresh-attempt clear on successful embed-path entry
• null-sentinel early return from main() so no __VINEXT_RSC_* globals leak during the reload window
• pagehide/pageshow pair correctly restores isPageUnloading after bfcache restore
• console.error suppression during unload mirrors Next.js

Verified against Next.js canary packages/next/src/client/components/router-reducer/fetch-server-response.ts — the behavior is equivalent.

Prefetch path is safe: prefetchRscResponse only snapshots when response.ok (shims/navigation.ts:448), and snapshotRscResponse preserves the original content-type (shims/navigation.ts:389), so a 200-with-text/html response that reached the prefetch cache (via status-ok-but-wrong-ct) will still be rejected by the new content-type check at the nav site.

One substantive non-blocking finding below about the hard-nav target URL differing from Next.js on redirect-to-error chains, plus two small test nits. None are blockers — approving the approach.

[ask-bonk]

Non-blocking parity observation: Next.js' equivalent path hard-navigates to the response URL (the post-redirect destination), not the original request URL:

// fetch-server-response.ts, canary
if (!isFlightResponse || !res.ok || !res.body) {
  // ...
  return doMpaNavigation(responseUrl.toString())  // uses res.url, not the original
}

This PR uses window.location.href = currentHref, which is the original request URL. For the common case (no server-side redirect) these are the same and behavior matches. For a redirect chain that lands on a non-ok response — e.g. /old-page → 307 → /new-page → 500 — the browser's fetch follows the redirect, so navResponse.url is /new-page.rsc, but vinext hard-navs back to /old-page. The server then re-issues the 307 and the user lands on /new-page showing the HTML error page, which is correct eventually, but:

• The user briefly sees the URL bar flash /old-page before the redirect follows
• Any analytics/error-reporting that keys off URL sees the redirect-source URL, not the failing destination
• One extra HTTP round-trip (the redirect replay)

The fix is to strip the .rsc suffix from the response URL when available:

Suggested change

	window.location.href = currentHref;
	const navContentType = navResponse.headers.get("content-type") ?? "";
	const isRscResponse = navContentType.startsWith("text/x-component");
	if (!navResponse.ok || !isRscResponse || !navResponse.body) {
	// Prefer the response URL (post-redirect) so we don't bounce the
	// user off the original request URL just to re-follow the same
	// redirect; matches Next.js' doMpaNavigation(responseUrl.toString()).
	// Strip the .rsc suffix so the browser lands on the user-facing URL.
	const responseUrl = navResponseUrl ?? navResponse.url;
	const hardNavTarget = responseUrl
	? new URL(responseUrl, window.location.origin).pathname.replace(/\.rsc$/, "") +
	new URL(responseUrl, window.location.origin).search
	: currentHref;
	window.location.href = hardNavTarget;
	return;
	}

Filing as a follow-up is fine — it's an edge case and the current behavior is not broken, just slightly wasteful.

[ask-bonk]

Nit: the **/about.rsc** glob matches /about.rsc, /about.rsc?foo=1, and — because of the trailing ** — anything that happens to contain /about.rsc as a substring. The intercept stays active for the lifetime of the page, including after the hard-nav lands on /about. If the /about page has any <Link href="/about?..."> self-links or prefetches that re-emit /about.rsc, those hits would count toward aboutRscHits and could push the assertion over <= 2.

In practice the test fixture probably doesn't have self-prefetches on /about, but tightening the glob to anchor-match would make the test robust against future fixture changes:

Suggested change

	await page.route("**/about.rsc**", (route) => {
	await page.route(/\/about\.rsc(\?|$)/, (route) => {

Not a blocker; flagging because the <= 2 bound is the load-bearing assertion for the "no loop" claim.

[ask-bonk]

Minor: the intercept returns contentType: "text/html", which is important because it exercises the content-type guard in addition to the !ok guard. Worth a comment so a future maintainer tightening the body to "" or swapping to text/x-component doesn't silently stop exercising the combined guard path:

Suggested change

	});
	return route.fulfill({
	status: 500,
	// The combination (status 500, text/html body) exercises both the
	// !ok guard and the content-type guard at the nav site. Preserve
	// both when editing — a 500 with a valid RSC content-type would
	// only hit the !ok path, and a 200 with text/html would only hit
	// the content-type path.
	contentType: "text/html",
	body: "<html><body><h1>Internal Server Error</h1></body></html>",
	});

[ask-bonk]

Nit (not a blocker, and earlier reviews touched this): the first-attempt branch schedules window.location.reload() then returns null. The comment on the function header (lines 836-839) describes the null return as the mechanism that prevents the half-hydrated-globals invariant violation. That invariant holds only because main() happens to be synchronous after await readInitialRscStream() — there's no microtask yield between the null check at line 1025 and the global-assign lines below. If a future refactor interposes any async work (e.g. an await for React preloading) between those two points, the first-attempt branch starts leaking globals during the pre-unload window.

One way to pin the invariant at the call site is to hoist the if (rscStream === null) return; check to immediately after await readInitialRscStream() (it already is — good) and add a comment there referencing the synchronous-tail requirement. Or, more robustly, push the global-assignment into a helper that the null-branch cannot reach.

Fine as-is; flagging because the invariant is load-bearing and currently only enforced by code-placement convention.

[ask-bonk]

Review posted successfully at #875 (review).

Summary

The fix is correct and matches Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts (verified against canary). Prior review rounds were thorough — sessionStorage is guarded, content-type parity on both paths, bounded reload-loop guard, null-sentinel early return, pagehide/pageshow pair.

Posted a batched review with 4 inline comments — no blockers:

1. (substantive, non-blocking) Hard-nav target URL divergence from Next.js. Next.js uses doMpaNavigation(responseUrl.toString()) — the response URL after redirects. This PR uses currentHref — the original request URL. On redirect-to-error chains (/old → 307 → /new → 500), vinext hard-navs back to /old and forces the server to re-follow the redirect. User sees the URL briefly flash the source, analytics key off the wrong URL, and an extra HTTP round-trip occurs. Suggested using navResponseUrl ?? navResponse.url with .rsc stripped.

2. (test nit) The **/about.rsc** glob is substring-match; any post-hard-nav self-prefetch from /about would count toward aboutRscHits and could push over <= 2. Suggested a regex bound.

3. (test nit) The contentType: "text/html" on the intercept is load-bearing — it exercises the content-type guard in addition to the !ok guard. Worth a comment so future maintainers don't unknowingly drop the dual-guard coverage.

4. (invariant nit) recoverFromBadInitialRscResponse's null-return invariant ("no __VINEXT_RSC_* globals registered") only holds because main() is synchronous after await readInitialRscStream(). Any future await interposed between the null check and the global-assignments silently breaks the invariant. Worth a comment pin or a helper.

Prefetch safety confirmed: prefetchRscResponse only snapshots when response.ok, and snapshotRscResponse preserves content-type, so a status-200-but-wrong-ct prefetch still gets rejected by the nav-site content-type check.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Reviewed the current state. The fix is correct and matches Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts. Prior review rounds have been thorough and every substantive concern has been addressed:

• sessionStorage access guarded against SecurityError via the three helper functions
• Content-type parity (text/x-component) applied to both the nav path and the initial-hydration path
• Bounded reload-loop guard with fresh-attempt clear on successful embed-path entry
• null-sentinel early return from main(), with bootstrapHydration extracted as a separate synchronous helper so the null branch structurally cannot reach the global assignments
• pagehide/pageshow pair correctly resets isPageUnloading after bfcache restore
• Hard-nav target uses the post-redirect navResponse.url with .rsc stripped, matching Next.js' doMpaNavigation(responseUrl.toString())
• Test uses waitForLoadState("networkidle") + upper/lower bounds on hit count, /\/about\.rsc(\?|$)/ regex for anchored matching

Two small non-blocking observations below — neither needs to hold this PR.

Prefetch path is safe: prefetchRscResponse only snapshots when response.ok, and snapshotRscResponse preserves the original content-type, so a 200-with-text/html response that reached the prefetch cache is still rejected by the new content-type check at the nav site.

[ask-bonk]

Parity nit (non-blocking): the client-side nav path at line 1212 guards on all three conditions Next.js checks — !navResponse.ok || !isRscResponse || !navResponse.body. The initial-hydration path only checks !ok and content-type; there is no !rscResponse.body branch.

The !body case is rare in practice (a 200 with valid text/x-component content-type but a null body) but it is specifically called out in Next.js' guard and is the cheapest possible addition here for exact parity:

Suggested change

	const contentType = rscResponse.headers.get("content-type") ?? "";
	if (!rscResponse.ok) {
	return recoverFromBadInitialRscResponse(`returned ${rscResponse.status}`);
	}
	// Guard against proxies/CDNs that return 200 with a rewritten Content-Type
	// (e.g. text/html instead of text/x-component). Such responses cannot be
	// parsed as RSC and would throw the same opaque parse error this fallback
	// exists to prevent.
	const contentType = rscResponse.headers.get("content-type") ?? "";
	if (!contentType.startsWith("text/x-component")) {
	return recoverFromBadInitialRscResponse(
	`returned non-RSC content-type "${contentType || "(missing)"}"`,
	);
	}
	// Missing body (e.g. 204 No Content, or an edge worker that returned ok
	// headers without piping the stream) would fail the same way downstream.
	// Matches Next.js' `!res.body` branch in fetch-server-response.ts.
	if (!rscResponse.body) {
	return recoverFromBadInitialRscResponse("returned empty body");
	}

[ask-bonk]

Minor trailing-slash edge case (non-blocking): toRscUrl at shims/navigation.ts:267 strips any trailing slash before appending .rsc, so /foo/ becomes /foo.rsc. Stripping .rsc here with /\.rsc$/ produces /foo, not /foo/.

For sites configured with trailingSlash: true, the hard-nav target then differs from the user's original href by one character, and the server will issue an extra 308 to the canonical /foo/ form. The user-visible effect is one additional HTTP round-trip on the error path and a brief URL flash in the address bar, which mirrors the redirect-chain concern the navResponseUrl ?? navResponse.url change was meant to avoid.

Not worth blocking on — the error path is already the slow path, and most apps do not use trailingSlash: true. Worth filing as a follow-up, or restoring the trailing slash when currentHref had one to avoid the extra hop.

[ask-bonk]

Nit: this test is largely redundant with the first test — the first test already asserts page.url() ends without .rsc (toBe("${BASE}/this-route-does-not-exist")), which transitively covers the same invariant. Consider folding this assertion into the first test and dropping the third test() block to keep the suite tight, or adding a comment that explains what this third test uniquely exercises.

Non-blocker — a small bit of focused coverage is also fine if it documents the URL-shape invariant explicitly.

[ask-bonk]

Review posted successfully at #875 (review).

Summary

Posted a batched review with 3 inline comments acknowledging the thorough iteration and surfacing 3 non-blocking observations:

1. (parity nit) Missing !rscResponse.body check in readInitialRscStream. The client-side nav path guards on all three of Next.js' conditions (!ok || !isRscResponse || !body), but the initial-hydration path only guards on !ok + content-type. Rare in practice (200 with valid content-type and null body), but it's the final piece of exact parity with fetch-server-response.ts.

2. (trailing-slash nit) /\.rsc$/ strip loses trailing slash. toRscUrl normalizes /foo/ → /foo.rsc, and stripping gives /foo. For sites with trailingSlash: true, the hard-nav target is off-by-one-char and triggers an extra 308 round-trip on the error path. Mirrors the redirect-chain concern the navResponseUrl ?? navResponse.url change addressed.

3. (test nit) Third test is largely redundant with the first test's URL-suffix assertion. Suggested folding in or adding a comment explaining what's uniquely exercised.

Overall assessment: the fix is correct, matches Next.js behavior exactly (verified against canary fetch-server-response.ts), and prior review rounds thoroughly covered sessionStorage guarding, content-type parity, bounded reload-loop guard, null-sentinel return, pagehide/pageshow pairing, and redirect-chain response-URL handling. No blockers.

github run

[james-elicx]

/bigbonk re-review for issues

[ask-bonk]

Fix matches Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts, and the iteration across prior review rounds is thorough — sessionStorage is guarded, content-type parity is on both paths, the reload-loop guard is bounded and cleared on success, bootstrapHydration is factored out as a synchronous helper so the null-branch structurally cannot leak __VINEXT_RSC_* globals, pagehide/pageshow pair handles bfcache, isPageUnloading suppresses unload-aborted fetch errors, and the hard-nav target prefers navResponseUrl ?? navResponse.url with trailing-slash preservation. Prefetch path is confirmed safe (snapshot only on response.ok, content-type preserved).

A few small non-blocking observations below — none are blockers.

[ask-bonk]

Hash fragment is dropped on hard-nav: hardNavTarget = pathname + parsed.search builds the target from the response URL, which — for a .rsc fetch — does not carry the browser's hash fragment. If the user clicked a <Link href="/foo#section"> that 500s, the hard-nav lands on /foo and silently loses the #section anchor.

Next.js' doMpaNavigation(responseUrl.toString()) has the same limitation (the response URL never carries a fragment), so this is parity with upstream — not a regression. Worth preserving the fragment from currentHref as a small improvement over Next.js, or at least a code comment flagging the divergence between the user's clicked href and the hard-nav target:

Suggested change

	hardNavTarget = pathname + parsed.search;
	hardNavTarget = pathname + parsed.search;
	// Preserve the hash from the user's clicked href — the response
	// URL from a .rsc fetch never carries a fragment, so dropping it
	// here would silently strip `/foo#section` to `/foo`. Next.js'
	// doMpaNavigation has the same limitation; this is a small
	// ergonomic win over upstream.
	const origHash = new URL(currentHref, window.location.origin).hash;
	if (origHash) hardNavTarget += origHash;

[ask-bonk]

Test coverage gap (non-blocking): the new redirect-aware hard-nav logic — preferring navResponseUrl ?? navResponse.url over currentHref so a chain like /old → 307 → /new → 500 hard-navs to /new rather than /old — has no test in rsc-fetch-errors.spec.ts. Both existing tests exercise direct 404/500 responses without redirect hops, so the responseUrl branch and the original-vs-response URL divergence it guards against are only covered by code review.

Worth adding a test with a Playwright route intercept that returns 307 Location to a second intercepted 500 endpoint, asserting the final URL is the 307 target and the address bar never flashes the source URL. Fine to defer as a follow-up; flagging because this was called out in the last review round as the scenario motivating the switch from currentHref.

[ask-bonk]

Minor: the !navResponse.body branch here is reachable after await fetch(...) has resolved, at which point Response.body is a ReadableStream | null but is null only for responses with no body (e.g. 204, or a HEAD request). For an actual HTTP 200/500 with an empty body, body is a non-null empty stream, not null. The practical impact is that an edge worker that returns ok headers with a 0-byte body would still be fed to createFromFetch and throw downstream — matching the behavior Next.js' !res.body check is supposed to prevent.

Not a blocker — Next.js' guard has the same theoretical gap, and the content-type check catches most of these in practice. Noting only for future reference.

[ask-bonk]

waitForLoadState("networkidle") is the right call here, but note the interaction with the persistent /\/about\.rsc(\?|$)/ intercept: after the hard-nav to /about, if the server happens to not embed RSC in the HTML (e.g. a future change to the embedding strategy), the fallback fetch in readInitialRscStream hits the intercepted 500, triggers the reload-flag path, reloads the page, and on the second hit returns null — producing brief network activity that delays networkidle beyond the default 30s timeout only if the reload itself re-issues long-running requests.

Current fixture does embed RSC via __VINEXT_RSC__, so this is fine today. If the embedding path ever becomes conditional, this test's timing could become flaky without an obvious failure mode. Worth a code comment pinning the assumption, or an explicit aboutRscHits snapshot before networkidle and after, so a regression in the embed path shows up as a count change.

[ask-bonk]

Review posted successfully at #875 (review).

Summary

The fix is correct and matches Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts. Prior review rounds have been thorough — sessionStorage guarded, content-type parity on both paths, bounded reload-loop guard, bootstrapHydration factored out so the null-branch structurally can't leak globals, pagehide/pageshow handles bfcache, isPageUnloading suppresses unload noise, hard-nav prefers post-redirect navResponseUrl with trailing-slash preservation. Prefetch safety confirmed.

Posted a batched review with 4 non-blocking inline comments:

1. (small ergonomic) Hash fragment dropped on hard-nav. hardNavTarget = pathname + parsed.search loses #section from currentHref. Next.js has the same gap, so this is parity — but preserving the fragment would be a small win over upstream.
2. (test coverage gap) The new navResponseUrl ?? navResponse.url redirect-aware logic (the /old → 307 → /new → 500 scenario the last round motivated) has no test — both existing tests exercise direct non-ok responses without redirect hops.
3. (theoretical parity gap) !navResponse.body after await fetch() is null only for 204-style no-body responses; a 0-byte 200 would still be fed to createFromFetch. Matches Next.js' same gap.
4. (test timing interaction) The persistent /about.rsc intercept + waitForLoadState("networkidle") combination depends on the fixture's embedded-RSC branch firing on reload. Fine today; worth a comment pinning the assumption.

No blockers.

github run

[james-elicx]

/bigbonk re-review for issues

[ask-bonk]

Reviewed the current state. The fix matches Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts, and this iteration (since the last /bigbonk round) addressed the remaining parity gaps flagged in round 6: !rscResponse.body is now guarded in readInitialRscStream, trailing-slash preservation covers trailingSlash: true sites, hash fragments survive the hard-nav, a redirect-chain test now exercises the navResponseUrl ?? navResponse.url branch, and isPageUnloading is cleaned up on both pagehide and pageshow.

One small dead-code observation (file-level since it's outside the diff), and a couple of non-blocking edge-case notes. No blockers.

File-level observation: dead throw at line 941-943. The pre-existing if (!rscResponse.body) throw new Error("[vinext] Initial RSC response had no body") at line 941-943 is now unreachable because the new !rscResponse.body guard at line 921-923 returns before it. Not broken, just stale — worth removing in a follow-up so a future reader doesn't assume the throw is load-bearing and add another layer of logic around it.

Prefetch safety confirmed: prefetchRscResponse only snapshots when response.ok, and snapshotRscResponse preserves content-type, so a cached 200-with-text/html response is still rejected by the content-type guard at the nav site.

[ask-bonk]

Non-blocking parity observation: isPageUnloading is set globally on pagehide, so this suppression applies to any error surfacing through the outer catch during the unload window — not just fetch aborts triggered by the unload itself. A racing RSC fetch that genuinely fails (server crash, stream-parse error) and happens to complete between pagehide and the document actually tearing down will now be silently swallowed. In practice the page is seconds from being gone, and Next.js has identical behavior, so this is parity — not a regression.

Worth flagging only because this slightly broadens the suppression surface vs the pre-fix behavior where every such error was logged. Noting for future debugging — if a user reports "my server threw but I see nothing in the console when I click a link", this is the code path that suppresses it.

[ask-bonk]

The redirect-chain test relies on two Playwright route handlers firing in sequence: the 307 intercept on /redirect-src.rsc, then the fetch() following the redirect with redirect: "follow" (the default) and Playwright re-entering the route table for /about.rsc. This works today, but the dependency on Playwright's redirect-intercept re-dispatch is subtle — a future test migration to a different mocking layer (MSW browser, a service-worker fetch shim) could silently stop exercising the chain if that layer follows redirects transparently without re-entering the route table, at which point the test would assert against the real backend for /about.rsc and the navResponseUrl ?? navResponse.url invariant would stop being covered.

One-line comment pinning the assumption would help:

Suggested change

	});
	// Intercept chain requires Playwright to re-enter the route table on the
	// 307 follow-up so both handlers fire in sequence (fetch defaults to
	// redirect:follow). Migrating to a mocker that follows redirects without
	// re-dispatching would silently skip the /about.rsc intercept and the
	// test would fall through to the real backend.
	await page.route(/\/redirect-src\.rsc(\?|$)/, (route) => {

[ask-bonk]

Nit: the redirect-chain test doesn't assert the user-visible invariant it was added to protect — that the URL bar never flashes /redirect-src en route to /about. page.waitForURL(/about) + final expect(page.url()).toBe(.../about) together only confirm the final state; an intermediate reflection of /redirect-src in history (which would happen if a future refactor dropped the navResponseUrl ?? navResponse.url branch and hard-nav'd to currentHref) would still pass because the server's subsequent 307 → /about converges to the same final URL.

A lightweight add would capture the URL timeline via framenavigated and assert the sequence never includes /redirect-src:

Suggested change

	expect(page.url()).toBe(`${BASE}/about`);
	const frameUrls: string[] = [];
	page.on("framenavigated", (frame) => {
	if (frame === page.mainFrame()) frameUrls.push(frame.url());
	});
	const navigationPromise = page.waitForURL(`${BASE}/about`, { timeout: 10_000 });
	await page.evaluate(() => {
	void (window as any).__VINEXT_RSC_NAVIGATE__("/redirect-src");
	});
	await navigationPromise;
	expect(page.url()).toBe(`${BASE}/about`);
	// The document URL must never flash /redirect-src — the hard-nav target
	// is the post-redirect /about, not the original clicked href. Without
	// this assertion a regression that dropped `navResponseUrl ?? navResponse.url`
	// would still pass (the server's 307 converges to /about eventually).
	expect(frameUrls.some((url) => url.includes("/redirect-src"))).toBe(false);
	expect(srcRscHits).toBeGreaterThanOrEqual(1);
	expect(aboutRscHits).toBeGreaterThanOrEqual(1);

Non-blocking.

[ask-bonk]

Review posted successfully at #875 (review).

Summary

Posted a batched review acknowledging that this iteration addressed the prior round's parity gaps (!rscResponse.body guard, trailing-slash preservation, hash-fragment preservation, redirect-chain test, pagehide/pageshow pairing) and verified against Next.js canary fetch-server-response.ts.

3 inline comments, all non-blocking:

1. (parity observation) isPageUnloading suppression is global — broadens suppression surface beyond just unload-aborted fetches, so a racing RSC fetch that genuinely fails during the unload window is now silently swallowed. Matches Next.js behavior, so parity not regression, but worth flagging for future debugging.

2. (test robustness) Redirect-chain test relies on Playwright re-dispatching through the route table on the 307 follow-up — works today with browser fetch default redirect: follow, but a future test-mocker migration could silently stop exercising the chain. Suggested a one-line comment pinning the assumption.

3. (test coverage nit) Redirect-chain test doesn't assert the user-visible invariant it protects — the final URL converges to /about even under a regression that dropped navResponseUrl ?? navResponse.url, because the server's 307 would re-route. Suggested a framenavigated listener asserting /redirect-src never appears in the URL timeline.

Also flagged at PR-body level: the pre-existing throw new Error("[vinext] Initial RSC response had no body") at line 941-943 is now dead code (unreachable after the new !rscResponse.body guard at 921-923). File-level only since those lines are outside the diff.

Prefetch safety confirmed.

github run

[james-elicx]

/bigbonk re-review for issues

[ask-bonk]

Reviewed the final state. The fix correctly mirrors Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts and every substantive concern from the prior review rounds has been resolved:

• sessionStorage access guarded against SecurityError via helper wrappers
• Content-type parity (text/x-component) on both the nav path and the initial-hydration path
• Bounded reload-loop guard with fresh clear on successful embed-path entry
• bootstrapHydration factored out so the null-sentinel branch structurally cannot reach __VINEXT_RSC_* global assignments, even if a future refactor interposes async work
• pagehide/pageshow pair handles bfcache restore
• isPageUnloading suppresses noisy diagnostics from unload-aborted fetches
• Hard-nav target uses navResponseUrl ?? navResponse.url with .rsc stripped, trailing slash preserved, and hash fragment preserved from the user's original href
• Redirect-chain test asserts the URL bar never flashes the source URL via framenavigated

Prefetch safety confirmed: prefetchRscResponse at shims/navigation.ts:448 only snapshots when response.ok, and snapshotRscResponse preserves the original content-type (line 389), so a 200-with-text/html that somehow reached the prefetch cache is still rejected by the nav-site content-type guard.

A couple of small non-blocking observations below — none are blockers. Two of them are residual edge cases, and one is a dead-code cleanup suggestion for the same file (outside the diff).

[ask-bonk]

Edge case worth flagging (non-blocking): when sessionStorage.setItem persistently throws — the exact scenario the try/catch wrappers were added to handle — the reload-loop guard silently disengages. readReloadFlag() returns null on every reload (get either throws or returns a value that was never persisted), the else branch fires, writeReloadFlag() swallows the throw, and window.location.reload() runs again. On a persistently broken .rsc endpoint in a storage-denied environment (strict-mode iframe, certain enterprise-locked browser policies), this is an infinite reload loop rather than the intended "reload once and fall back to HTML".

The behavior is strictly better than the pre-PR opaque parse failure in the common case, and likely very rare in practice, but the PR body notes "the same behavior as a hard browser refresh on a broken server" — which is accurate for the non-storage-denied path only. A module-level let priorBadRscPath: string | null = null; in-memory fallback (consulted when readReloadFlag() returns null) would close the loop across environments without relying on storage.

Worth filing as a follow-up, not a blocker.

[ask-bonk]

Minor invariant observation (non-blocking): this guard fires after if (navId !== activeNavigationId) return; on line 1191, but the value of currentHref at that point reflects any server-side redirects that happened on earlier iterations of this while (true) loop (the redirect branch at line 1246-1258 reassigns it). So the fallback hardNavTarget = currentHref on line 1216 — used when responseUrl is falsy (empty string) — would point to the final destination of the redirect chain rather than the user's original clicked href. That is the right behavior for the common case of same-loop-iteration, but worth noting that currentHref is not the user's original clicked href on redirect chains: it tracks the latest history entry.

In combination with the responseUrl preference on line 1215, this is correct today. Flagging only because the invariant (currentHref kept in sync with history across redirect hops, so hard-nav on !ok is safe) is load-bearing and currently only maintained by code-placement convention in the redirect branch on line 1253. A future refactor that moves redirect handling into a helper but forgets to reassign currentHref in the caller would silently misroute users on the error path.

[ask-bonk]

Coverage nit (non-blocking): this redirect-chain test fulfills the 307 with Location: ${BASE}/about.rsc but does not exercise the trailing-slash preservation branch (line 1225-1227 in app-browser-entry.ts). The new logic in that branch fires when currentHref has a trailing slash that toRscUrl stripped before the round-trip; the branch is untested by this suite.

A targeted way to cover it: add an assertion that a client nav to a path like /about/ on a trailingSlash: true fixture (if one exists) hard-navs to /about/ and not /about. Since the project's trailingSlash story may not have a dedicated fixture today, this is reasonable to defer as a follow-up — the current tests cover the dominant error-path scenario.

[ask-bonk]

Micro-nit: the new URL(currentHref, window.location.origin) is parsed twice — once on line 1224 to extract origPathname, and again on line 1232 to extract origHash. Not a perf concern (URL parsing is fast), just a readability win to hoist const origUrl = new URL(currentHref, window.location.origin); once:

Suggested change

	const origHash = new URL(currentHref, window.location.origin).hash;
	const origUrl = new URL(currentHref, window.location.origin);
	if (origUrl.pathname.length > 1 && origUrl.pathname.endsWith("/") && !pathname.endsWith("/")) {
	pathname += "/";
	}
	hardNavTarget = pathname + parsed.search;
	// Preserve the hash from the user's clicked href — a .rsc response
	// URL never carries a fragment, so dropping it would silently strip
	// `/foo#section` down to `/foo`.
	if (origUrl.hash) hardNavTarget += origUrl.hash;

Non-blocking.

[ask-bonk]

Review posted successfully at #875 (review).

Summary

The fix is correct and matches Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts. After 8 review rounds, every substantive concern has been addressed: sessionStorage guarded, content-type parity on both paths, bounded reload-loop guard, bootstrapHydration factored out so the null-branch structurally cannot leak globals, pagehide/pageshow pair for bfcache, isPageUnloading suppression, hard-nav target uses post-redirect URL with trailing-slash + hash-fragment preservation, and the redirect-chain test uses framenavigated to assert no URL flash.

Posted 4 non-blocking comments — all small residual observations:

1. (edge case) Storage-denied environments (strict iframes, some enterprise policies) — if sessionStorage.setItem persistently throws, the reload-loop guard silently disengages and can loop indefinitely. Module-level in-memory fallback would close this.
2. (invariant) currentHref as the fallback hard-nav target when responseUrl is falsy reflects post-redirect state, not the user's original clicked href — load-bearing invariant currently maintained only by code-placement convention in the redirect branch.
3. (test coverage nit) Trailing-slash preservation branch (line 1225-1227) has no E2E coverage — would require a trailingSlash: true fixture.
4. (micro-nit) new URL(currentHref, ...) parsed twice in the hard-nav target builder — hoist once for readability.

No blockers.

github run

[james-elicx]

/bigbonk re-review for issues

[ask-bonk]

Reviewed the final state. The fix correctly mirrors Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts, and nine rounds of prior review have addressed every substantive concern:

• sessionStorage access guarded against SecurityError via the three helper wrappers, plus a post-write verification that aborts instead of looping in storage-denied environments (closes the infinite-reload edge case raised in the prior round)
• Content-type + !body parity (text/x-component startsWith) on both the nav path and the initial-hydration path
• Bounded reload-loop guard with fresh clear on successful embed-path entry
• bootstrapHydration factored out as a synchronous helper so the null-sentinel branch structurally cannot reach __VINEXT_RSC_* global assignments
• pagehide/pageshow pair handles bfcache restore correctly
• isPageUnloading suppresses unload-aborted fetch diagnostics
• Hard-nav target prefers post-redirect navResponseUrl ?? navResponse.url with trailing-slash and hash-fragment preservation
• Redirect-chain test asserts URL bar never flashes the source URL via framenavigated

Prefetch safety confirmed: prefetchRscResponse only snapshots when response.ok, and snapshotRscResponse preserves content-type, so a cached 200-with-text/html is still rejected by the nav-site content-type guard.

All CI checks are green. Three small non-blocking observations below — all optional follow-ups.

[ask-bonk]

Dead code follow-up (non-blocking): the pre-existing if (!rscResponse.body) throw new Error("[vinext] Initial RSC response had no body") that used to live a few lines below this branch was removed in this PR (the throw is gone in the diff), but worth a quick scan of the file to confirm no other stale check references the old invariant. The new guard here converts the throw into a structured recovery path, which is the right shape.

[ask-bonk]

Minor invariant note (non-blocking): the !navResponse.body branch fires after await fetch(...) resolves, at which point Response.body is null only for responses explicitly without a body (e.g. 204 No Content, HEAD, or an edge worker that closed the stream before piping). A genuine HTTP 200/500 with a zero-byte body still produces a non-null empty ReadableStream, so this branch won't catch that case — it will be fed to createFromFetch and throw downstream.

Matches Next.js' same theoretical gap in fetch-server-response.ts, and the content-type check catches most of these in practice. Not worth changing; noting only for future reference if someone investigates why a 0-byte RSC response still produces an opaque parse error.

[ask-bonk]

Minor test nit (non-blocking): hitsBeforeNetworkIdle + aboutRscHits === hitsBeforeNetworkIdle is a nice tight embedded-RSC pin. One subtle edge: if Playwright's waitForLoadState("networkidle") returns synchronously because the network was already idle at the call site, the two snapshots are trivially equal and the assertion passes without actually exercising the embed path under network pressure.

Not a real concern for this fix since the reload-loop regression would produce hits regardless of timing, but if a future variant of this test ever wants to prove "the embed path actually ran", consider asserting against a fixture-provided hydration marker (e.g. a body attribute set only by the embed branch) rather than inferring from request counts.

[ask-bonk]

Review posted successfully at #875 (review).

Summary

Verified the final state of this thoroughly-iterated PR against the checked-out code. The fix correctly mirrors Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts, and all CI checks are green.

Every substantive concern from the nine prior review rounds has been addressed:

• sessionStorage SecurityError guard + post-write verification that closes the storage-denied infinite-loop edge case from the previous round
• Content-type + !body parity on both the nav and initial-hydration paths
• bootstrapHydration extracted as a synchronous helper so the null-sentinel branch structurally can't leak hydration globals
• pagehide/pageshow pair correctly handles bfcache
• Hard-nav target uses post-redirect URL with trailing-slash and hash-fragment preservation
• Redirect-chain test asserts the URL bar never flashes the source URL via framenavigated

Posted three small non-blocking inline comments:

1. Dead-code sanity check (prior throw removed in diff — confirm no stale refs)
2. !navResponse.body catches null body but not 0-byte empty streams — same gap as Next.js, noted for future debugging
3. Test's networkidle comparison trivially passes if network was already idle; suggested a fixture-provided hydration marker if a future test wants to genuinely prove the embed path ran

No blockers.

github run

[NathanDrake2406]

@james-elicx do we need to change approval permission for a new model?

[james-elicx]

It should be fine

[james-elicx]

/bigbonk re-review for blocking issues or approve

[ask-bonk]

Final-state review. The fix correctly mirrors Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts, and ten-plus rounds of iteration have resolved every substantive concern: sessionStorage guarded + post-write verification that aborts rather than loops in storage-denied envs, content-type + !body parity on both nav and initial-hydration paths, bootstrapHydration factored out so the null-sentinel branch structurally cannot register __VINEXT_RSC_* globals, pagehide/pageshow pair for bfcache, isPageUnloading suppression, hard-nav target uses post-redirect URL with trailing-slash + hash-fragment preservation, and the redirect-chain test asserts URL timeline via framenavigated.

Prefetch safety confirmed: prefetchRscResponse snapshots only when response.ok, and snapshotRscResponse preserves content-type, so a 200-with-text/html cached prefetch is still rejected at the nav-site content-type guard.

All CI checks are green.

Three small non-blocking observations below.

[ask-bonk]

Timing gap (non-blocking): the console listener is registered after page.goto(${BASE}/) and waitForAppRouterHydration, so any RSC stream-parse error fired during the home page's initial hydration is not captured. This matters for the 500 test specifically: the home page's <Link href="/about"> triggers a prefetch of /about.rsc that hits the intercepted 500. The prefetch path is !ok-gated and discards cleanly (confirmed), but if a future regression ever routed a prefetch through createFromFetch (or a parse error fired inside prefetchRscResponse before the !ok branch), this test would silently miss it.

Moving the listener registration before page.goto would close the gap:

Suggested change

	});
	const consoleErrors: string[] = [];
	page.on("console", (msg) => {
	if (msg.type() === "error") {
	consoleErrors.push(msg.text());
	}
	});
	await page.goto(`${BASE}/`);
	await waitForAppRouterHydration(page);

Not a blocker — the prefetch path's current !ok-gate makes this hypothetical today.

[ask-bonk]

Nit (non-blocking): hasRscContext = msg.includes("RSC") || msg.includes("vinext") correctly catches the PR body's documented failure mode ([vinext] RSC navigation error: Error: Connection closed) because the outer console.error prefix lands in msg.text(). However, a raw SyntaxError: Unexpected token '<' in JSON at position 0 fired from deep inside the RSC parser on the initial-hydration path (not wrapped by the [vinext] prefix) would not contain RSC or vinext and would be filtered out here. The createFromFetch / createFromReadableStream / Failed to parse RSC strands cover most variants, so this is a small coverage edge rather than a real gap. Worth a thought if the test starts missing a regression in the initial-hydration path specifically.

[ask-bonk]

Observation (non-blocking, outside the diff's scope): fetch(toRscUrl(...)) on line 916 can reject on a true network error (DNS failure, offline, connection reset before any response bytes). That rejection propagates out of readInitialRscStream, out of main, and surfaces as an unhandled promise rejection since void main() has no .catch. The non-ok path now has a clean recovery, but a rejection is still a bare red console error + window.onerror / error-reporting hooks firing.

One-line fix (consider as a follow-up, not for this PR):

void main().catch((err) => {
  console.error("[vinext] hydration aborted:", err);
});

Not in this PR's scope, but now that every documented response shape has a structured recovery, the network-error path is the last remaining case that produces the same user-visible symptom this PR was added to eliminate.

[ask-bonk]

Review posted successfully at #875 (review).

Summary

Posted a batched review confirming the final state of this heavily-iterated PR. The fix correctly mirrors Next.js' !isFlightResponse || !res.ok || !res.body → doMpaNavigation guard from fetch-server-response.ts, and every substantive concern from the prior 10+ review rounds has been resolved:

• sessionStorage guarded against SecurityError + post-write verification that aborts rather than loops in storage-denied environments
• Content-type + !body parity on both nav and initial-hydration paths
• bootstrapHydration factored out so the null-sentinel branch structurally cannot register __VINEXT_RSC_* globals
• pagehide/pageshow pair for bfcache
• isPageUnloading suppression
• Hard-nav target uses post-redirect URL with trailing-slash + hash-fragment preservation
• Redirect-chain test asserts URL timeline via framenavigated

Three non-blocking observations raised:

1. (test timing) console listener registered after page.goto/waitForAppRouterHydration in the 500 test — misses any stream-parse errors fired during the home page's initial hydration (including the /about.rsc prefetch). Current prefetch !ok-gate makes this hypothetical today.
2. (test filter nit) isRscStreamParseError gates generic strings on "RSC"/"vinext" context markers. A raw SyntaxError: Unexpected token '<' from deep in the RSC parser without the [vinext] prefix would be filtered out — small coverage edge.
3. (outside-diff follow-up) fetch() network errors still surface as unhandled promise rejections since void main() has no .catch. Now that every documented response shape has a structured recovery, this is the last remaining case producing the pre-fix symptom.

No blockers.

github run