diff --git a/packages/vinext/src/entries/app-rsc-entry.ts b/packages/vinext/src/entries/app-rsc-entry.ts index f08b13227..57ab27b2c 100644 --- a/packages/vinext/src/entries/app-rsc-entry.ts +++ b/packages/vinext/src/entries/app-rsc-entry.ts @@ -447,6 +447,8 @@ export default __createAppRscHandler({ dispatchMatchedPage({ cleanPathname, formState, + actionError, + actionFailed, handlerStart, interceptionContext, isProgressiveActionRender, @@ -513,6 +515,8 @@ export default __createAppRscHandler({ interceptionContext, expireSeconds: __expireTime, formState, + actionError, + actionFailed, isProgressiveActionRender, isProduction: process.env.NODE_ENV === "production", isRscRequest, diff --git a/packages/vinext/src/server/app-page-dispatch.ts b/packages/vinext/src/server/app-page-dispatch.ts index 26733c676..390221422 100644 --- a/packages/vinext/src/server/app-page-dispatch.ts +++ b/packages/vinext/src/server/app-page-dispatch.ts @@ -157,6 +157,8 @@ type DispatchAppPageOptions = { fetchCache?: FetchCacheMode | null; findIntercept: (pathname: string) => AppPageDispatchIntercept | null; formState?: ReactFormState | null; + actionError?: unknown; + actionFailed?: boolean; generateStaticParams?: ValidateAppPageDynamicParamsOptions["generateStaticParams"]; getFontLinks: () => string[]; getFontPreloads: () => AppPageFontPreload[]; @@ -583,6 +585,9 @@ async function dispatchAppPageInner( const pageBuildResult = await buildAppPageElement({ buildPageElement() { + if (options.actionFailed) { + 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..e878a333c 100644 --- a/packages/vinext/src/server/app-rsc-handler.ts +++ b/packages/vinext/src/server/app-rsc-handler.ts @@ -85,6 +85,8 @@ type AppRscRouteMatch = { type DispatchMatchedPageOptions = { cleanPathname: string; formState: ReactFormState | null; + actionError?: unknown; + actionFailed?: boolean; handlerStart: number; interceptionContext: string | null; isProgressiveActionRender: boolean; @@ -116,6 +118,18 @@ type HandleProgressiveActionRequestOptions = { request: Request; }; +type ProgressiveActionFormStateResult = + | { + formState: ReactFormState | null; + kind: "form-state"; + } + | { + actionError: unknown; + actionFailed: true; + formState: null; + kind: "form-state"; + }; + type HandleServerActionRequestOptions = { actionId: string | null; cleanPathname: string; @@ -167,7 +181,7 @@ type CreateAppRscHandlerOptions = { ensureInstrumentation?: () => Promise; handleProgressiveActionRequest: ( options: HandleProgressiveActionRequestOptions, - ) => Promise; + ) => Promise; handleServerActionRequest: ( options: HandleServerActionRequestOptions, ) => Promise; @@ -420,6 +434,12 @@ async function handleAppRscRequest( if (progressiveActionResult instanceof Response) return progressiveActionResult; const isProgressiveActionRender = progressiveActionResult?.kind === "form-state"; const formState = isProgressiveActionRender ? progressiveActionResult.formState : null; + const failedProgressiveActionResult = + isProgressiveActionRender && "actionFailed" in progressiveActionResult + ? progressiveActionResult + : null; + const actionFailed = failedProgressiveActionResult !== null; + const actionError = failedProgressiveActionResult?.actionError; const serverActionResponse = await options.handleServerActionRequest({ actionId, @@ -521,6 +541,8 @@ async function handleAppRscRequest( return options.dispatchMatchedPage({ cleanPathname, formState, + actionError, + actionFailed, 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..96bfbb334 100644 --- a/packages/vinext/src/server/app-server-action-execution.ts +++ b/packages/vinext/src/server/app-server-action-execution.ts @@ -73,10 +73,17 @@ type AppServerActionRoute = { pattern: string; }; -type ProgressiveServerActionResult = { - formState: ReactFormState | null; - kind: "form-state"; -}; +type ProgressiveServerActionResult = + | { + formState: ReactFormState | null; + kind: "form-state"; + } + | { + actionError: unknown; + actionFailed: true; + formState: null; + kind: "form-state"; + }; type AppServerActionMatch = { params: AppPageParams; @@ -198,16 +205,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 +294,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,22 +419,43 @@ export async function handleProgressiveServerActionRequest( return null; } - let actionControlResponse: ActionControlResponse | null = null; + let actionRedirect: AppServerActionRedirect | null = null; + let actionError: unknown = undefined; + let actionFailed = false; 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; + actionFailed = true; + 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(); + if (actionFailed) { + return { kind: "form-state", formState: null, actionError, actionFailed }; + } + const formState = await options.decodeFormState(actionResult, body); return { kind: "form-state", formState: formState ?? null }; } @@ -477,9 +468,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 +479,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 +492,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 +506,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-page-dispatch.test.ts b/tests/app-page-dispatch.test.ts index efd628f9e..dd1fbd5aa 100644 --- a/tests/app-page-dispatch.test.ts +++ b/tests/app-page-dispatch.test.ts @@ -74,6 +74,8 @@ function createDispatchOptions( clearRequestContext?: DispatchOptions["clearRequestContext"]; generateStaticParams?: DispatchOptions["generateStaticParams"]; formState?: DispatchOptions["formState"]; + actionError?: DispatchOptions["actionError"]; + actionFailed?: DispatchOptions["actionFailed"]; isProgressiveActionRender?: DispatchOptions["isProgressiveActionRender"]; isProduction?: boolean; isRscRequest?: boolean; @@ -126,6 +128,8 @@ function createDispatchOptions( hasPageModule: true, handlerStart: 10, formState: overrides.formState, + actionError: overrides.actionError, + actionFailed: overrides.actionFailed, interceptionContext: null, isProgressiveActionRender: overrides.isProgressiveActionRender, isProduction: overrides.isProduction ?? false, @@ -272,6 +276,31 @@ describe("app page dispatch", () => { await expect(response.text()).resolves.toBe("page"); }); + it("renders not-found HTML when a progressive action calls notFound()", async () => { + const buildPageElement = vi.fn(async () => React.createElement("main", null, "page")); + const renderHttpAccessFallbackPage = vi.fn( + async () => new Response("not found", { status: 404 }), + ); + const { options } = createDispatchOptions({ + actionError: { digest: "NEXT_HTTP_ERROR_FALLBACK;404" }, + actionFailed: true, + buildPageElement, + isProgressiveActionRender: true, + }); + options.renderHttpAccessFallbackPage = renderHttpAccessFallbackPage; + + const response = await dispatchAppPage(options); + + expect(response.status).toBe(404); + await expect(response.text()).resolves.toBe("not found"); + expect(buildPageElement).not.toHaveBeenCalled(); + expect(renderHttpAccessFallbackPage).toHaveBeenCalledWith( + 404, + { matchedParams: { slug: "hello" } }, + null, + ); + }); + it("does not bypass cached production HTML for arbitrary draft cookie values", async () => { vi.stubEnv("__VINEXT_DRAFT_SECRET", "draft-secret"); const isrGet = vi.fn(async () => diff --git a/tests/app-server-action-execution.test.ts b/tests/app-server-action-execution.test.ts index 70b02eec7..fc6402048 100644 --- a/tests/app-server-action-execution.test.ts +++ b/tests/app-server-action-execution.test.ts @@ -458,67 +458,95 @@ 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 }, + actionFailed: true, + }); 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( + 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(result).toEqual({ + kind: "form-state", + formState: null, + actionError: error, + actionFailed: true, + }); + 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(); + }); + + it("passes falsy action execution failures to the page render path", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + const result = await handleProgressiveServerActionRequest( createOptions({ - cleanPathname: "/action-source", - clearRequestContext: clearContext, async decodeAction() { return () => { - throw new Error("boom"); + throw 0; }; }, - getAndClearPendingCookies: clearedCookies, - reportRequestError(error) { - reportedErrors.push(error); - }, }), - ), - ); - - 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(); + expect(result).toEqual({ + kind: "form-state", + formState: null, + actionError: 0, + actionFailed: true, + }); + } finally { + errorSpy.mockRestore(); + } }); // Ported from Next.js: test/e2e/app-dir/no-server-actions/no-server-actions.test.ts