fix(actions): return 404 for MPA action on app with no server actions (#1340) - #1853
Conversation
…#1340) A multipart form POST to an App Router page is always a server-action attempt. When the form body decodes to no action at all (e.g. the build has no server actions, so decodeAction returns null rather than throwing), vinext fell through to a 200 page render instead of Next.js' 404 + x-nextjs-action-not-found. The fetch-action variant (via the Next-Action header) already worked; only the MPA/no-JS form-POST variant was wrong. Gate the new 404 on the posted-to route being a page: route handlers run after action dispatch and legitimately accept raw multipart POSTs, so they still fall through. The page-vs-route distinction uses the static __loadPage / __loadRouteHandler markers, available before lazy module hydration. Finishes the no-server-actions MPA case of #1340 (error-boundary, content-type, and unrecognized-action handling landed in #1386/#1668).
|
/bigbonk review for issues |
commit: |
|
Review: fix(actions): return 404 for MPA action on app with no server actions (#1340)I reviewed the diff, traced the surrounding action-dispatch code, confirmed parity against the upstream Next.js test, and ran the relevant suites. This is a solid, well-targeted fix. No blocking issues. Verification performed
What's correct
Non-blocking observations
ConclusionBehavior matches Next.js, tests are well-chosen and ported from upstream, the route-handler fall-through guard is a correct vinext-specific addition, and the layering is clean. LGTM. |
|
Review posted to PR #1853. SummaryI reviewed PR #1853 (fix: return 404 for MPA action on app with no server actions, #1340). The fix is solid and well-targeted — LGTM with no blocking issues. What I verified:
Key correctness findings:
Two non-blocking observations:
|
…te match (#1340) Address ask-bonk review on #1853: - Add a dev-server integration test that POSTs multipart/form-data to a page (/about) and asserts 404 + x-nextjs-action-not-found, locking in the entry-side matchRoute + __loadPage/__loadRouteHandler classification end-to-end (previously only unit-tested by passing hasPageRoute directly). - Gate the progressive-action route match on isProgressiveServerActionRequest so matchRoute no longer runs on every App Router request — only for actual multipart/no-actionId POST candidates.
|
Thanks for the review. Addressed both non-blocking observations in b62af5d:
|
|
/bigbonk review for issues |
Re-review (post b62af5d): fix(actions): 404 for MPA action on app with no server actions (#1340)I independently re-reviewed the diff after the follow-up commit, traced the dispatch path through Verification performed
Correctness findings
Non-blocking observations
ConclusionBehavior matches Next.js, the page-vs-route gating is correct and now covered both at the unit and integration level, layering follows AGENTS.md (entry stays thin; decision lives in the typed server helper), and there's no regression for valid actions or route handlers. LGTM. |
|
Review posted to PR #1853. SummaryI re-reviewed PR #1853 (fix: 404 for MPA action on app with no server actions, #1340) including the follow-up commit Verified:
Key correctness findings:
One non-blocking observation: |
Finishes the no-server-actions MPA case of #1340 (error-boundary/content-type/unrecognized landed in #1386/#1668).
Problem
When an app has no server actions and a non-fetch MPA / no-JS form POST (
multipart/form-data) is submitted to a page, vinext rendered the page with 200 instead of Next.js' 404 +x-nextjs-action-not-found: 1. The fetch-action variant (via theNext-Actionheader) already returned 404; only the MPA/form-POST variant on a no-server-actions app was wrong.Root cause: a multipart form POST to an App Router page is always a server-action attempt. When the form body decodes to no action at all, React's
decodeActionreturnsnull(it does not throw), so the progressive handler fell through to a normal page render.Fix
In the progressive (no-JS) action path, when
decodeActionyields no action and the posted-to route is a page, return the Next.js action-not-found response (404 +x-nextjs-action-not-found: 1) and logFailed to find Server Action. This request might be from an older or newer deployment., mirroring Next.js.The 404 is gated on the route being a page: route handlers (
route.ts) are dispatched after action handling in vinext and legitimately accept raw multipart POSTs, so they still fall through. The page-vs-route distinction uses the static__loadPage/__loadRouteHandlermarkers, available before lazy module hydration.Tests
Ported from upstream
test/e2e/app-dir/no-server-actions/no-server-actions.test.ts("should error when triggering an MPA action on an app with no server actions") intotests/app-server-action-execution.test.ts:Closes #1340