reject actions with invalid RSC responses (#67059)

When a server action responds with something other than an RSC response,
we currently silently ignore the error and it doesn't get propagated to
any rejection handlers.

This adjusts the handling so that if the server action response is a
non-successful status code, we reject the server action promise. If the
error is `text/plain`, we'll automatically propagate the text content as
the error text. Otherwise, the promise is rejected with a fallback
message.
This commit is contained in:
Zack Tanner 2024-06-28 11:18:52 +02:00 committed by GitHub
parent b2eafbf6b3
commit 87e45521d2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 97 additions and 11 deletions

View file

@ -106,10 +106,9 @@ async function fetchServerAction(
) )
: undefined : undefined
let isFlightResponse = const contentType = res.headers.get('content-type')
res.headers.get('content-type') === RSC_CONTENT_TYPE_HEADER
if (isFlightResponse) { if (contentType === RSC_CONTENT_TYPE_HEADER) {
const response: ActionFlightResponse = await createFromFetch( const response: ActionFlightResponse = await createFromFetch(
Promise.resolve(res), Promise.resolve(res),
{ {
@ -136,6 +135,19 @@ async function fetchServerAction(
revalidatedParts, revalidatedParts,
} }
} }
// Handle invalid server action responses
if (res.status >= 400) {
// The server can respond with a text/plain error message, but we'll fallback to something generic
// if there isn't one.
const error =
contentType === 'text/plain'
? await res.text()
: 'An unexpected response was received from the server.'
throw new Error(error)
}
return { return {
redirectLocation, redirectLocation,
revalidatedParts, revalidatedParts,
@ -167,9 +179,7 @@ export function serverActionReducer(
? state.nextUrl ? state.nextUrl
: null : null
mutable.inFlightServerAction = fetchServerAction(state, nextUrl, action) return fetchServerAction(state, nextUrl, action).then(
return mutable.inFlightServerAction.then(
async ({ async ({
actionResult, actionResult,
actionFlightData: flightData, actionFlightData: flightData,
@ -207,9 +217,6 @@ export function serverActionReducer(
) )
} }
// Remove cache.data as it has been resolved at this point.
mutable.inFlightServerAction = null
if (redirectLocation) { if (redirectLocation) {
const newHref = createHrefFromUrl(redirectLocation, false) const newHref = createHrefFromUrl(redirectLocation, false)
mutable.canonicalUrl = newHref mutable.canonicalUrl = newHref

View file

@ -251,7 +251,7 @@ function getAppRelativeRedirectUrl(
host: Host, host: Host,
redirectUrl: string redirectUrl: string
): URL | null { ): URL | null {
if (redirectUrl.startsWith('/')) { if (redirectUrl.startsWith('/') || redirectUrl.startsWith('./')) {
// Make sure we are appending the basePath to relative URLS // Make sure we are appending the basePath to relative URLS
return new URL(`${basePath}${redirectUrl}`, 'http://n') return new URL(`${basePath}${redirectUrl}`, 'http://n')
} }

View file

@ -7,7 +7,7 @@ import {
waitFor, waitFor,
getRedboxSource, getRedboxSource,
} from 'next-test-utils' } from 'next-test-utils'
import type { Request, Response, Route } from 'playwright' import type { Page, Request, Response, Route } from 'playwright'
import fs from 'fs-extra' import fs from 'fs-extra'
import { join } from 'path' import { join } from 'path'
@ -91,6 +91,56 @@ describe('app-dir action handling', () => {
).toBe(true) ).toBe(true)
}) })
it('should propagate errors from a `text/plain` response to an error boundary', async () => {
const customErrorText = 'Custom error!'
const browser = await next.browser('/error-handling', {
beforePageLoad(page: Page) {
page.route('**/error-handling', async (route: Route) => {
const requestHeaders = await route.request().allHeaders()
if (requestHeaders['next-action']) {
await route.fulfill({
status: 500,
contentType: 'text/plain',
body: customErrorText,
})
} else {
await route.continue()
}
})
},
})
await browser.elementById('submit-transition').click()
const error = await browser.waitForElementByCss('#error-text')
expect(await error.text()).toBe(customErrorText)
})
it('should trigger an error boundary for action responses with an invalid content-type', async () => {
const customErrorText = 'Custom error!'
const browser = await next.browser('/error-handling', {
beforePageLoad(page: Page) {
page.route('**/error-handling', async (route: Route) => {
const requestHeaders = await route.request().allHeaders()
if (requestHeaders['next-action']) {
await route.fulfill({
status: 500,
contentType: 'application/json',
body: JSON.stringify({ error: customErrorText }),
})
} else {
await route.continue()
}
})
},
})
await browser.elementById('submit-transition').click()
const error = await browser.waitForElementByCss('#error-text')
expect(await error.text()).toBe(
'An unexpected response was received from the server.'
)
})
it('should support headers and cookies', async () => { it('should support headers and cookies', async () => {
const browser = await next.browser('/header') const browser = await next.browser('/header')

View file

@ -0,0 +1,17 @@
'use client' // Error components must be Client Components
export default function Error({ error, reset }) {
return (
<div>
<h2 id="error-text">{error.message}</h2>
<button
onClick={
// Attempt to recover by trying to re-render the segment
() => reset()
}
>
Try again
</button>
</div>
)
}

View file

@ -1,8 +1,11 @@
'use client' 'use client'
import { useTransition } from 'react'
import { action } from './actions' import { action } from './actions'
export default function Page() { export default function Page() {
const [, startTransition] = useTransition()
return ( return (
<main> <main>
<p> <p>
@ -18,6 +21,15 @@ export default function Page() {
> >
Submit Submit
</button> </button>
<button
id="submit-transition"
onClick={async () => {
startTransition(() => action())
}}
>
Action that triggers an error
</button>
</main> </main>
) )
} }