1 Commits

Author	SHA1	Message	Date
James Anderson	7b41e950bd	fix(app-page): align redirect()/notFound() handling under loading.tsx with Next.js (#1057) ... * fix(app-page-probe): always await page promise so redirect()/notFound() under loading.tsx return 307/404 The probe used to fire-and-forget the page promise when a route-level loading.tsx was present (`awaitAsyncResult: !hasLoadingBoundary`). For async pages that throw `redirect()` or `notFound()`, that swallowed the rejection during probe; the RSC stream then re-ran the page under the route Suspense boundary, where React converted the throw into a "Switched to client rendering" error serialized into a 200 HTML body instead of a clean 307/404. Awaiting the page in the probe surfaces the special error early, parity with the no-loading-boundary path. The cost is that the outer page function runs serially with the RSC render rather than in parallel — in practice most async work in `loading.tsx`-shielded routes happens in nested Suspense children, not in the outer page function body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: capture special-error digests post-render instead of awaiting probe Replaces the previous "always await page in probe" approach with a post-render swap that mirrors Next.js's mechanism in app-render.tsx: the page render's onError captures NEXT_REDIRECT / NEXT_HTTP_ERROR_FALLBACK digests; after the HTML stream drains, the lifecycle inspects the tracker and swaps a 307/404 in place of the streamed "Switched to client rendering" body. This avoids the 2× page invocation cost of awaiting in the probe (the previous fix called the page once during probe and again during RSC render, serially). Now the page runs once, inside the RSC render, and the route-level Suspense boundary (loading.tsx) traps the throw as designed. The buffered stream still contains the same final HTML for non-redirecting pages. Trade-off matches Next.js: loading.tsx fallback visibility is sacrificed for routes that redirect (you cannot both flush the loading state and reverse course to a 307). Non-redirecting routes with loading.tsx now buffer the full response rather than streaming progressively — same trade-off Next.js makes with its prerender-based pipeline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "refactor: capture special-error digests post-render instead of awaiting probe" This reverts commit 179b20f88033f3731677a91faeb022389247223e. * refactor: align with Next.js — skip page probe + post-shell race for special errors Replaces the always-await fix with a Next.js-aligned pipeline: - Skip the page probe entirely when a route-level loading.tsx is present. The page now runs once, inside the RSC render, instead of twice serially. - Capture NEXT_REDIRECT / NEXT_HTTP_ERROR_FALLBACK digests in the rscErrorTracker via React's onError callback (previously dropped). - After the SSR shell is ready but before bytes are flushed, race the captured digest against a 50 ms swap window. A digest that fires within the window — sync throws and short-async like ~10 ms auth checks — is converted to a 307/404. Late rejections fall through to the streamed body, matching Next.js's "until-first-byte-is-flushed" semantic. Trade-offs vs. the previous always-await fix: - Page invoked once instead of twice (no more 2× latency for async outer page functions). - TTFB for loading.tsx routes goes from ~outer-page-time to ~50 ms. - loading.tsx fallback is visible immediately after the swap window closes — the OpenNext-compat E2E that broke under the buffer-everything approach passes. Slow async redirects (>50 ms) still leak as serialized stream errors. Next.js handles that case by injecting <meta http-equiv="refresh"> mid-stream; doing the same in vinext is a separate, larger change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: drop 50ms swap-window — deterministic post-shell digest check Removes the timer-race hack and replaces it with the same shape Next.js uses (`app-render.tsx:4293`): after the SSR shell promise resolves, inspect the rscErrorTracker for a captured NEXT_REDIRECT/NEXT_HTTP_ERROR_FALLBACK digest and swap the response to a 307/404 if one is present. Why this works without a timer: the SSR pipeline naturally runs many microtask awaits between the page Promise rejecting (in onError) and the lifecycle reading the tracker. Throws that resolve in microtasks during shell rendering are deterministically captured. Throws that require macrotask boundaries (real I/O, setTimeout) are NOT caught and fall through to the streamed body — exactly the same trade-off Next.js makes (the digest survives in the Flight payload for the client router to consume). Updates the regression fixture to use the deterministic case (sync throw inside an async page) instead of the pre-fix repro's setTimeout trick. The setTimeout case can't be deterministically caught without either a timer (rejected as a hack) or React-internals patches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: regression for permanentRedirect() under loading.tsx returning 308 Adds a fixture and integration test mirroring the redirect()-with- loading.tsx case but using permanentRedirect (308). Confirms the post-shell digest swap honors the statusCode field from the NEXT_REDIRECT digest rather than coercing to the 307 default — closes a flagged gap from the Next.js parity audit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: regression for forbidden()/unauthorized() under loading.tsx Adds fixtures and integration tests verifying the post-shell digest swap correctly preserves the status code from NEXT_HTTP_ERROR_FALLBACK digests: - forbidden() under loading.tsx → 403 with root forbidden.tsx body - unauthorized() under loading.tsx → 401 with root unauthorized.tsx body Confirms resolveAppPageSpecialError reads `parseInt(parts[1], 10)` from the digest rather than coercing to 404 — closes the parity-audit's "forbidden/unauthorized status coercion" gap (which turned out to already be handled correctly; this just locks it in). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(app-page): apply basePath to redirect Location header Mirrors Next.js's `addPathPrefix(getURLFromRedirectError(err), basePath)` in app-render.tsx — `redirect("/about")` from a page mounted under basePath "/blog" now produces `Location: /blog/about` instead of the raw "/about". - Adds `basePath` to BuildAppPageSpecialErrorResponseOptions and a small helper that resolves the redirect target against the request URL and prefixes app-internal absolute paths. - External redirects (different origin) and targets that already start with the basePath are passed through unchanged. - Threads the entry-generated `__basePath` through dispatch and into both the page and layout special-error response builders. - Closes a parity gap flagged in the Next.js audit; covered by 5 new unit assertions exercising internal / external / already-prefixed / unconfigured / root-target cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(app-page): preserve cookies().set() on redirect responses Mirrors Next.js's `appendMutableCookies(headers, requestStore.mutableCookies)` in app-render.tsx. Auth flows that do cookies().set("session", "..."); redirect("/dashboard"); must preserve the Set-Cookie on the 307, otherwise the redirected request lands without the just-issued session and the user bounces back to login. - Adds an optional `getAndClearPendingCookies` to BuildAppPageSpecialErrorResponseOptions that, when set, drains pending Set-Cookie values accumulated during the render and appends them to the redirect response. - Wires `getAndClearPendingCookies` from `vinext/shims/headers` into both the page and layout special-error response builders in dispatch. - Only applied to redirect responses to match Next.js — http-access-fallback responses leave cookies to the rendered boundary. - Closes a parity gap from the audit; covered by a new unit test exercising redirect-with-cookies, redirect-without-cookies, and the http-access-fallback-cookies-not-bled cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: regression for notFound() under loading.tsx Adds a fixture and integration test covering the most common loading-boundary special-error case in real apps: a dynamic detail page with a loading state that calls notFound() when the record is missing. Distinct from the forbidden()/unauthorized() coverage — notFound() throws the bare "NEXT_NOT_FOUND" digest (not "NEXT_HTTP_ERROR_FALLBACK;404"), which takes a separate branch in resolveAppPageSpecialError. This locks in that the post-shell digest swap handles both branches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 16:46:55 +00:00