fix(actions): enforce serverActions.bodySizeLimit on fetch actions (#1828)

[james-elicx]

Problem

serverActions.bodySizeLimit was not enforced for fetch (client-invoked) server actions: oversized requests were rejected with a bare 413 Payload Too Large plain-text response. That bypasses the React client error boundary and surfaces the wrong status/content-type, so the four upstream suites failed:

• app-action-size-limit-invalid.test.ts — "should error for requests that exceed the size limit" (plaintext + multipart)
• app-action-size-limit-invalid-node-middleware.test.ts — same two, with node-runtime middleware

Fix

Next.js throws the body-exceeded error before the action runs and, for fetch actions, emits a 500 Flight response carrying the rejected action result so the nearest client error boundary catches it (renders the route's error.js). vinext now mirrors that:

• In handleServerActionRscRequest, both oversized-body checks (content-length pre-check and stream-read limit) now call a new renderFetchActionBodyExceededResponse, which renders a Flight stream with returnValue: { ok: false } and no page root (the action never ran, nothing revalidated), returning status 500 with the RSC content-type.
• The browser entry already throws normalizeServerActionThrownValue(...) when a fetch action result has returnValue.ok === false and no root, which propagates to the error boundary — no client change needed.
• The error message matches upstream (Body exceeded {limit} limit. + docs link) so it reads identically in logs.

The no-JS progressive form-POST path keeps its existing 413 — there is no client error boundary there to catch a Flight error, so a 413 is the right behavior.

Tests

Added two unit cases to tests/app-server-action-execution.test.ts covering the content-length and stream-limit triggers: assert status 500, RSC content-type, that the action is never loaded, and that the body-exceeded error is embedded in returnValue. Existing 413 assertions on the progressive path are unchanged.

vp check clean on both changed files; targeted suite (60 tests) green.

Closes #1828

[james-elicx]

/bigbonk review for issues

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@1834

npm i https://pkg.pr.new/vinext@1834

commit: 1ad3387

[ask-bonk]

Review: enforce serverActions.bodySizeLimit on fetch actions

The overall approach is right and matches Next.js: throw the body-exceeded error before the action runs, emit a 500 Flight response carrying returnValue: { ok: false } and no page root so the client error boundary catches it, and keep the 413 on the no-JS progressive form-POST path. The clearRequestContext deferral via createServerActionRscResponse (fires after the stream drains) is consistent with the other RSC action paths, and the unit tests are well-targeted.

However, there is one blocking correctness issue that means this PR likely does not actually pass the upstream suites it claims to fix.

Blocking: error message format does not match Next.js (breaks the upstream tests)

formatBodySizeLimit reconstructs a size label from the parsed byte count, producing values like 2 MB / 1 MB (uppercase unit, space-separated). Next.js emits the verbatim bodySizeLimit config string instead:

// next/src/server/app-render/action-handler.ts
`Body exceeded ${bodySizeLimit} limit.\n` + ...

The upstream suites this PR says it fixes assert the log message literally:

// test/e2e/app-dir/actions/app-action-size-limit-invalid.test.ts (and -node-middleware variant)
expect.stringContaining('Error: Body exceeded 2mb limit')   // config is bodySizeLimit: '2mb'

With bodySizeLimit: '2mb', vinext stores 2097152 bytes and formatBodySizeLimit(2097152) returns "2 MB", so the emitted message is Body exceeded 2 MB limit. — which does not contain Body exceeded 2mb limit. It diverges on both casing (MB vs mb) and spacing (2 MB vs 2mb). The default (1mb) has the same problem (1 MB vs 1mb).

The PR's own unit test only asserts the reconstructed form ("Body exceeded 10 B limit"), so it passes locally while the upstream e2e assertion would still fail. The 60-test targeted suite being green does not exercise the upstream message format.

Fix: thread the original config string through to the runtime (like Next.js does) instead of reconstructing it. The raw value is available at parse time in config/next-config.ts:1281 (serverActionsConfig?.bodySizeLimit) but is currently discarded after parseBodySizeLimit. Plumb a serverActionsBodySizeLimitLabel (the verbatim string, defaulting to "1mb" when unset) alongside the byte count through app-rsc-entry.ts -> HandleServerActionRscRequestOptions, and use it in createBodyExceededError. Then update the unit test to assert the verbatim string and, ideally, port the upstream e2e assertion so this regression is caught.

The code comment at app-server-action-execution.ts:409-411 already acknowledges this divergence ("Next.js surfaces the original config string here ... we reconstruct an equivalent label") — but the label is not actually equivalent, which is precisely the bug.

Non-blocking observations

• Parity check (good): the App Router prod server delegates to the built RSC entry, so this fix is inherited there. The Pages Router bodySizeLimit semantics are separate (API routes), so no change is needed in server/prod-server.ts for this. Worth confirming there's no other fetch-action 413 path left behind, but the two call sites here look complete.
• formatBodySizeLimit can be deleted once the config string is threaded through — it has no other callers and removing it avoids a second, subtly-wrong size formatter in the codebase.

Summary

Architecture and response shaping are correct. The single blocker is the error-message format mismatch, which means the stated upstream suites are not actually satisfied. Plumb the verbatim config string through rather than reconstructing it from bytes, then assert the verbatim string in tests.

[ask-bonk]

Review posted on PR #1834 (CHANGES_REQUESTED).

Summary

The PR's architecture is correct — it mirrors Next.js by emitting a 500 Flight response (returnValue: { ok: false }, no page root) for oversized fetch actions so the client error boundary catches it, while keeping 413 for the no-JS progressive path. The clearRequestContext deferral is consistent with the other RSC action paths.

One blocking issue found:

• Error message format diverges from Next.js, breaking the upstream tests it claims to fix. formatBodySizeLimit reconstructs a size label from bytes, yielding 2 MB / 1 MB. Next.js emits the verbatim config string (2mb / 1mb). The upstream suites (app-action-size-limit-invalid.test.ts and its node-middleware variant) assert stringContaining('Error: Body exceeded 2mb limit') — which Body exceeded 2 MB limit. does not satisfy (casing + spacing). The PR's own unit test only checks the reconstructed form ("10 B"), so it passes locally while the actual upstream assertion would still fail. Verified by computing formatBodySizeLimit(2097152) → "2 MB".

Fix: thread the original bodySizeLimit config string (available but discarded at config/next-config.ts:1281) through to the runtime and use it in createBodyExceededError, instead of reconstructing it; then assert the verbatim string and ideally port the upstream e2e assertion. The code comment at lines 409-411 even admits the divergence — but the reconstructed label is not actually equivalent.

I verified vp check is clean on both files and the targeted 60-test suite passes, but noted that green status doesn't cover the upstream message-format mismatch.

github run

[james-elicx]

Addressed the blocking finding in 54b2ee2.

The body-exceeded error now uses the verbatim serverActions.bodySizeLimit config string instead of reconstructing it from the parsed byte count. I plumbed a new serverActionsBodySizeLimitLabel (the raw config value, e.g. "2mb", defaulting to "1mb" when unset) through next-config.ts -> index.ts -> app-rsc-entry.ts -> HandleServerActionRscRequestOptions, and createBodyExceededError now emits Body exceeded 2mb limit. byte-for-byte, matching the upstream e2e log assertion. The lossy formatBodySizeLimit reconstructor is removed.

Tests updated: the execution unit test asserts the verbatim Body exceeded 2mb limit string, and next-config.test.ts locks in the label (string/numeric/default) preservation.

/bigbonk review for issues

[ask-bonk]

Re-review: verbatim bodySizeLimit string in body-exceeded error (commit 54b2ee2)

The blocking finding from the previous review is resolved correctly for the targeted suites. I verified against the upstream source (next/src/server/app-render/action-handler.ts) and the e2e test:

// action-handler.ts
const bodySizeLimit = serverActions?.bodySizeLimit ?? defaultBodySizeLimit
...
`Body exceeded ${bodySizeLimit} limit.\n` + ...

// test/e2e/app-dir/actions/app-action-size-limit-invalid.test.ts (config: bodySizeLimit: '2mb')
expect.stringContaining('Error: Body exceeded 2mb limit')

Next.js interpolates the raw config value verbatim, so '2mb' → Body exceeded 2mb limit.. The new serverActionsBodySizeLimitLabel plumbing produces exactly that, byte-for-byte. The lossy formatBodySizeLimit reconstructor is fully removed (no dangling references), and the unit test now asserts the verbatim Body exceeded 2mb limit instead of the old reconstructed 10 B form. I ran the targeted cases locally and confirmed the #1828 content-length/stream-limit tests and the three next-config label tests pass. The architecture (500 Flight response, returnValue: { ok: false }, deferred clearRequestContext) is unchanged and still correct.

Non-blocking: default label diverges from Next.js (1mb vs 1 MB)

When serverActions.bodySizeLimit is unset, this PR defaults serverActionsBodySizeLimitLabel to "1mb", so the error reads Body exceeded 1mb limit.. Next.js uses a different default literal:

const defaultBodySizeLimit = '1 MB'   // note: uppercase + space

so upstream emits Body exceeded 1 MB limit. for the unconfigured case. This is the same class of divergence (casing + spacing) that the original blocker was about — just on the default path instead of the configured one. The two suites this PR fixes both set '2mb', so they don't exercise the default and won't catch it, but any future test relying on the default-limit message (or anyone diffing logs against Next.js) would see a mismatch.

Suggested: default the label to "1 MB" (matching Next.js's defaultBodySizeLimit literal) rather than "1mb", and update the next-config default-case assertion to "1 MB". Low risk, keeps the message identical to upstream on every path.

Numeric config (good)

For a numeric bodySizeLimit (e.g. 5242880), String(5242880) → "5242880", which matches Next.js interpolating the number directly (Body exceeded 5242880 limit.). The next-config test locks this in.

Summary

The blocking issue is fixed for the suites in scope — verbatim string threading is the right approach and matches upstream. The only remaining nit is the default label (1mb should be 1 MB to match defaultBodySizeLimit); non-blocking since the targeted tests configure '2mb'.

[ask-bonk]

Review posted on PR #1834 (COMMENTED).

Summary

The blocking finding from the prior review is resolved correctly in commit 54b2ee2. I verified against the upstream Next.js source (action-handler.ts) and the e2e test fixture:

• Verbatim string threading is the right fix. Next.js interpolates the raw bodySizeLimit config value directly (Body exceeded ${bodySizeLimit} limit.), and the targeted suite configures '2mb' and asserts 'Error: Body exceeded 2mb limit'. The new serverActionsBodySizeLimitLabel plumbing (next-config → index → app-rsc-entry → handler options) emits exactly that, byte-for-byte.
• formatBodySizeLimit is fully removed — no dangling references. Tests now assert the verbatim string. I ran the #1828 and next-config label tests locally; all pass.
• Numeric config is correct — String(5242880) matches upstream's direct number interpolation.

One non-blocking divergence found

The default label diverges from Next.js. When bodySizeLimit is unset, this PR defaults to "1mb" (→ Body exceeded 1mb limit.), but Next.js uses const defaultBodySizeLimit = '1 MB' (uppercase + space → Body exceeded 1 MB limit.). This is the same casing/spacing class of mismatch as the original blocker, just on the default path. It's non-blocking because both suites in scope configure '2mb' and don't exercise the default — but I suggested defaulting to "1 MB" to keep the message identical to upstream on every path.

github run

[james-elicx]

Addressed the non-blocking default-label nit in 1ad3387.

When serverActions.bodySizeLimit is unset, the label now defaults to "1 MB" (matching Next.js' defaultBodySizeLimit = '1 MB' literal) instead of "1mb", so the unconfigured-case message reads Body exceeded 1 MB limit. byte-for-byte. Updated the next-config default-case assertion accordingly.

/bigbonk review for issues

[ask-bonk]

Re-review: default bodySizeLimit label → 1 MB (commit 1ad3387)

The non-blocking default-label nit from the previous re-review is resolved correctly. Verified the full chain and ran the targeted suites locally — all green.

What I checked

• Default literal matches Next.js. When serverActions.bodySizeLimit is unset, the label now defaults to "1 MB" (uppercase + space), matching Next.js' const defaultBodySizeLimit = '1 MB'. The unconfigured-case message now reads Body exceeded 1 MB limit. byte-for-byte.
• Default is consistent across both fallback sites. next-config.ts:1189 (object default) and next-config.ts:1295 (unset-branch), plus the runtime fallback at app-rsc-entry.ts:195 (config?.bodySizeLimitLabel ?? "1 MB"), all use "1 MB" — no remaining "1mb" default anywhere. The verbatim label threads cleanly: next-config → index.ts:2519 → app-rsc-entry (__MAX_ACTION_BODY_SIZE_LABEL) → maxActionBodySizeLabel → createBodyExceededError (app-server-action-execution.ts:931), which interpolates it verbatim.
• Configured/numeric paths untouched. "2mb" → "2mb" and 5242880 → "5242880" assertions are unchanged and still correct. The only remaining "1mb" reference is the parseBodySizeLimit("1mb") parse test (expected — that exercises parsing, not the label) and config-string examples in comments/error text.
• Tests updated. The next-config default-case assertion now expects "1 MB", and the detectNextIntlConfig expected-config object was updated to match.

Verification

• tests/next-config.test.ts (bodySizeLimit label cases) — pass
• tests/app-server-action-execution.test.ts (60 tests, incl. #1828 content-length/stream-limit) — pass
• vp check on all three changed files — clean (format, lint, types)

Summary

LGTM. The default path now matches upstream on every code path (configured, numeric, and unset). No further issues — nothing blocking.

[ask-bonk]

Review posted on PR #1834 (COMMENTED).

Summary

The non-blocking default-label nit from the prior re-review is resolved correctly in commit 1ad3387. No issues found.

• Default literal now matches Next.js. Unset serverActions.bodySizeLimit defaults the label to "1 MB" (matching defaultBodySizeLimit = '1 MB'), so the unconfigured message reads Body exceeded 1 MB limit. byte-for-byte.
• Consistent across all fallback sites — next-config.ts:1189, next-config.ts:1295, and the runtime fallback at app-rsc-entry.ts:195 all use "1 MB"; no stray "1mb" default remains. The only surviving "1mb" is the parseBodySizeLimit("1mb") parse test (correct — exercises parsing, not the label).
• Configured/numeric paths untouched ("2mb", "5242880").
• Verified locally: next-config label tests pass, app-server-action-execution.test.ts (60 tests) passes, and vp check is clean on all three changed files.

LGTM, nothing blocking.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Re-review (commit 1ad3387): verified against upstream Next.js source + e2e fixture

I traced the full flow end-to-end against the local Next.js canary clone and the actual upstream test this PR closes. No blocking issues. LGTM.

Confirmed correct against authoritative upstream

I verified each claim in the PR against action-handler.ts and the e2e fixture rather than relying on the green local suite:

• Status 500 is correct. test/e2e/app-dir/actions/app-action-size-limit-invalid.test.ts:139 asserts expect(actionResponse.status()).toBe(500) // TODO: 413?. Upstream throws ApiError(413, ...) at action-handler.ts:920, but the generic fetch-action catch (action-handler.ts:1257-1287) overrides it to res.statusCode = 500 and emits a Flight response with the rejected actionResult and skipPageRendering when nothing revalidated. vinext mirrors this exactly: 500, RSC content-type, returnValue: { ok: false }, no root.
• The server-log assertion is satisfied. The test asserts on server logs, not the client error object: logs must contain expect.stringContaining('Error: Body exceeded 2mb limit') and the docs-link string (lines 151-158). The PR's console.error("[vinext] Server action error:", error) with the verbatim-label message (Body exceeded 2mb limit.\nTo configure the body size limit...) produces exactly that. The verbatim 2mb threading (next-config -> index -> app-rsc-entry -> handler options) is the right fix and matches Body exceeded ${bodySizeLimit} limit. at action-handler.ts:922.
• Default label 1 MB matches upstream. action-handler.ts:901 uses const defaultBodySizeLimit = '1 MB'. The unset path now defaults to "1 MB" at next-config.ts (object default + unset-branch) and the runtime fallback in app-rsc-entry.ts. Numeric config (String(5242880)) also matches upstream's direct number interpolation. The next-config test locks all three (string/numeric/default).
• Error boundary still renders. Upstream asserts #error shows "Something went wrong!" (the route error.js). vinext's browser entry throws via normalizeServerActionThrownValue(...) for ok: false + no root (app-browser-entry.ts:1688-1690); since 500 is not a valid access-fallback status (only 401/403/404 per http-access-fallback.ts), it surfaces as a generic error and renders error.js. Correct outcome.
• 413 retained on the progressive (no-JS) path (app-server-action-execution.ts:744-748, 760-765). Correct — no client error boundary there to catch a Flight error.
• Dev/prod parity holds. App Router prod delegates to the built RSC entry, so the fix is inherited. There is no separate fetch-action 413 path in prod-server.ts/dev-server.ts (those handle Pages Router API-route limits separately), so nothing else needs updating.

Verification run locally

• tests/app-server-action-execution.test.ts (60 tests, incl. both #1828 cases) + tests/next-config.test.ts (171 tests) — 231 passed.
• vp check on all four changed source files — clean (format, lint, types).

Minor non-blocking observation (pre-existing, not introduced here)

For an RSC fetch-action response, Next.js's client reducer ignores the HTTP status and surfaces the real serialized action error from the Flight payload (response.a, server-action-reducer.ts:261). vinext's browser entry instead synthesizes a NEXT_HTTP_ERROR_FALLBACK;500 error and discards returnValue.data (app-browser-action-result.ts:88). The error boundary still renders correctly (so the upstream test passes), but the surfaced error object differs from Next.js. This is pre-existing browser-entry behavior unrelated to this PR and does not affect the suites in scope — worth a separate follow-up if exact client error-object parity is ever desired.

Nothing blocking. The fix is correct and faithfully mirrors upstream.

[ask-bonk]

Review posted on PR #1834 (COMMENTED).

Summary

I verified the final state (commit 1ad3387) end-to-end against the upstream Next.js canary source and the actual e2e fixture this PR closes, rather than relying on the green local suite. No blocking issues — LGTM.

Confirmed correct against authoritative upstream:

• Status 500 matches app-action-size-limit-invalid.test.ts:139 (toBe(500) // TODO: 413?). vinext mirrors the generic fetch-action error path in action-handler.ts:1257-1287 (500, RSC content-type, returnValue: { ok: false }, no page root).
• Server-log assertion satisfied — the upstream test asserts on server logs (stringContaining('Error: Body exceeded 2mb limit') + docs link), which the PR's console.error with the verbatim-label message produces byte-for-byte. The 2mb/1 MB/numeric label threading matches Body exceeded ${bodySizeLimit} limit. and defaultBodySizeLimit = '1 MB'.
• Error boundary renders — vinext throws for ok: false + no root; 500 is not a valid access-fallback status, so it surfaces as a generic error and renders error.js (the test's "Something went wrong!" assertion).
• 413 retained on the no-JS progressive path; dev/prod parity holds (App Router prod inherits the fix via the built RSC entry).

Verification: 231 tests pass across both targeted suites; vp check clean on all four source files.

One non-blocking observation (pre-existing, not introduced by this PR): vinext's browser entry synthesizes a NEXT_HTTP_ERROR_FALLBACK;500 error and discards returnValue.data, whereas Next.js surfaces the real serialized action error from the Flight payload. The error boundary still renders correctly so the suites pass, but exact client error-object parity differs — a candidate for a separate follow-up.

github run

stale