Invoking redirecting action can cause runtime client error due to type mismatch

[juliusmarminge]

Link to the code that reproduces this issue

https://github.com/juliusmarminge/next-redirect-action-bug

To Reproduce

Sandbox: https://stackblitz.com/edit/stackblitz-starters-ajhapq?file=app%2FGoToGoogle.tsx

1. Open devtools console
2. Click Go to google
3. See error in console

CleanShot.2024-03-27.at.16.02.27.mp4

Current vs. Expected behavior

Due to the current architecture of redirect throwing in server actions, the type is (accurately) never, signaling that no code after the redirect() will execute. This is great and provides a great developer experience when making actions. However, it comes with a footgun that I think has been overlooked during the API design phase.

When a function returns never, we expect the function not to return, and thus code below wont get executed. While this is the case within the action, it is not the case at the place where the action is invoked. When invoking the action, the function does not throw but instead returns undefined before eventually the browser navigates to the redirected URL.

In the sandbox above, we conditionally return an envelope ({ ok: false, message: string }, and since the other branch of the action throws, the entire functions return type is just the envelope. On the client, we now get a type mismatch since invoking the action can return { ok: false, message: string } | undefined but the undefined part is not in the type. This will very likely be overlooked by users and thus cause runtime errors (as seen in the demo). I'd expect the return type to be a union with the undefined part so we can get TypeScript assistance in checking for this condition to avoid these errors.

---

With the current architecture, this is indeed not possible. My suggestion would be to

• introduce a new redirect function that instead of throwing, returns an error that can be handled by the Next.js runtime when the action gets executed. (perhaps under next/navigation/future which has done before with the <Image /> component for example)
• document this footgun with the current redirect function, informing developers of this behavior
• In a future major, replace the current behavior with the returning one.

Here is how actions would be written in this new way:

'use server';

import { redirect } from 'next/navigation/future';

export const redirectMe = async (input: string) => {
  if (input.length === 0) {
    return { ok: false as const, message: 'Something went wrong' };
  }
  return redirect('https://google.com');
  // 👆 **return**
};

typeof redirectMe; // (input: string) => { ok: false, message: string } | undefined

The redirect function would return some symbol-like thing that can be identified when the action is executed and trigger the redirect behavior similar to today. For reference, see trpc/trpc#5593

Some pseudo code:

// next/navigation/future
export const redirect = (url: string, redirectType: RedirectType) => {
  // do stuff

  // return something that the runtime can identify, but type it as undefined
  return new NextjsRedirectError() as unknown as undefined;
}

// next/internal/run-action
const runAction = (action) => {
  try {
    const result = await action();
    if (result instanceof NextjsRedirectError) {
      // do stuff, throw, whatever
    }
  } catch (e) {}
}

Of course this comes with a new mismatch that it returns an error but the type says undefined, but I think that's way less dangerous and also don't see why one would const variable = redirect(url).

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: Ubuntu 20.04.0 LTS Wed Mar 27 2024 16:04:01 GMT+0100 (Central European Standard Time)
  Available memory (MB): NaN
  Available CPU cores: 8
Binaries:
  Node: 18.18.0
  npm: 10.2.3
  Yarn: 1.22.19
  pnpm: 8.15.3
Relevant Packages:
  next: 14.2.0-canary.43
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.4.3
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

App Router

Which stage(s) are affected? (Select all that apply)

next dev (local), next start (local)

Additional context

Don't know why #63770 got closed, are sandboxes not valid reproductions??

_NEXT-3299

[chris-trait]

We are seeing this as well, noticing uncaught client side errors when a redirect happens from a server action, the client side call is returning undefined even though it's typed to return a value.

I expect the client side function to not return at all when a redirect happens, otherwise for the return to be typed with | undefined.