fix: only set request phase to "action" when actually running an action (#76993)

When running a server action, we want `requestStore.phase` to be set to `'action'`, because it makes `cookies()` mutable. However we were doing this in a pretty coarse-grained manner -- it was set at the beginning of `handleAction`, but wasn't always properly cleaned up in case of early returns that don't render anything (e.g. `{ type: 'not-found' }`). This PR fixes that -- we only set the phase to `'action'` while the action is running, and immediately switch it to `'render'` when it settles.

Incidentally, early 'not-found' returns from `handleAction` are currently bugged (see #77012 for more context + fix), so we were also accidentally allowing mutatating cookies while rendering a response to an unrecognized action, which should be a 404, but is currently a 200 + a page render. but that's such a niche edge case that it's not really relevant to real usage.

Author: lubieowoce

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

@@ -510,13 +510,9 @@ export async function handleAction({
     // that in the work store to be up-to-date for subsequent rendering.
     workStore.isDraftMode = requestStore.draftMode.isEnabled
 
-    requestStore.phase = 'render'
-
     return generateFlight(...args)
   }
 
-  requestStore.phase = 'action'
-
   // When running actions the default is no-store, you can still `cache: 'force-cache'`
   workStore.fetchCache = 'default-no-store'
 
@@ -674,18 +670,22 @@ export async function handleAction({
               // Only warn if it's a server action, otherwise skip for other post requests
               warnBadServerActionRequest()
 
-              const actionReturnedState = await workUnitAsyncStorage.run(
-                requestStore,
-                action
-              )
+              let actionReturnedState: unknown
+              requestStore.phase = 'action'
+              try {
+                actionReturnedState = await workUnitAsyncStorage.run(
+                  requestStore,
+                  action
+                )
+              } finally {
+                requestStore.phase = 'render'
+              }
 
               formState = await decodeFormState(
                 actionReturnedState,
                 formData,
                 serverModuleMap
               )
-
-              requestStore.phase = 'render'
             }
 
             // Skip the fetch path
@@ -829,18 +829,22 @@ export async function handleAction({
               // Only warn if it's a server action, otherwise skip for other post requests
               warnBadServerActionRequest()
 
-              const actionReturnedState = await workUnitAsyncStorage.run(
-                requestStore,
-                action
-              )
+              let actionReturnedState: unknown
+              requestStore.phase = 'action'
+              try {
+                actionReturnedState = await workUnitAsyncStorage.run(
+                  requestStore,
+                  action
+                )
+              } finally {
+                requestStore.phase = 'render'
+              }
 
               formState = await decodeFormState(
                 actionReturnedState,
                 formData,
                 serverModuleMap
               )
-
-              requestStore.phase = 'render'
             }
 
             // Skip the fetch path
@@ -917,9 +921,15 @@ export async function handleAction({
           actionId!
         ]
 
-      const returnVal = await workUnitAsyncStorage.run(requestStore, () =>
-        actionHandler.apply(null, boundActionArguments)
-      )
+      let returnVal: unknown
+      requestStore.phase = 'action'
+      try {
+        returnVal = await workUnitAsyncStorage.run(requestStore, () =>
+          actionHandler.apply(null, boundActionArguments)
+        )
+      } finally {
+        requestStore.phase = 'render'
+      }
 
       // For form actions, we need to continue rendering the page.
       if (isFetchAction) {
@@ -1017,7 +1027,6 @@ export async function handleAction({
         // swallow error, it's gonna be handled on the client
       }
 
-      requestStore.phase = 'render'
       return {
         type: 'done',
         result: await generateFlight(req, ctx, requestStore, {