feat: decouple cookies and action state from redirect error

packages/next/src/client/components/redirect.ts

@@ -1,5 +1,3 @@
-import { requestAsyncStorage } from './request-async-storage.external'
-import type { ResponseCookies } from '../../server/web/spec-extension/cookies'
 import { actionAsyncStorage } from './action-async-storage.external'
 import { RedirectStatusCode } from './redirect-status-code'
 
@@ -10,22 +8,17 @@ export enum RedirectType {
   replace = 'replace',
 }
 
-export type RedirectError<U extends string> = Error & {
-  digest: `${typeof REDIRECT_ERROR_CODE};${RedirectType};${U};${RedirectStatusCode};`
-  mutableCookies: ResponseCookies
+export type RedirectError = Error & {
+  digest: `${typeof REDIRECT_ERROR_CODE};${RedirectType};${string};${RedirectStatusCode};`
 }
 
 export function getRedirectError(
   url: string,
   type: RedirectType,
   statusCode: RedirectStatusCode = RedirectStatusCode.TemporaryRedirect
-): RedirectError<typeof url> {
-  const error = new Error(REDIRECT_ERROR_CODE) as RedirectError<typeof url>
+): RedirectError {
+  const error = new Error(REDIRECT_ERROR_CODE) as RedirectError
   error.digest = `${REDIRECT_ERROR_CODE};${type};${url};${statusCode};`
-  const requestStore = requestAsyncStorage.getStore()
-  if (requestStore) {
-    error.mutableCookies = requestStore.mutableCookies
-  }
   return error
 }
 
@@ -52,12 +45,7 @@ export function redirect(
   throw getRedirectError(
     url,
     redirectType,
-    // If we're in an action, we want to use a 303 redirect
-    // as we don't want the POST request to follow the redirect,
-    // as it could result in erroneous re-submissions.
-    actionStore?.isAction
-      ? RedirectStatusCode.SeeOther
-      : RedirectStatusCode.TemporaryRedirect
+    RedirectStatusCode.TemporaryRedirect
   )
 }
 
@@ -77,17 +65,7 @@ export function permanentRedirect(
   url: string,
   type: RedirectType = RedirectType.replace
 ): never {
-  const actionStore = actionAsyncStorage.getStore()
-  throw getRedirectError(
-    url,
-    type,
-    // If we're in an action, we want to use a 303 redirect
-    // as we don't want the POST request to follow the redirect,
-    // as it could result in erroneous re-submissions.
-    actionStore?.isAction
-      ? RedirectStatusCode.SeeOther
-      : RedirectStatusCode.PermanentRedirect
-  )
+  throw getRedirectError(url, type, RedirectStatusCode.PermanentRedirect)
 }
 
 /**
@@ -97,9 +75,7 @@ export function permanentRedirect(
  * @param error the error that may reference a redirect error
  * @returns true if the error is a redirect error
  */
-export function isRedirectError<U extends string>(
-  error: unknown
-): error is RedirectError<U> {
+export function isRedirectError(error: unknown): error is RedirectError {
   if (
     typeof error !== 'object' ||
     error === null ||
@@ -132,9 +108,7 @@ export function isRedirectError<U extends string>(
  * @param error the error that may be a redirect error
  * @return the url if the error was a redirect error
  */
-export function getURLFromRedirectError<U extends string>(
-  error: RedirectError<U>
-): U
+export function getURLFromRedirectError(error: RedirectError): string
 export function getURLFromRedirectError(error: unknown): string | null {
   if (!isRedirectError(error)) return null
 
@@ -143,19 +117,15 @@ export function getURLFromRedirectError(error: unknown): string | null {
   return error.digest.split(';').slice(2, -2).join(';')
 }
 
-export function getRedirectTypeFromError<U extends string>(
-  error: RedirectError<U>
-): RedirectType {
+export function getRedirectTypeFromError(error: RedirectError): RedirectType {
   if (!isRedirectError(error)) {
     throw new Error('Not a redirect error')
   }
 
   return error.digest.split(';', 2)[1] as RedirectType
 }
 
-export function getRedirectStatusCodeFromError<U extends string>(
-  error: RedirectError<U>
-): number {
+export function getRedirectStatusCodeFromError(error: RedirectError): number {
   if (!isRedirectError(error)) {
     throw new Error('Not a redirect error')
   }

packages/next/src/server/api-utils/index.ts

@@ -25,7 +25,7 @@ export function wrapApiHandler<T extends (...args: any[]) => any>(
   handler: T
 ): T {
   return ((...args) => {
-    getTracer().getRootSpanAttributes()?.set('next.route', page)
+    getTracer().setRootSpanAttribute('next.route', page)
     // Call API route method
     return getTracer().trace(
       NodeSpan.runHandler,

packages/next/src/server/app-render/action-handler.ts

@@ -13,7 +13,6 @@ import {
 } from '../../client/components/app-router-headers'
 import { isNotFoundError } from '../../client/components/not-found'
 import {
-  getRedirectStatusCodeFromError,
   getRedirectTypeFromError,
   getURLFromRedirectError,
   isRedirectError,
@@ -43,6 +42,7 @@ import { HeadersAdapter } from '../web/spec-extension/adapters/headers'
 import { fromNodeOutgoingHttpHeaders } from '../web/utils'
 import { selectWorkerForForwarding } from './action-utils'
 import { isNodeNextRequest, isWebNextRequest } from '../base-http/helpers'
+import { RedirectStatusCode } from '../../client/components/redirect-status-code'
 
 function formDataFromSearchQueryString(query: string) {
   const searchParams = new URLSearchParams(query)
@@ -855,7 +855,6 @@ export async function handleAction({
   } catch (err) {
     if (isRedirectError(err)) {
       const redirectUrl = getURLFromRedirectError(err)
-      const statusCode = getRedirectStatusCodeFromError(err)
       const redirectType = getRedirectTypeFromError(err)
 
       await addRevalidationHeader(res, {
@@ -865,7 +864,7 @@ export async function handleAction({
 
       // if it's a fetch action, we'll set the status code for logging/debugging purposes
       // but we won't set a Location header, as the redirect will be handled by the client router
-      res.statusCode = statusCode
+      res.statusCode = RedirectStatusCode.SeeOther
 
       if (isFetchAction) {
         return {
@@ -882,14 +881,11 @@ export async function handleAction({
         }
       }
 
-      if (err.mutableCookies) {
-        const headers = new Headers()
-
-        // If there were mutable cookies set, we need to set them on the
-        // response.
-        if (appendMutableCookies(headers, err.mutableCookies)) {
-          res.setHeader('set-cookie', Array.from(headers.values()))
-        }
+      // If there were mutable cookies set, we need to set them on the
+      // response.
+      const headers = new Headers()
+      if (appendMutableCookies(headers, requestStore.mutableCookies)) {
+        res.setHeader('set-cookie', Array.from(headers.values()))
       }
 
       res.setHeader('Location', redirectUrl)

packages/next/src/server/app-render/app-render.tsx

@@ -1043,7 +1043,7 @@ async function renderToHTMLOrFlightImpl(
     res,
   }
 
-  getTracer().getRootSpanAttributes()?.set('next.route', pagePath)
+  getTracer().setRootSpanAttribute('next.route', pagePath)
 
   if (isStaticGeneration) {
     // We're either building or revalidating. In either case we need to
@@ -1576,40 +1576,32 @@ async function renderToStream(
       throw err
     }
 
+    let errorType: 'not-found' | 'redirect' | undefined
+
     if (isNotFoundError(err)) {
+      errorType = 'not-found'
       res.statusCode = 404
-    }
-    let hasRedirectError = false
-    if (isRedirectError(err)) {
-      hasRedirectError = true
+    } else if (isRedirectError(err)) {
+      errorType = 'redirect'
       res.statusCode = getRedirectStatusCodeFromError(err)
-      if (err.mutableCookies) {
-        const headers = new Headers()
 
-        // If there were mutable cookies set, we need to set them on the
-        // response.
-        if (appendMutableCookies(headers, err.mutableCookies)) {
-          setHeader('set-cookie', Array.from(headers.values()))
-        }
-      }
       const redirectUrl = addPathPrefix(
         getURLFromRedirectError(err),
         renderOpts.basePath
       )
-      setHeader('Location', redirectUrl)
-    }
 
-    const is404 = res.statusCode === 404
-    if (!is404 && !hasRedirectError && !shouldBailoutToCSR) {
+      // If there were mutable cookies set, we need to set them on the
+      // response.
+      const headers = new Headers()
+      if (appendMutableCookies(headers, ctx.requestStore.mutableCookies)) {
+        setHeader('set-cookie', Array.from(headers.values()))
+      }
+
+      setHeader('location', redirectUrl)
+    } else if (!shouldBailoutToCSR) {
       res.statusCode = 500
     }
 
-    const errorType = is404
-      ? 'not-found'
-      : hasRedirectError
-        ? 'redirect'
-        : undefined
-
     const [errorPreinitScripts, errorBootstrapScript] = getRequiredScripts(
       renderOpts.buildManifest,
       ctx.assetPrefix,
@@ -2724,46 +2716,38 @@ async function prerenderToStream(
       throw err
     }
 
+    // If we errored when we did not have an RSC stream to read from. This is
+    // not just a render error, we need to throw early.
+    if (reactServerPrerenderResult === null) {
+      throw err
+    }
+
+    let errorType: 'not-found' | 'redirect' | undefined
+
     if (isNotFoundError(err)) {
+      errorType = 'not-found'
       res.statusCode = 404
-    }
-    let hasRedirectError = false
-    if (isRedirectError(err)) {
-      hasRedirectError = true
+    } else if (isRedirectError(err)) {
+      errorType = 'redirect'
       res.statusCode = getRedirectStatusCodeFromError(err)
-      if (err.mutableCookies) {
-        const headers = new Headers()
 
-        // If there were mutable cookies set, we need to set them on the
-        // response.
-        if (appendMutableCookies(headers, err.mutableCookies)) {
-          setHeader('set-cookie', Array.from(headers.values()))
-        }
-      }
       const redirectUrl = addPathPrefix(
         getURLFromRedirectError(err),
         renderOpts.basePath
       )
-      setHeader('Location', redirectUrl)
-    }
 
-    const is404 = res.statusCode === 404
-    if (!is404 && !hasRedirectError && !shouldBailoutToCSR) {
-      res.statusCode = 500
-    }
+      // If there were mutable cookies set, we need to set them on the
+      // response.
+      const headers = new Headers()
+      if (appendMutableCookies(headers, ctx.requestStore.mutableCookies)) {
+        setHeader('set-cookie', Array.from(headers.values()))
+      }
 
-    if (reactServerPrerenderResult === null) {
-      // We errored when we did not have an RSC stream to read from. This is not just a render
-      // error, we need to throw early
-      throw err
+      setHeader('location', redirectUrl)
+    } else if (!shouldBailoutToCSR) {
+      res.statusCode = 500
     }
 
-    const errorType = is404
-      ? 'not-found'
-      : hasRedirectError
-        ? 'redirect'
-        : undefined
-
     const [errorPreinitScripts, errorBootstrapScript] = getRequiredScripts(
       renderOpts.buildManifest,
       ctx.assetPrefix,

packages/next/src/server/base-server.ts

@@ -123,10 +123,6 @@ import { getTracer, isBubbledError, SpanKind } from './lib/trace/tracer'
 import { BaseServerSpan } from './lib/trace/constants'
 import { I18NProvider } from './lib/i18n-provider'
 import { sendResponse } from './send-response'
-import {
-  handleBadRequestResponse,
-  handleInternalServerErrorResponse,
-} from './route-modules/helpers/response-handlers'
 import {
   fromNodeOutgoingHttpHeaders,
   normalizeNextQueryParam,
@@ -2587,7 +2583,7 @@ export default abstract class Server<
             Log.error(err)
 
             // Otherwise, send a 500 response.
-            await sendResponse(req, res, handleInternalServerErrorResponse())
+            await sendResponse(req, res, new Response(null, { status: 500 }))
 
             return null
           }
@@ -2597,7 +2593,7 @@ export default abstract class Server<
         ) {
           // An OPTIONS request to a page handler is invalid.
           if (req.method === 'OPTIONS' && !is404Page) {
-            await sendResponse(req, res, handleBadRequestResponse())
+            await sendResponse(req, res, new Response(null, { status: 400 }))
             return null
           }
 
@@ -3575,7 +3571,7 @@ export default abstract class Server<
       shouldEnsure: false,
     })
     if (result) {
-      getTracer().getRootSpanAttributes()?.set('next.route', pathname)
+      getTracer().setRootSpanAttribute('next.route', pathname)
       try {
         return await this.renderToResponseWithComponents(ctx, result)
       } catch (err) {

packages/next/src/server/lib/trace/tracer.ts

@@ -450,6 +450,14 @@ class NextTracerImpl implements NextTracer {
     const spanId = context.active().getValue(rootSpanIdKey) as number
     return rootSpanAttributesStore.get(spanId)
   }
+
+  public setRootSpanAttribute(key: AttributeNames, value: AttributeValue) {
+    const spanId = context.active().getValue(rootSpanIdKey) as number
+    const attributes = rootSpanAttributesStore.get(spanId)
+    if (attributes) {
+      attributes.set(key, value)
+    }
+  }
 }
 
 const getTracer = (() => {

packages/next/src/server/render.tsx

@@ -1391,7 +1391,7 @@ export async function renderToHTMLImpl(
     }
   }
 
-  getTracer().getRootSpanAttributes()?.set('next.route', renderOpts.page)
+  getTracer().setRootSpanAttribute('next.route', renderOpts.page)
   const documentResult = await getTracer().trace(
     RenderSpan.renderDocument,
     {

packages/next/src/server/route-modules/app-route/helpers/auto-implement-methods.ts

@@ -1,10 +1,13 @@
 import type { AppRouteHandlerFn, AppRouteHandlers } from '../module'
 
 import { HTTP_METHODS, type HTTP_METHOD } from '../../../web/http'
-import { handleMethodNotAllowedResponse } from '../../helpers/response-handlers'
 
 const AUTOMATIC_ROUTE_METHODS = ['HEAD', 'OPTIONS'] as const
 
+function handleMethodNotAllowedResponse(): Response {
+  return new Response(null, { status: 405 })
+}
+
 export function autoImplementMethods(
   handlers: AppRouteHandlers
 ): Record<HTTP_METHOD, AppRouteHandlerFn> {