fix: respond with 404 for unrecognized action ids (#77012)
> [!IMPORTANT]
> view this diff with "hide whitespace changes", it's unreadable
otherwise.
This is an alternative approach to #80613. We touch a lot of same areas
but do different changes there, and i also have a bunch more changes
stacked on top, so the plan is to just revert that and do this instead.
---
server action calls with an unrecognized action id (i.e. one not found
in `serverModuleMap`) have been returning a 200 instead of a 404 for
quite some time. and also rendering the page they were called on.
### The problem
this happened due to an unfortunate mistake. `handleAction` has a
section like this:
```ts
await actionAsyncStorage.run(
{ isAction: true },
// "the callback"
async () => {
...
}
);
return {
type: 'done',
result: actionResult,
formState,
}
```
inside that callback, we have many places that validate the action id
and try to bail out if it's not recognized:
```ts
try {
actionModId = getActionModIdOrError(actionId);
} catch (err) {
console.error(err);
return { type: 'not-found '}
}
```
this was clearly intended to be an early return from `handleAction`. but
it didn't work, because outside the callback we're doing
```ts
await actionAsyncStorage.run(...)
```
i.e. **discarding the callback's return value**. So instead of returning
`'not-found'` as intended, we end up falling through to the `'done'`
return right after, and returning `{ type: 'done', result: undefined,
formState: undefined }`.
then, our caller,`renderToHTMLToFlightImpl` looks at that what we
returned, and sees a 'done'
https://github.com/vercel/next.js/blob/768183c5ddbaa94c1f6004816a1fae282fa0ab04/packages/next/src/server/app-render/app-render.tsx#L1522-L1547
but because if there's no `result` or `formState`, it'll fall through
the conditionals and do the same thing it'd do if we returned
`null/undefined` (i.e. if `!actionRequestResult`). so we end up on the
rendering path, render a page, and finally respond with a 200.
### The fix, part 1
This PR fixes this. we're now directly returning the value from the
callback (`return await actionAsyncStorage.run(...)`) . i got rid of all
the weird fallthroughs and made each of the branches return something
explicitly. I've also changed `handleAction`'s return type to have a
`... | null` instead of `... | undefined` (which is interpreted as "this
was not an action request") so that it's not possible to fall off the
end of the function without an explicit return.
### The fix, part 2
To make this more complicated, returning `'not-found'` for fetch actions
was actually also incorrect! that value makes `renderToHTMLToFlightImpl`
render an HTML 404 page, which is pretty useless as a (fetch) action
response -- it's got the wrong content type! (#80613 relied on this to
make the action fail on the client-side, which isn't ideal, because the
error message doesn't indicate what the problem is).
To solve this, `handleAction` now returns a custom 404 response tagged
with a `x-nextjs-action-not-found` header (and uses `{ type: 'done' }`
instead of `{ type: 'not-found' }` to bypass the aformentioned 404
handling). Then, on the client, `fetchServerAction` checks for this
header, and throw a corresponding error. Other 404s will still throw a
`An unexpected response was received from the server.` error instead,
because they had to come from something else, like a bad proxy rewrite.
