From aad163aefd2f2fe4677d85c2c6540accf008294f Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 16 May 2026 16:37:33 +1000 Subject: [PATCH 1/5] fix(server): progressive actions trigger error boundaries on HTTP fallbacks --- .../vinext/src/server/app-page-dispatch.ts | 4 + packages/vinext/src/server/app-rsc-handler.ts | 3 + .../src/server/app-server-action-execution.ts | 79 ++++++----------- tests/app-server-action-execution.test.ts | 87 ++++++++++--------- 4 files changed, 77 insertions(+), 96 deletions(-) diff --git a/packages/vinext/src/server/app-page-dispatch.ts b/packages/vinext/src/server/app-page-dispatch.ts index 26733c676..abfd76f23 100644 --- a/packages/vinext/src/server/app-page-dispatch.ts +++ b/packages/vinext/src/server/app-page-dispatch.ts @@ -157,6 +157,7 @@ type DispatchAppPageOptions = { fetchCache?: FetchCacheMode | null; findIntercept: (pathname: string) => AppPageDispatchIntercept | null; formState?: ReactFormState | null; + actionError?: unknown; generateStaticParams?: ValidateAppPageDynamicParamsOptions["generateStaticParams"]; getFontLinks: () => string[]; getFontPreloads: () => AppPageFontPreload[]; @@ -583,6 +584,9 @@ async function dispatchAppPageInner( const pageBuildResult = await buildAppPageElement({ buildPageElement() { + if (options.actionError) { + throw options.actionError; + } return options.buildPageElement( route, options.params, diff --git a/packages/vinext/src/server/app-rsc-handler.ts b/packages/vinext/src/server/app-rsc-handler.ts index e3da9ea51..6f42f5eea 100644 --- a/packages/vinext/src/server/app-rsc-handler.ts +++ b/packages/vinext/src/server/app-rsc-handler.ts @@ -85,6 +85,7 @@ type AppRscRouteMatch = { type DispatchMatchedPageOptions = { cleanPathname: string; formState: ReactFormState | null; + actionError?: unknown; handlerStart: number; interceptionContext: string | null; isProgressiveActionRender: boolean; @@ -420,6 +421,7 @@ async function handleAppRscRequest( if (progressiveActionResult instanceof Response) return progressiveActionResult; const isProgressiveActionRender = progressiveActionResult?.kind === "form-state"; const formState = isProgressiveActionRender ? progressiveActionResult.formState : null; + const actionError = isProgressiveActionRender ? progressiveActionResult.actionError : undefined; const serverActionResponse = await options.handleServerActionRequest({ actionId, @@ -521,6 +523,7 @@ async function handleAppRscRequest( return options.dispatchMatchedPage({ cleanPathname, formState, + actionError, handlerStart, interceptionContext: interceptionContextHeader, isProgressiveActionRender, diff --git a/packages/vinext/src/server/app-server-action-execution.ts b/packages/vinext/src/server/app-server-action-execution.ts index 9aef8217a..c1f4488c4 100644 --- a/packages/vinext/src/server/app-server-action-execution.ts +++ b/packages/vinext/src/server/app-server-action-execution.ts @@ -198,16 +198,6 @@ export type HandleServerActionRscRequestOptions< toInterceptOpts: (intercept: AppServerActionIntercept) => TInterceptOpts; }; -type ActionControlResponse = - | { - kind: "redirect"; - url: string; - } - | { - kind: "status"; - statusCode: number; - }; - /** * Matches Next.js' server action argument cap to prevent stack overflow in * Function.prototype.apply when decoding hostile action payloads. @@ -297,33 +287,6 @@ export async function readActionFormDataWithLimit( }).formData(); } -function getActionControlResponse(error: unknown): ActionControlResponse | null { - const digest = getNextErrorDigest(error); - if (!digest) return null; - - const redirect = parseNextRedirectDigest(digest); - if (redirect) { - return { - kind: "redirect", - url: redirect.url, - }; - } - - const httpError = parseNextHttpErrorDigest(digest); - if (httpError) { - if (!Number.isInteger(httpError.status)) { - return null; - } - - return { - kind: "status", - statusCode: httpError.status, - }; - } - - return null; -} - function getActionRedirect(error: unknown): AppServerActionRedirect | null { const digest = getNextErrorDigest(error); if (!digest) return null; @@ -449,24 +412,39 @@ export async function handleProgressiveServerActionRequest( return null; } - let actionControlResponse: ActionControlResponse | null = null; + let actionRedirect: AppServerActionRedirect | null = null; + let actionError: unknown = undefined; let actionResult: unknown; const previousHeadersPhase = options.setHeadersAccessPhase("action"); try { actionResult = await action(); } catch (error) { - actionControlResponse = getActionControlResponse(error); - if (!actionControlResponse) { - throw error; + actionRedirect = getActionRedirect(error); + if (!actionRedirect) { + actionError = error; + const isControlFlow = + getActionHttpFallbackStatus(error) !== null || isServerActionNotFoundError(error, null); + if (!isControlFlow) { + console.error("[vinext] Server action error:", error); + options.reportRequestError( + normalizeError(error), + { + path: options.cleanPathname, + method: options.request.method, + headers: Object.fromEntries(options.request.headers.entries()), + }, + { routerKind: "App Router", routePath: options.cleanPathname, routeType: "action" }, + ); + } } } finally { options.setHeadersAccessPhase(previousHeadersPhase); } - if (!actionControlResponse) { + if (!actionRedirect) { getAndClearActionRevalidationKind(); - const formState = await options.decodeFormState(actionResult, body); - return { kind: "form-state", formState: formState ?? null }; + const formState = actionError ? null : await options.decodeFormState(actionResult, body); + return { kind: "form-state", formState: formState ?? null, actionError }; } const actionPendingCookies = options.getAndClearPendingCookies(); @@ -477,9 +455,7 @@ export async function handleProgressiveServerActionRequest( options.clearRequestContext(); const headers = new Headers(); - if (actionControlResponse.kind === "redirect") { - headers.set("Location", new URL(actionControlResponse.url, options.request.url).toString()); - } + headers.set("Location", new URL(actionRedirect.url, options.request.url).toString()); mergeMiddlewareResponseHeaders(headers, options.middlewareHeaders); for (const cookie of actionPendingCookies) { headers.append("Set-Cookie", cookie); @@ -490,7 +466,7 @@ export async function handleProgressiveServerActionRequest( setActionRevalidatedHeader(headers, actionRevalidationKind); return new Response(null, { - status: actionControlResponse.kind === "redirect" ? 303 : actionControlResponse.statusCode, + status: 303, headers, }); } catch (error) { @@ -503,10 +479,7 @@ export async function handleProgressiveServerActionRequest( getAndClearActionRevalidationKind(); options.getAndClearPendingCookies(); - // Next.js rethrows generic MPA action errors into its page render path. - // vinext does not yet implement that form-state render path, so unexpected - // action failures remain request failures here. - console.error("[vinext] Server action error:", error); + console.error("[vinext] Server action payload parsing error:", error); options.reportRequestError( normalizeError(error), { @@ -520,7 +493,7 @@ export async function handleProgressiveServerActionRequest( return internalServerErrorResponse( process.env.NODE_ENV === "production" ? undefined - : "Server action failed: " + getServerActionFailureMessage(error), + : "Server action parsing failed: " + getServerActionFailureMessage(error), ); } } diff --git a/tests/app-server-action-execution.test.ts b/tests/app-server-action-execution.test.ts index 70b02eec7..19f39a083 100644 --- a/tests/app-server-action-execution.test.ts +++ b/tests/app-server-action-execution.test.ts @@ -458,65 +458,66 @@ describe("app server action execution helpers", () => { expect((await request.formData()).get("field")).toBe("value"); }); - it("maps action HTTP fallback errors to status responses", async () => { - for (const [digest, statusCode] of [ - ["NEXT_NOT_FOUND", 404], - ["NEXT_HTTP_ERROR_FALLBACK;403", 403], - ]) { + it("passes HTTP fallback errors as actionError to be rendered by error boundaries", async () => { + for (const digest of ["NEXT_NOT_FOUND", "NEXT_HTTP_ERROR_FALLBACK;403"]) { const clearContext = vi.fn(); const reportedErrors: Error[] = []; - const response = requireProgressiveActionResponse( - await handleProgressiveServerActionRequest( - createOptions({ - clearRequestContext: clearContext, - async decodeAction() { - return () => { - throw { digest }; - }; - }, - reportRequestError(error) { - reportedErrors.push(error); - }, - }), - ), + const result = await handleProgressiveServerActionRequest( + createOptions({ + clearRequestContext: clearContext, + async decodeAction() { + return () => { + throw { digest }; + }; + }, + reportRequestError(error) { + reportedErrors.push(error); + }, + }), ); - expect(response.status).toBe(statusCode); + expect(result).toEqual({ + kind: "form-state", + formState: null, + actionError: { digest }, + }); expect(reportedErrors).toEqual([]); - expect(clearContext).toHaveBeenCalledTimes(1); + expect(clearContext).not.toHaveBeenCalled(); // Let app-rsc-handler clear it after render } }); - it("reports action execution failures and clears pending cookies", async () => { + it("passes action execution failures as actionError to be rendered by error boundaries", async () => { const reportedErrors: Error[] = []; const clearedCookies = vi.fn(() => ["session=1; Path=/"]); const clearContext = vi.fn(); const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const response = requireProgressiveActionResponse( - await handleProgressiveServerActionRequest( - createOptions({ - cleanPathname: "/action-source", - clearRequestContext: clearContext, - async decodeAction() { - return () => { - throw new Error("boom"); - }; - }, - getAndClearPendingCookies: clearedCookies, - reportRequestError(error) { - reportedErrors.push(error); - }, - }), - ), + const error = new Error("boom"); + const result = await handleProgressiveServerActionRequest( + createOptions({ + cleanPathname: "/action-source", + clearRequestContext: clearContext, + async decodeAction() { + return () => { + throw error; + }; + }, + getAndClearPendingCookies: clearedCookies, + reportRequestError(err) { + reportedErrors.push(err); + }, + }), ); - expect(response.status).toBe(500); - expect(await response.text()).toBe("Server action failed: boom"); - expect(reportedErrors.map((error) => error.message)).toEqual(["boom"]); - expect(clearedCookies).toHaveBeenCalledTimes(1); - expect(clearContext).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + kind: "form-state", + formState: null, + actionError: error, + }); + expect(reportedErrors.map((e) => e.message)).toEqual(["boom"]); + expect(clearedCookies).not.toHaveBeenCalled(); // Only cleared if response is rendered here + expect(clearContext).not.toHaveBeenCalled(); // Handled by app-rsc-handler errorSpy.mockRestore(); }); From f78bbd214d2a99b6e115fdc0a3ea867cb63f0bf2 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 16 May 2026 16:48:27 +1000 Subject: [PATCH 2/5] fix(server): type progressive action errors --- packages/vinext/src/server/app-rsc-handler.ts | 6 +++++- packages/vinext/src/server/app-server-action-execution.ts | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/vinext/src/server/app-rsc-handler.ts b/packages/vinext/src/server/app-rsc-handler.ts index 6f42f5eea..254dbb5c6 100644 --- a/packages/vinext/src/server/app-rsc-handler.ts +++ b/packages/vinext/src/server/app-rsc-handler.ts @@ -168,7 +168,11 @@ type CreateAppRscHandlerOptions = { ensureInstrumentation?: () => Promise; handleProgressiveActionRequest: ( options: HandleProgressiveActionRequestOptions, - ) => Promise; + ) => Promise< + | Response + | { actionError?: unknown; formState: ReactFormState | null; kind: "form-state" } + | null + >; handleServerActionRequest: ( options: HandleServerActionRequestOptions, ) => Promise; diff --git a/packages/vinext/src/server/app-server-action-execution.ts b/packages/vinext/src/server/app-server-action-execution.ts index c1f4488c4..1ecff981b 100644 --- a/packages/vinext/src/server/app-server-action-execution.ts +++ b/packages/vinext/src/server/app-server-action-execution.ts @@ -74,6 +74,7 @@ type AppServerActionRoute = { }; type ProgressiveServerActionResult = { + actionError?: unknown; formState: ReactFormState | null; kind: "form-state"; }; From ad33016cf183a6799e74f64ac4e1620b6b461529 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 16 May 2026 16:55:31 +1000 Subject: [PATCH 3/5] fix: guard stale popstate scroll restoration with active nav id --- .../vinext/src/server/app-browser-entry.ts | 30 +++-- .../vinext/src/server/app-browser-popstate.ts | 39 +++++++ tests/app-browser-entry.test.ts | 104 ++++++++++++++++++ 3 files changed, 162 insertions(+), 11 deletions(-) create mode 100644 packages/vinext/src/server/app-browser-popstate.ts diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index 1c40bcd21..80c12a8ca 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -75,6 +75,7 @@ import { type AppRouterState, type OperationLane, } from "./app-browser-state.js"; +import { createPopstateRestoreHandler } from "./app-browser-popstate.js"; import { DevRecoveryBoundary, RedirectBoundary } from "vinext/shims/error-boundary"; import { AppRouterContext } from "vinext/shims/internal/app-router-context"; import { ElementsContext, Slot } from "vinext/shims/slot"; @@ -1329,19 +1330,26 @@ function bootstrapHydration(rscStream: ReadableStream): void { // Pages Router scroll restoration is handled in shims/navigation.ts:1289 with // microtask-based deferral for compatibility with non-RSC navigation. // See: https://github.com/vercel/next.js/discussions/41934#discussioncomment-4602607 - window.addEventListener("popstate", (event) => { - notifyAppRouterTransitionStart(window.location.href, "traverse"); - const pendingNavigation = - window.__VINEXT_RSC_NAVIGATE__?.(window.location.href, 0, "traverse") ?? Promise.resolve(); - window.__VINEXT_RSC_PENDING__ = pendingNavigation; - void pendingNavigation.finally(() => { - restorePopstateScrollPosition(event.state); - if (window.__VINEXT_RSC_PENDING__ === pendingNavigation) { - window.__VINEXT_RSC_PENDING__ = null; - } - }); + const handlePopstate = createPopstateRestoreHandler({ + getActiveNavigationId: browserNavigationController.getActiveNavigationId.bind( + browserNavigationController, + ), + getPendingNavigation: () => window.__VINEXT_RSC_PENDING__, + getNavigate: () => window.__VINEXT_RSC_NAVIGATE__, + isCurrentNavigation: browserNavigationController.isCurrentNavigation.bind( + browserNavigationController, + ), + notifyAppRouterTransitionStart: (href) => { + notifyAppRouterTransitionStart(href, "traverse"); + }, + restorePopstateScrollPosition, + setPendingNavigation: (pendingNavigation) => { + window.__VINEXT_RSC_PENDING__ = pendingNavigation; + }, }); + window.addEventListener("popstate", handlePopstate); + if (import.meta.hot) { const handleRscUpdate = async (): Promise => { try { diff --git a/packages/vinext/src/server/app-browser-popstate.ts b/packages/vinext/src/server/app-browser-popstate.ts new file mode 100644 index 000000000..d0751c043 --- /dev/null +++ b/packages/vinext/src/server/app-browser-popstate.ts @@ -0,0 +1,39 @@ +type RestoreScrollPosition = (state: unknown) => void; +type NavigateRsc = ( + href: string, + redirectDepth?: number, + navigationKind?: "navigate" | "traverse" | "refresh", +) => Promise; + +type BrowserPopstateRestoreDeps = { + getActiveNavigationId: () => number; + getPendingNavigation: () => Promise | null | undefined; + getNavigate: () => NavigateRsc | undefined; + isCurrentNavigation: (navId: number) => boolean; + notifyAppRouterTransitionStart: (href: string) => void; + restorePopstateScrollPosition: RestoreScrollPosition; + setPendingNavigation: (pendingNavigation: Promise | null) => void; +}; + +export function createPopstateRestoreHandler( + deps: BrowserPopstateRestoreDeps, +): (event: PopStateEvent) => void { + return (event) => { + deps.notifyAppRouterTransitionStart(window.location.href); + const navigate = deps.getNavigate(); + const pendingNavigation = navigate?.(window.location.href, 0, "traverse") ?? Promise.resolve(); + const popstateNavId = deps.getActiveNavigationId(); + + deps.setPendingNavigation(pendingNavigation); + + void pendingNavigation.finally(() => { + if (deps.isCurrentNavigation(popstateNavId)) { + deps.restorePopstateScrollPosition(event.state); + } + + if (deps.getPendingNavigation() === pendingNavigation) { + deps.setPendingNavigation(null); + } + }); + }; +} diff --git a/tests/app-browser-entry.test.ts b/tests/app-browser-entry.test.ts index 6366446ee..4f3bfaf88 100644 --- a/tests/app-browser-entry.test.ts +++ b/tests/app-browser-entry.test.ts @@ -15,6 +15,7 @@ import { hydrateRootInTransition, } from "../packages/vinext/src/server/app-browser-hydration.js"; import { createAppBrowserNavigationController } from "../packages/vinext/src/server/app-browser-navigation-controller.js"; +import { createPopstateRestoreHandler } from "../packages/vinext/src/server/app-browser-popstate.js"; import { VINEXT_RSC_COMPATIBILITY_ID_HEADER, resolveRscCompatibilityNavigationDecision, @@ -251,6 +252,16 @@ function stubWindow(href: string) { return { assign, replace, storage }; } +function createDeferred(): { resolve: () => void; promise: Promise } { + let resolve = () => { + throw new Error("Promise was not initialized"); + }; + const promise = new Promise((resolveInner) => { + resolve = resolveInner; + }); + return { promise, resolve }; +} + afterEach(() => { vi.restoreAllMocks(); vi.unstubAllGlobals(); @@ -2993,6 +3004,99 @@ describe("app browser entry previousNextUrl helpers", () => { }); }); +describe("createPopstateRestoreHandler", () => { + it("restores scroll only after the latest popstate navigation commits", async () => { + const restoreCalls: unknown[] = []; + const firstNavigation = createDeferred(); + const secondNavigation = createDeferred(); + let popstateCalls = 0; + const popstate = vi.fn(() => { + popstateCalls += 1; + if (popstateCalls === 1) { + return firstNavigation.promise; + } + return secondNavigation.promise; + }); + let activeNavigationId = 0; + + stubWindow("https://example.com/feed"); + window.__VINEXT_RSC_PENDING__ = null; + + const handler = createPopstateRestoreHandler({ + getActiveNavigationId: () => activeNavigationId, + getNavigate: () => { + activeNavigationId += 1; + return (href: string) => popstate(); + }, + getPendingNavigation: () => window.__VINEXT_RSC_PENDING__, + isCurrentNavigation: (navId) => navId === activeNavigationId, + notifyAppRouterTransitionStart: () => {}, + restorePopstateScrollPosition: (scrollState) => { + restoreCalls.push(scrollState); + }, + setPendingNavigation: (pendingNavigation) => { + window.__VINEXT_RSC_PENDING__ = pendingNavigation; + }, + }); + + handler({ state: { __vinext_scrollY: 10 } } as PopStateEvent); + handler({ state: { __vinext_scrollY: 20 } } as PopStateEvent); + + expect(window.__VINEXT_RSC_PENDING__).toBe(secondNavigation.promise); + + secondNavigation.resolve(); + await secondNavigation.promise; + await Promise.resolve(); + + expect(restoreCalls).toEqual([{ __vinext_scrollY: 20 }]); + expect(window.__VINEXT_RSC_PENDING__).toBeNull(); + + firstNavigation.resolve(); + await firstNavigation.promise; + await Promise.resolve(); + + expect(restoreCalls).toEqual([{ __vinext_scrollY: 20 }]); + expect(window.__VINEXT_RSC_PENDING__).toBeNull(); + }); + + it("clears __VINEXT_RSC_PENDING__ when a stale popstate navigation settles", async () => { + const restoreCalls: unknown[] = []; + const navigation = createDeferred(); + let activeNavigationId = 1; + + stubWindow("https://example.com/feed"); + window.__VINEXT_RSC_PENDING__ = null; + + const handler = createPopstateRestoreHandler({ + getActiveNavigationId: () => activeNavigationId, + getNavigate: () => { + activeNavigationId = 1; + return () => navigation.promise; + }, + getPendingNavigation: () => window.__VINEXT_RSC_PENDING__, + isCurrentNavigation: (navId) => navId === activeNavigationId, + notifyAppRouterTransitionStart: () => {}, + restorePopstateScrollPosition: (scrollState) => { + restoreCalls.push(scrollState); + }, + setPendingNavigation: (pendingNavigation) => { + window.__VINEXT_RSC_PENDING__ = pendingNavigation; + }, + }); + + handler({ state: { __vinext_scrollY: 10 } } as PopStateEvent); + expect(window.__VINEXT_RSC_PENDING__).toBe(navigation.promise); + + activeNavigationId = 2; + navigation.resolve(); + await navigation.promise; + await Promise.resolve(); + + expect(restoreCalls).toEqual([]); + expect(window.__VINEXT_RSC_PENDING__).toBeNull(); + }); +}); + describe("devOnCaughtError (hydrateRoot dev handler)", () => { it("ignores redirect sentinels handled by RedirectBoundary", () => { const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); From 334fbd4f93fe0105419d49f1c68d9fc3fde51a0a Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 16 May 2026 17:02:42 +1000 Subject: [PATCH 4/5] revert(server): drop action error changes from popstate PR The popstate race branch included server action commits from a separate branch. That made the PR review surface change progressive action error semantics unrelated to browser history restoration. Restore those files to main's behavior in this branch and keep only the popstate scroll restoration fix. Also correct the new popstate tests so targeted check and unit coverage pass. --- .../vinext/src/server/app-page-dispatch.ts | 4 - packages/vinext/src/server/app-rsc-handler.ts | 9 +- .../src/server/app-server-action-execution.ts | 80 +++++++++++------ tests/app-browser-entry.test.ts | 4 +- tests/app-server-action-execution.test.ts | 87 +++++++++---------- 5 files changed, 99 insertions(+), 85 deletions(-) diff --git a/packages/vinext/src/server/app-page-dispatch.ts b/packages/vinext/src/server/app-page-dispatch.ts index abfd76f23..26733c676 100644 --- a/packages/vinext/src/server/app-page-dispatch.ts +++ b/packages/vinext/src/server/app-page-dispatch.ts @@ -157,7 +157,6 @@ type DispatchAppPageOptions = { fetchCache?: FetchCacheMode | null; findIntercept: (pathname: string) => AppPageDispatchIntercept | null; formState?: ReactFormState | null; - actionError?: unknown; generateStaticParams?: ValidateAppPageDynamicParamsOptions["generateStaticParams"]; getFontLinks: () => string[]; getFontPreloads: () => AppPageFontPreload[]; @@ -584,9 +583,6 @@ async function dispatchAppPageInner( const pageBuildResult = await buildAppPageElement({ buildPageElement() { - if (options.actionError) { - throw options.actionError; - } return options.buildPageElement( route, options.params, diff --git a/packages/vinext/src/server/app-rsc-handler.ts b/packages/vinext/src/server/app-rsc-handler.ts index 254dbb5c6..e3da9ea51 100644 --- a/packages/vinext/src/server/app-rsc-handler.ts +++ b/packages/vinext/src/server/app-rsc-handler.ts @@ -85,7 +85,6 @@ type AppRscRouteMatch = { type DispatchMatchedPageOptions = { cleanPathname: string; formState: ReactFormState | null; - actionError?: unknown; handlerStart: number; interceptionContext: string | null; isProgressiveActionRender: boolean; @@ -168,11 +167,7 @@ type CreateAppRscHandlerOptions = { ensureInstrumentation?: () => Promise; handleProgressiveActionRequest: ( options: HandleProgressiveActionRequestOptions, - ) => Promise< - | Response - | { actionError?: unknown; formState: ReactFormState | null; kind: "form-state" } - | null - >; + ) => Promise; handleServerActionRequest: ( options: HandleServerActionRequestOptions, ) => Promise; @@ -425,7 +420,6 @@ async function handleAppRscRequest( if (progressiveActionResult instanceof Response) return progressiveActionResult; const isProgressiveActionRender = progressiveActionResult?.kind === "form-state"; const formState = isProgressiveActionRender ? progressiveActionResult.formState : null; - const actionError = isProgressiveActionRender ? progressiveActionResult.actionError : undefined; const serverActionResponse = await options.handleServerActionRequest({ actionId, @@ -527,7 +521,6 @@ async function handleAppRscRequest( return options.dispatchMatchedPage({ cleanPathname, formState, - actionError, handlerStart, interceptionContext: interceptionContextHeader, isProgressiveActionRender, diff --git a/packages/vinext/src/server/app-server-action-execution.ts b/packages/vinext/src/server/app-server-action-execution.ts index 1ecff981b..9aef8217a 100644 --- a/packages/vinext/src/server/app-server-action-execution.ts +++ b/packages/vinext/src/server/app-server-action-execution.ts @@ -74,7 +74,6 @@ type AppServerActionRoute = { }; type ProgressiveServerActionResult = { - actionError?: unknown; formState: ReactFormState | null; kind: "form-state"; }; @@ -199,6 +198,16 @@ export type HandleServerActionRscRequestOptions< toInterceptOpts: (intercept: AppServerActionIntercept) => TInterceptOpts; }; +type ActionControlResponse = + | { + kind: "redirect"; + url: string; + } + | { + kind: "status"; + statusCode: number; + }; + /** * Matches Next.js' server action argument cap to prevent stack overflow in * Function.prototype.apply when decoding hostile action payloads. @@ -288,6 +297,33 @@ export async function readActionFormDataWithLimit( }).formData(); } +function getActionControlResponse(error: unknown): ActionControlResponse | null { + const digest = getNextErrorDigest(error); + if (!digest) return null; + + const redirect = parseNextRedirectDigest(digest); + if (redirect) { + return { + kind: "redirect", + url: redirect.url, + }; + } + + const httpError = parseNextHttpErrorDigest(digest); + if (httpError) { + if (!Number.isInteger(httpError.status)) { + return null; + } + + return { + kind: "status", + statusCode: httpError.status, + }; + } + + return null; +} + function getActionRedirect(error: unknown): AppServerActionRedirect | null { const digest = getNextErrorDigest(error); if (!digest) return null; @@ -413,39 +449,24 @@ export async function handleProgressiveServerActionRequest( return null; } - let actionRedirect: AppServerActionRedirect | null = null; - let actionError: unknown = undefined; + let actionControlResponse: ActionControlResponse | null = null; let actionResult: unknown; const previousHeadersPhase = options.setHeadersAccessPhase("action"); try { actionResult = await action(); } catch (error) { - actionRedirect = getActionRedirect(error); - if (!actionRedirect) { - actionError = error; - const isControlFlow = - getActionHttpFallbackStatus(error) !== null || isServerActionNotFoundError(error, null); - if (!isControlFlow) { - console.error("[vinext] Server action error:", error); - options.reportRequestError( - normalizeError(error), - { - path: options.cleanPathname, - method: options.request.method, - headers: Object.fromEntries(options.request.headers.entries()), - }, - { routerKind: "App Router", routePath: options.cleanPathname, routeType: "action" }, - ); - } + actionControlResponse = getActionControlResponse(error); + if (!actionControlResponse) { + throw error; } } finally { options.setHeadersAccessPhase(previousHeadersPhase); } - if (!actionRedirect) { + if (!actionControlResponse) { getAndClearActionRevalidationKind(); - const formState = actionError ? null : await options.decodeFormState(actionResult, body); - return { kind: "form-state", formState: formState ?? null, actionError }; + const formState = await options.decodeFormState(actionResult, body); + return { kind: "form-state", formState: formState ?? null }; } const actionPendingCookies = options.getAndClearPendingCookies(); @@ -456,7 +477,9 @@ export async function handleProgressiveServerActionRequest( options.clearRequestContext(); const headers = new Headers(); - headers.set("Location", new URL(actionRedirect.url, options.request.url).toString()); + if (actionControlResponse.kind === "redirect") { + headers.set("Location", new URL(actionControlResponse.url, options.request.url).toString()); + } mergeMiddlewareResponseHeaders(headers, options.middlewareHeaders); for (const cookie of actionPendingCookies) { headers.append("Set-Cookie", cookie); @@ -467,7 +490,7 @@ export async function handleProgressiveServerActionRequest( setActionRevalidatedHeader(headers, actionRevalidationKind); return new Response(null, { - status: 303, + status: actionControlResponse.kind === "redirect" ? 303 : actionControlResponse.statusCode, headers, }); } catch (error) { @@ -480,7 +503,10 @@ export async function handleProgressiveServerActionRequest( getAndClearActionRevalidationKind(); options.getAndClearPendingCookies(); - console.error("[vinext] Server action payload parsing error:", error); + // Next.js rethrows generic MPA action errors into its page render path. + // vinext does not yet implement that form-state render path, so unexpected + // action failures remain request failures here. + console.error("[vinext] Server action error:", error); options.reportRequestError( normalizeError(error), { @@ -494,7 +520,7 @@ export async function handleProgressiveServerActionRequest( return internalServerErrorResponse( process.env.NODE_ENV === "production" ? undefined - : "Server action parsing failed: " + getServerActionFailureMessage(error), + : "Server action failed: " + getServerActionFailureMessage(error), ); } } diff --git a/tests/app-browser-entry.test.ts b/tests/app-browser-entry.test.ts index 4f3bfaf88..8045f32b5 100644 --- a/tests/app-browser-entry.test.ts +++ b/tests/app-browser-entry.test.ts @@ -253,7 +253,7 @@ function stubWindow(href: string) { } function createDeferred(): { resolve: () => void; promise: Promise } { - let resolve = () => { + let resolve: () => void = () => { throw new Error("Promise was not initialized"); }; const promise = new Promise((resolveInner) => { @@ -3026,7 +3026,7 @@ describe("createPopstateRestoreHandler", () => { getActiveNavigationId: () => activeNavigationId, getNavigate: () => { activeNavigationId += 1; - return (href: string) => popstate(); + return () => popstate(); }, getPendingNavigation: () => window.__VINEXT_RSC_PENDING__, isCurrentNavigation: (navId) => navId === activeNavigationId, diff --git a/tests/app-server-action-execution.test.ts b/tests/app-server-action-execution.test.ts index 19f39a083..70b02eec7 100644 --- a/tests/app-server-action-execution.test.ts +++ b/tests/app-server-action-execution.test.ts @@ -458,66 +458,65 @@ describe("app server action execution helpers", () => { expect((await request.formData()).get("field")).toBe("value"); }); - it("passes HTTP fallback errors as actionError to be rendered by error boundaries", async () => { - for (const digest of ["NEXT_NOT_FOUND", "NEXT_HTTP_ERROR_FALLBACK;403"]) { + it("maps action HTTP fallback errors to status responses", async () => { + for (const [digest, statusCode] of [ + ["NEXT_NOT_FOUND", 404], + ["NEXT_HTTP_ERROR_FALLBACK;403", 403], + ]) { const clearContext = vi.fn(); const reportedErrors: Error[] = []; - const result = await handleProgressiveServerActionRequest( - createOptions({ - clearRequestContext: clearContext, - async decodeAction() { - return () => { - throw { digest }; - }; - }, - reportRequestError(error) { - reportedErrors.push(error); - }, - }), + const response = requireProgressiveActionResponse( + await handleProgressiveServerActionRequest( + createOptions({ + clearRequestContext: clearContext, + async decodeAction() { + return () => { + throw { digest }; + }; + }, + reportRequestError(error) { + reportedErrors.push(error); + }, + }), + ), ); - expect(result).toEqual({ - kind: "form-state", - formState: null, - actionError: { digest }, - }); + expect(response.status).toBe(statusCode); expect(reportedErrors).toEqual([]); - expect(clearContext).not.toHaveBeenCalled(); // Let app-rsc-handler clear it after render + expect(clearContext).toHaveBeenCalledTimes(1); } }); - it("passes action execution failures as actionError to be rendered by error boundaries", async () => { + it("reports action execution failures and clears pending cookies", async () => { const reportedErrors: Error[] = []; const clearedCookies = vi.fn(() => ["session=1; Path=/"]); const clearContext = vi.fn(); const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const error = new Error("boom"); - const result = await handleProgressiveServerActionRequest( - createOptions({ - cleanPathname: "/action-source", - clearRequestContext: clearContext, - async decodeAction() { - return () => { - throw error; - }; - }, - getAndClearPendingCookies: clearedCookies, - reportRequestError(err) { - reportedErrors.push(err); - }, - }), + const response = requireProgressiveActionResponse( + await handleProgressiveServerActionRequest( + createOptions({ + cleanPathname: "/action-source", + clearRequestContext: clearContext, + async decodeAction() { + return () => { + throw new Error("boom"); + }; + }, + getAndClearPendingCookies: clearedCookies, + reportRequestError(error) { + reportedErrors.push(error); + }, + }), + ), ); - expect(result).toEqual({ - kind: "form-state", - formState: null, - actionError: error, - }); - expect(reportedErrors.map((e) => e.message)).toEqual(["boom"]); - expect(clearedCookies).not.toHaveBeenCalled(); // Only cleared if response is rendered here - expect(clearContext).not.toHaveBeenCalled(); // Handled by app-rsc-handler + expect(response.status).toBe(500); + expect(await response.text()).toBe("Server action failed: boom"); + expect(reportedErrors.map((error) => error.message)).toEqual(["boom"]); + expect(clearedCookies).toHaveBeenCalledTimes(1); + expect(clearContext).toHaveBeenCalledTimes(1); errorSpy.mockRestore(); }); From b7289c10bfaaf3b3863b211ed885d64126bb9072 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 16 May 2026 17:14:50 +1000 Subject: [PATCH 5/5] test(app-router): wait for RSC cache seeding The same-build visited payload E2E asserted cache replay after visible commit from a void router.push. The visited-response cache is populated when the internal RSC navigation promise resolves, so the test could navigate away before the cache entry existed and count a second fetch. Wrap the internal navigation function in the test and wait for the captured promise before asserting the replay path. --- .../app-router/build-id-navigation.spec.ts | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/e2e/app-router/build-id-navigation.spec.ts b/tests/e2e/app-router/build-id-navigation.spec.ts index dd4872701..3916e1b37 100644 --- a/tests/e2e/app-router/build-id-navigation.spec.ts +++ b/tests/e2e/app-router/build-id-navigation.spec.ts @@ -3,6 +3,7 @@ import { waitForAppRouterHydration } from "../helpers"; const BASE = "http://localhost:4174"; const VISITED_CACHE_MARKER = "__VINEXT_VISITED_CACHE_MARKER__"; +const RSC_NAVIGATION_PROMISE_MARKER = "__VINEXT_TEST_RSC_NAVIGATION_PROMISE__"; async function pushAppRoute(page: Page, pathname: string): Promise { await page.evaluate((target) => { @@ -14,6 +15,47 @@ async function pushAppRoute(page: Page, pathname: string): Promise { }, pathname); } +async function captureRscNavigationPromises(page: Page): Promise { + await page.evaluate((marker) => { + const navigate = window.__VINEXT_RSC_NAVIGATE__; + if (typeof navigate !== "function") { + throw new Error("window.__VINEXT_RSC_NAVIGATE__ is not installed"); + } + + const wrappedNavigate: typeof navigate = ( + href, + redirectDepth, + navigationKind, + historyUpdateMode, + previousNextUrlOverride, + programmaticTransition, + ) => { + const pendingNavigation = navigate( + href, + redirectDepth, + navigationKind, + historyUpdateMode, + previousNextUrlOverride, + programmaticTransition, + ); + Reflect.set(window, marker, pendingNavigation); + return pendingNavigation; + }; + + window.__VINEXT_RSC_NAVIGATE__ = wrappedNavigate; + }, RSC_NAVIGATION_PROMISE_MARKER); +} + +async function waitForLastRscNavigation(page: Page): Promise { + await page.waitForFunction( + (marker) => Reflect.get(window, marker), + RSC_NAVIGATION_PROMISE_MARKER, + ); + await page.evaluate(async (marker) => { + await Reflect.get(window, marker); + }, RSC_NAVIGATION_PROMISE_MARKER); +} + test.describe("App Router RSC compatibility navigation", () => { test("replays same-build visited RSC payloads instead of refetching or reloading", async ({ page, @@ -28,9 +70,13 @@ test.describe("App Router RSC compatibility navigation", () => { await page.goto(`${BASE}/`); await waitForAppRouterHydration(page); + await captureRscNavigationPromises(page); await pushAppRoute(page, "/about"); await expect(page.locator("h1")).toHaveText("About"); + // router.push commits visible UI before the RSC navigation promise has + // finished seeding the visited-response cache this test asserts on. + await waitForLastRscNavigation(page); expect(aboutRscRequests).toHaveLength(1); await page.evaluate((marker) => { @@ -42,6 +88,7 @@ test.describe("App Router RSC compatibility navigation", () => { router.push("/"); }, VISITED_CACHE_MARKER); await expect(page.locator("h1")).toHaveText("Welcome to App Router"); + await waitForLastRscNavigation(page); await pushAppRoute(page, "/about"); await expect(page.locator("h1")).toHaveText("About");