Fix Server Action error logs for unhandled POST requests (#64315)

## Why

Currently, Server Action handlers are just normal routes and they
accepting POST requests. The only way to differentiate a normal request
(page access, API route, etc.) from a Server Action request is to check
that if it's a POST and has a Server Action ID set via headers or body
(e.g. `multipart/form-data`).

Usually, for existing page and API routes the correct handlers (page
renderer, API route handler) will take precedence over Server Action's.
But if the route doesn't exist (e.g. 404) it will still go through
Server Action's handler and result in an error. And we're eagerly
logging out that error because it might be an application failure, like
passing a wrong Action ID.

## How

In this PR we are making sure that the error is only logged if the
Action ID isn't `null`. This means that it's an intentional Server
Action request with a wrong ID. If the ID is `null`, we just handle it
like 404 and log nothing.

Fixes #64214.

Closes NEXT-3071
This commit is contained in:
Shu Ding 2024-04-12 01:21:28 +02:00 committed by GitHub
parent 6d96c3f468
commit 34524f01ad
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 64 additions and 15 deletions

View file

@ -36,10 +36,7 @@ import {
NEXT_CACHE_REVALIDATED_TAGS_HEADER,
NEXT_CACHE_REVALIDATE_TAG_TOKEN_HEADER,
} from '../../lib/constants'
import {
getIsServerAction,
getServerActionRequestMetadata,
} from '../lib/server-action-request-meta'
import { getServerActionRequestMetadata } from '../lib/server-action-request-meta'
import { isCsrfOriginAllowed } from './csrf-protection'
import { warn } from '../../build/output/log'
import { RequestCookies, ResponseCookies } from '../web/spec-extension/cookies'
@ -399,11 +396,16 @@ export async function handleAction({
const contentType = req.headers['content-type']
const { serverActionsManifest, page } = ctx.renderOpts
const { actionId, isURLEncodedAction, isMultipartAction, isFetchAction } =
getServerActionRequestMetadata(req)
const {
actionId,
isURLEncodedAction,
isMultipartAction,
isFetchAction,
isServerAction,
} = getServerActionRequestMetadata(req)
// If it's not a Server Action, skip handling.
if (!getIsServerAction(req)) {
if (!isServerAction) {
return
}
@ -578,7 +580,9 @@ export async function handleAction({
try {
actionModId = getActionModIdOrError(actionId, serverModuleMap)
} catch (err) {
console.error(err)
if (actionId !== null) {
console.error(err)
}
return {
type: 'not-found',
}
@ -667,7 +671,9 @@ export async function handleAction({
try {
actionModId = getActionModIdOrError(actionId, serverModuleMap)
} catch (err) {
console.error(err)
if (actionId !== null) {
console.error(err)
}
return {
type: 'not-found',
}
@ -718,7 +724,9 @@ To configure the body size limit for Server Actions, see: https://nextjs.org/doc
actionModId =
actionModId ?? getActionModIdOrError(actionId, serverModuleMap)
} catch (err) {
console.error(err)
if (actionId !== null) {
console.error(err)
}
return {
type: 'not-found',
}

View file

@ -10,6 +10,7 @@ export function getServerActionRequestMetadata(
isURLEncodedAction: boolean
isMultipartAction: boolean
isFetchAction: boolean
isServerAction: boolean
} {
let actionId: string | null
let contentType: string | null
@ -34,14 +35,21 @@ export function getServerActionRequestMetadata(
req.method === 'POST'
)
return { actionId, isURLEncodedAction, isMultipartAction, isFetchAction }
const isServerAction = Boolean(
isFetchAction || isURLEncodedAction || isMultipartAction
)
return {
actionId,
isURLEncodedAction,
isMultipartAction,
isFetchAction,
isServerAction,
}
}
export function getIsServerAction(
req: IncomingMessage | BaseNextRequest | NextRequest
): boolean {
const { isFetchAction, isURLEncodedAction, isMultipartAction } =
getServerActionRequestMetadata(req)
return Boolean(isFetchAction || isURLEncodedAction || isMultipartAction)
return getServerActionRequestMetadata(req).isServerAction
}

View file

@ -139,6 +139,32 @@ createNextDescribe(
).toBeGreaterThanOrEqual(currentTimestamp)
})
it('should not log errors for non-action form POSTs', async () => {
const logs: string[] = []
next.on('stdout', (log) => {
logs.push(log)
})
next.on('stderr', (log) => {
logs.push(log)
})
const browser = await next.browser('/non-action-form')
await browser.elementByCss('button').click()
await check(() => browser.url(), next.url + '/', true, 2)
// we don't have access to runtime logs on deploy
if (!isNextDeploy) {
await check(() => {
return logs.some((log) =>
log.includes('Failed to find Server Action "null"')
)
? 'error'
: ''
}, '')
}
})
it('should support setting cookies in route handlers with the correct overrides', async () => {
const res = await next.fetch('/handler')
const setCookieHeader = res.headers.get('set-cookie')

View file

@ -0,0 +1,7 @@
export default function Page() {
return (
<form action="/" method="POST">
<button type="submit">Submit</button>
</form>
)
}