From 87e45521d2e2c42623e054822128ae68cd0edc5f Mon Sep 17 00:00:00 2001
From: Zack Tanner <1939140+ztanner@users.noreply.github.com>
Date: Fri, 28 Jun 2024 11:18:52 +0200
Subject: [PATCH] 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.
---
 .../reducers/server-action-reducer.ts         | 25 +++++----
 .../src/server/app-render/action-handler.ts   |  2 +-
 test/e2e/app-dir/actions/app-action.test.ts   | 52 ++++++++++++++++++-
 .../actions/app/error-handling/error.js       | 17 ++++++
 .../actions/app/error-handling/page.js        | 12 +++++
 5 files changed, 97 insertions(+), 11 deletions(-)
 create mode 100644 test/e2e/app-dir/actions/app/error-handling/error.js

diff --git a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts
index 2d580a4a0f..4626db0dec 100644
--- a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts
+++ b/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts
@@ -106,10 +106,9 @@ async function fetchServerAction(
       )
     : undefined
 
-  let isFlightResponse =
-    res.headers.get('content-type') === RSC_CONTENT_TYPE_HEADER
+  const contentType = res.headers.get('content-type')
 
-  if (isFlightResponse) {
+  if (contentType === RSC_CONTENT_TYPE_HEADER) {
     const response: ActionFlightResponse = await createFromFetch(
       Promise.resolve(res),
       {
@@ -136,6 +135,19 @@ async function fetchServerAction(
       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 {
     redirectLocation,
     revalidatedParts,
@@ -167,9 +179,7 @@ export function serverActionReducer(
       ? state.nextUrl
       : null
 
-  mutable.inFlightServerAction = fetchServerAction(state, nextUrl, action)
-
-  return mutable.inFlightServerAction.then(
+  return fetchServerAction(state, nextUrl, action).then(
     async ({
       actionResult,
       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) {
         const newHref = createHrefFromUrl(redirectLocation, false)
         mutable.canonicalUrl = newHref
diff --git a/packages/next/src/server/app-render/action-handler.ts b/packages/next/src/server/app-render/action-handler.ts
index 72cf877ed1..fbca7f88c9 100644
--- a/packages/next/src/server/app-render/action-handler.ts
+++ b/packages/next/src/server/app-render/action-handler.ts
@@ -251,7 +251,7 @@ function getAppRelativeRedirectUrl(
   host: Host,
   redirectUrl: string
 ): URL | null {
-  if (redirectUrl.startsWith('/')) {
+  if (redirectUrl.startsWith('/') || redirectUrl.startsWith('./')) {
     // Make sure we are appending the basePath to relative URLS
     return new URL(`${basePath}${redirectUrl}`, 'http://n')
   }
diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts
index fec271dcbf..bc11adb5a4 100644
--- a/test/e2e/app-dir/actions/app-action.test.ts
+++ b/test/e2e/app-dir/actions/app-action.test.ts
@@ -7,7 +7,7 @@ import {
   waitFor,
   getRedboxSource,
 } 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 { join } from 'path'
 
@@ -91,6 +91,56 @@ describe('app-dir action handling', () => {
     ).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 () => {
     const browser = await next.browser('/header')
 
diff --git a/test/e2e/app-dir/actions/app/error-handling/error.js b/test/e2e/app-dir/actions/app/error-handling/error.js
new file mode 100644
index 0000000000..436e4a1e3c
--- /dev/null
+++ b/test/e2e/app-dir/actions/app/error-handling/error.js
@@ -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>
+  )
+}
diff --git a/test/e2e/app-dir/actions/app/error-handling/page.js b/test/e2e/app-dir/actions/app/error-handling/page.js
index 56cd1db6fa..2c93a95f0b 100644
--- a/test/e2e/app-dir/actions/app/error-handling/page.js
+++ b/test/e2e/app-dir/actions/app/error-handling/page.js
@@ -1,8 +1,11 @@
 'use client'
 
+import { useTransition } from 'react'
 import { action } from './actions'
 
 export default function Page() {
+  const [, startTransition] = useTransition()
+
   return (
     <main>
       <p>
@@ -18,6 +21,15 @@ export default function Page() {
       >
         Submit
       </button>
+
+      <button
+        id="submit-transition"
+        onClick={async () => {
+          startTransition(() => action())
+        }}
+      >
+        Action that triggers an error
+      </button>
     </main>
   )
 }