diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index 808b5d8ca..7019c89b7 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -801,10 +801,84 @@ function restorePopstateScrollPosition(state: unknown): void { }); } -async function readInitialRscStream(): Promise> { +// Set on pagehide so the RSC navigation catch block can distinguish expected +// fetch aborts (triggered by the unload itself) from real errors worth logging. +let isPageUnloading = false; + +const RSC_RELOAD_KEY = "__vinext_rsc_initial_reload__"; + +// sessionStorage can throw SecurityError in strict-mode iframes, storage- +// disabled browsers, and some Safari private-browsing configurations. Wrap +// every access so a recovery path for one error does not crash hydration. +function readReloadFlag(): string | null { + try { + return sessionStorage.getItem(RSC_RELOAD_KEY); + } catch { + return null; + } +} +function writeReloadFlag(path: string): void { + try { + sessionStorage.setItem(RSC_RELOAD_KEY, path); + } catch {} +} +function clearReloadFlag(): void { + try { + sessionStorage.removeItem(RSC_RELOAD_KEY); + } catch {} +} + +// A non-ok or wrong-content-type RSC response during initial hydration means +// the server cannot deliver a valid RSC payload for this URL. Parsing the +// response as RSC causes an opaque parse failure. On the first attempt, +// reload once so the server has a chance to render the correct error page +// as HTML. On the second attempt (detected via the sessionStorage flag), the +// endpoint is persistently broken. Returns 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 — so external probes never see a half-hydrated page. +function recoverFromBadInitialRscResponse(reason: string): null { + const currentPath = window.location.pathname + window.location.search; + if (readReloadFlag() === currentPath) { + clearReloadFlag(); + console.error( + `[vinext] Initial RSC fetch ${reason} after reload; aborting hydration. ` + + "Server-rendered HTML remains visible; client components will not hydrate.", + ); + return null; + } + writeReloadFlag(currentPath); + // Verify the write persisted. In storage-denied environments (strict-mode + // iframes, locked-down enterprise policies), every getItem returns null and + // every setItem silently no-ops, so the reload-loop guard cannot survive + // the reload — the page would loop forever. Abort instead so the user at + // least sees the server-rendered HTML. + if (readReloadFlag() !== currentPath) { + console.error( + `[vinext] Initial RSC fetch ${reason}; sessionStorage unavailable so the ` + + "reload-loop guard cannot persist — aborting hydration. " + + "Server-rendered HTML remains visible; client components will not hydrate.", + ); + return null; + } + // 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; +} + +async function readInitialRscStream(): Promise | null> { const vinext = getVinextBrowserGlobal(); if (vinext.__VINEXT_RSC__ || vinext.__VINEXT_RSC_CHUNKS__ || vinext.__VINEXT_RSC_DONE__) { + // Reaching the embedded-RSC branch means the server successfully rendered + // the page — any prior reload flag for this path is stale and must be + // cleared so a future failure gets its own fresh recovery attempt. + clearReloadFlag(); + if (vinext.__VINEXT_RSC__) { const embedData = vinext.__VINEXT_RSC__; delete vinext.__VINEXT_RSC__; @@ -841,6 +915,29 @@ async function readInitialRscStream(): Promise> { const rscResponse = await fetch(toRscUrl(window.location.pathname + window.location.search)); + 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) fails the same way downstream. + // Matches Next.js' `!res.body` branch in fetch-server-response.ts. + if (!rscResponse.body) { + return recoverFromBadInitialRscResponse("returned empty body"); + } + // Successful RSC response clears the guard so a subsequent reload of the + // same path after a transient failure still gets one recovery attempt. + clearReloadFlag(); + let params: Record = {}; const paramsHeader = rscResponse.headers.get("X-Vinext-Params"); if (paramsHeader) { @@ -854,10 +951,6 @@ async function readInitialRscStream(): Promise> { restoreHydrationNavigationContext(window.location.pathname, window.location.search, params); - if (!rscResponse.body) { - throw new Error("[vinext] Initial RSC response had no body"); - } - return rscResponse.body; } @@ -939,6 +1032,16 @@ async function main(): Promise { registerServerActionCallback(); const rscStream = await readInitialRscStream(); + // null signals that readInitialRscStream aborted hydration — either because + // a reload is in flight (first-attempt recovery) or the endpoint is + // persistently broken (post-reload). Bootstrap is a separate synchronous + // helper so the null-branch structurally cannot reach any __VINEXT_RSC_* + // global assignment, even if a future refactor interposes async work here. + if (rscStream === null) return; + bootstrapHydration(rscStream); +} + +function bootstrapHydration(rscStream: ReadableStream): void { const root = normalizeAppElementsPromise(createFromReadableStream(rscStream)); const initialNavigationSnapshot = createClientNavigationRenderSnapshot( window.location.href, @@ -1100,6 +1203,55 @@ async function main(): Promise { if (navId !== activeNavigationId) return; + // Any response that isn't a valid RSC payload (non-ok status, + // missing/rewritten Content-Type, or missing body) means the server + // returned something we cannot parse — typically an HTML error page + // or a proxy-rewritten response. Parsing such a body as an RSC stream + // throws a cryptic "Connection closed" error. Match Next.js behavior + // (fetch-server-response.ts:211, `!isFlightResponse || !res.ok || !res.body`): + // hard-navigate to the response URL so the server can render the correct + // error page as HTML. The outer finally handles + // settlePendingBrowserRouterState and clearPendingPathname on this + // return path. + // + // Prefer the post-redirect response URL over `currentHref`: on a + // redirect chain like `/old` → 307 → `/new` → 500, the browser's + // fetch already followed the redirect, so `navResponse.url` is the + // failing `/new` destination. Hard-navigating there directly avoids + // bouncing off `/old` just to re-follow the same 307, which would + // flash the wrong URL in the address bar and mis-key analytics. + // Matches Next.js' `doMpaNavigation(responseUrl.toString())`. Falls + // back to `currentHref` when no response URL is available. + const navContentType = navResponse.headers.get("content-type") ?? ""; + const isRscResponse = navContentType.startsWith("text/x-component"); + if (!navResponse.ok || !isRscResponse || !navResponse.body) { + const responseUrl = navResponseUrl ?? navResponse.url; + let hardNavTarget = currentHref; + if (responseUrl) { + const parsed = new URL(responseUrl, window.location.origin); + const origUrl = new URL(currentHref, window.location.origin); + let pathname = parsed.pathname.replace(/\.rsc$/, ""); + // toRscUrl strips trailing slash before appending .rsc, so the + // response URL loses it on the round-trip. Restore it when the + // original href had one so sites with trailingSlash:true don't + // incur an extra 308 to the canonical form on the error path. + 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; + } + window.location.href = hardNavTarget; + return; + } + const finalUrl = new URL(navResponseUrl ?? navResponse.url, window.location.origin); const requestedUrl = new URL(rscUrl, window.location.origin); @@ -1200,7 +1352,13 @@ async function main(): Promise { // Don't hard-navigate to a stale URL if this navigation was superseded by // a newer one — the newer navigation is already in flight and would be clobbered. if (navId !== activeNavigationId) return; - console.error("[vinext] RSC navigation error:", error); + // Suppress the diagnostic when the page is unloading: a hard-nav or anchor + // click tears down the document and aborts any in-flight RSC fetch, which + // surfaces here as an error. The page is already going away, so the log + // is just noise. Mirrors Next.js' isPageUnloading pattern. + if (!isPageUnloading) { + console.error("[vinext] RSC navigation error:", error); + } window.location.href = currentHref; } finally { // Single settlement site: covers normal return, early returns on stale-id @@ -1282,5 +1440,15 @@ async function main(): Promise { } if (typeof document !== "undefined") { + 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(); } diff --git a/tests/e2e/app-router/rsc-fetch-errors.spec.ts b/tests/e2e/app-router/rsc-fetch-errors.spec.ts new file mode 100644 index 000000000..9db054f03 --- /dev/null +++ b/tests/e2e/app-router/rsc-fetch-errors.spec.ts @@ -0,0 +1,222 @@ +/** + * RSC fetch error handling tests. + * + * Verifies that when an RSC navigation fetch returns a non-ok response (404, + * 500), the client performs a clean hard navigation to the destination URL + * rather than trying to parse the HTML error body as an RSC stream. + * + * Without the fix: + * - fetch(url.rsc) returns 404 HTML + * - createFromFetch throws a cryptic stream-parse error + * - The catch block logs "[vinext] RSC navigation error: ..." and hard-navs + * to the same URL again, which can loop + * + * With the fix: + * - !response.ok is detected immediately after fetch + * - Client hard-navigates directly to the destination URL (no .rsc suffix) + * - No stream-parse error is logged + * + * Ported behavior from Next.js fetch-server-response.ts:211: + * if (!isFlightResponse || !res.ok || !res.body) { + * return doMpaNavigation(responseUrl.toString()) + * } + */ +import { test, expect } from "@playwright/test"; +import { waitForAppRouterHydration } from "../helpers"; + +const BASE = "http://localhost:4174"; + +// Stream-parse errors thrown by createFromFetch / createFromReadableStream +// when handed a non-RSC payload (HTML error body, wrong content-type, empty +// stream). The pre-fix failure path produces one of these diagnostics; the +// filter here stays narrow on purpose so unrelated console errors (hydration +// timing, third-party scripts, JSON.parse in fixture code) never +// false-positive. Generic strings ("Connection closed", "Unexpected token") +// are gated on an RSC-context co-marker so a benign third-party JSON.parse +// diagnostic cannot satisfy them. +function isRscStreamParseError(msg: string): boolean { + const hasRscContext = msg.includes("RSC") || msg.includes("vinext"); + return ( + msg.includes("createFromFetch") || + msg.includes("createFromReadableStream") || + msg.includes("Failed to parse RSC") || + (hasRscContext && msg.includes("Connection closed")) || + (hasRscContext && msg.includes("Unexpected token")) + ); +} + +test.describe("RSC fetch non-ok response handling", () => { + test("client navigation to a non-existent route hard-navs to the non-.rsc URL", async ({ + page, + }) => { + const consoleErrors: string[] = []; + page.on("console", (msg) => { + if (msg.type() === "error") { + consoleErrors.push(msg.text()); + } + }); + + await page.goto(`${BASE}/about`); + await waitForAppRouterHydration(page); + + // Trigger RSC navigation to a route that does not exist (returns 404 HTML). + // We need to wait for the hard navigation, so we listen for the URL to change. + const navigationPromise = page.waitForURL(`${BASE}/this-route-does-not-exist`, { + timeout: 10_000, + }); + await page.evaluate(() => { + void (window as any).__VINEXT_RSC_NAVIGATE__("/this-route-does-not-exist"); + }); + await navigationPromise; + + // The browser must land on the non-.rsc URL — never on the .rsc variant. + expect(page.url()).toBe(`${BASE}/this-route-does-not-exist`); + + // The bug this PR fixes surfaces as one of a small set of RSC-stream + // parse errors when createFromFetch is handed an HTML body. Match only + // those diagnostics so an unrelated console error (e.g. a hydration- + // timing race that pre-existed this PR) does not false-positive here. + const rscParseError = consoleErrors.find((msg) => isRscStreamParseError(msg)); + expect(rscParseError).toBeUndefined(); + }); + + test("client navigation to a 500-route hard-navs to the destination URL without looping", async ({ + page, + }) => { + // Intercept the .rsc request for /about and return a 500 error. This + // intercept persists across navigations and reloads on this page, so if + // the fix is incomplete and a reload loop develops, the intercept hit + // count will grow without bound. + let aboutRscHits = 0; + await page.route(/\/about\.rsc(\?|$)/, (route) => { + aboutRscHits += 1; + return route.fulfill({ + status: 500, + // status 500 + text/html exercises both the !ok guard and the + // content-type guard at the nav site; editing either value in + // isolation drops the combined-guard coverage this test targets. + contentType: "text/html", + body: "

Internal Server Error

", + }); + }); + + const consoleErrors: string[] = []; + page.on("console", (msg) => { + if (msg.type() === "error") { + consoleErrors.push(msg.text()); + } + }); + + await page.goto(`${BASE}/`); + await waitForAppRouterHydration(page); + + const navigationPromise = page.waitForURL(`${BASE}/about`, { timeout: 10_000 }); + await page.evaluate(() => { + void (window as any).__VINEXT_RSC_NAVIGATE__("/about"); + }); + await navigationPromise; + + expect(page.url()).toBe(`${BASE}/about`); + + // Stability check: the hard-nav must settle. Without the + // readInitialRscStream reload-loop guard, the initial RSC fetch on the + // freshly-loaded /about page hits the intercepted 500 and reloads + // indefinitely — networkidle would never fire and the default timeout + // catches that. Tracking actual request activity avoids flaky wall-clock + // waits in CI. + const hitsBeforeNetworkIdle = aboutRscHits; + await page.waitForLoadState("networkidle"); + expect(page.url()).toBe(`${BASE}/about`); + // Pin the embedded-RSC assumption: after the hard-nav lands on /about, + // hydration must come from the HTML-embedded RSC branch and issue no + // further .rsc fetches. If a future change makes the embed path + // conditional and falls back to a fetch, this count would grow and the + // test would flag it rather than silently relying on networkidle timing. + expect(aboutRscHits).toBe(hitsBeforeNetworkIdle); + + // Expected trajectory: up to two hits — one from the home-page Link + // prefetch of /about.rsc (which the prefetch-cache discards because the + // response is !ok), and one from the client RSC nav fetch that triggers + // the hard-nav. Hydration timing can race the prefetch, in which case + // 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. + // 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); + + const rscParseError = consoleErrors.find((msg) => isRscStreamParseError(msg)); + expect(rscParseError).toBeUndefined(); + }); + + test("redirect chain to a non-ok endpoint hard-navs to the post-redirect URL", async ({ + page, + }) => { + // Chain: client nav to /redirect-src → fetch /redirect-src.rsc → + // 307 Location /about.rsc → 500. The hard-nav target must be /about + // (the post-redirect URL), not /redirect-src (the original request). + // Without the navResponseUrl ?? navResponse.url branch in the nav-site + // guard, the browser would bounce off /redirect-src and the server + // would re-issue the 307, flashing the wrong URL in the address bar + // and mis-keying analytics. + let srcRscHits = 0; + let aboutRscHits = 0; + // Intercept chain depends on Playwright re-entering the route table on + // the 307 follow-up so both handlers fire in sequence (browser 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) => { + srcRscHits += 1; + return route.fulfill({ + status: 307, + headers: { Location: `${BASE}/about.rsc` }, + }); + }); + await page.route(/\/about\.rsc(\?|$)/, (route) => { + aboutRscHits += 1; + return route.fulfill({ + status: 500, + contentType: "text/html", + body: "

Internal Server Error

", + }); + }); + + const consoleErrors: string[] = []; + page.on("console", (msg) => { + if (msg.type() === "error") { + consoleErrors.push(msg.text()); + } + }); + + // Capture the document URL at every main-frame navigation so we can + // assert the address bar never flashes /redirect-src en route to /about. + // Without this, a regression that dropped `navResponseUrl ?? navResponse.url` + // would still pass because the server's 307 converges to /about eventually. + const frameUrls: string[] = []; + page.on("framenavigated", (frame) => { + if (frame === page.mainFrame()) frameUrls.push(frame.url()); + }); + + await page.goto(`${BASE}/`); + await waitForAppRouterHydration(page); + + 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`); + expect(frameUrls.some((url) => url.includes("/redirect-src"))).toBe(false); + expect(srcRscHits).toBeGreaterThanOrEqual(1); + expect(aboutRscHits).toBeGreaterThanOrEqual(1); + + const rscParseError = consoleErrors.find((msg) => isRscStreamParseError(msg)); + expect(rscParseError).toBeUndefined(); + }); +});