Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Next Redirect - Server action redirects do not execute when inside try / catch blocks #55586

Closed
1 task done
Pbmangan opened this issue Sep 19, 2023 · 20 comments
Closed
1 task done
Labels
bug Issue was opened via the bug report template. locked

Comments

@Pbmangan
Copy link

Link to the code that reproduces this issue

https://github.com/Pbmangan/new-app/blob/main/app/action/page.tsx

To Reproduce

  1. Create simple server action with redirect to relative or absolute route (ensure it executes)
  2. Add a try / catch block around the redirect code
  3. See failure message such as below
    4.NEXT_REDIRECT at getRedirectError (webpack-internal:///(rsc)/./node_modules/next/dist/client/components/redirect.js:40:19) at redirect (webpack-internal:///(rsc)/./node_modules/next/dist/client/components/redirect.js:50:11) at $$ACTION_0 (webpack-internal:///(rsc)/./components/server-actions/action.ts:97:70)

Current vs. Expected behavior

Currently redirect intuitively would be used inside try catch blocks - such as when creating a customer checkout session for stripe. The Docs do not highlight the issue and it was not intuitive to solve.

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.6.0: Wed Jul  5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000
    Binaries:
      Node: 20.5.0
      npm: 2.15.12
      Yarn: 1.22.19
      pnpm: N/A
    Relevant Packages:
      next: 13.4.19
      eslint-config-next: 13.4.19
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 4.9.4
    Next.js Config:
      output: N/A

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

App Router

Additional context

No response

@Pbmangan Pbmangan added the bug Issue was opened via the bug report template. label Sep 19, 2023
@wwwdepot
Copy link

Invoking the redirect() function throws a NEXT_REDIRECT error and terminates rendering of the route segment in which it was thrown.

Follow the example at https://nextjs.org/docs/app/api-reference/functions/redirect, perform async task and return a condition, then redirect.

@Pbmangan
Copy link
Author

Shouldn't the redirect still be executed in this case regardless of the catch block?

@anthonyLock
Copy link

anthonyLock commented Sep 27, 2023

@Pbmangan I had the same issue as yourself. From looking at the code and what I can workout the redirect function, essentially throws a special type of error, and then when the page function throws and error, next checks to see if is of the redirect type and processes it. Luckily there is a function isRedirectError you can use this in your catch block.

function redirect(url, type) {
    if (type === void 0) type = "replace";
    throw getRedirectError(url, type);
}
import { isRedirectError } from "next/dist/client/components/redirect";
import { redirect } from "next/navigation";

export default async function Page() {
  try {
//API Calls to get data 
  redirect(`/new-route`);
 } catch (err: any) {
   if (isRedirectError(err)) {
      throw err;
    } else {
      console.log(err)
      console.log("ERROR FROM OTHER OPERATIONS")
      throw err
    }
  }
}

Hope this helps

@walfly
Copy link
Contributor

walfly commented Oct 2, 2023

This feels like really unexpected behavior. A server action might frequently take this form.

async function createAndNavigateToPost(post) {
  try {
     const newPost = await insertPostIntoDb(post)
     redirect(`/posts/${newPost.id}`)
  } catch (e) {
    // make the error visible to the form
    return {
      message: e.message
    }
  }
} 

Having to always check if the error is a redirect error is really unexpected behavior and pollutes the application code with framework idiosyncrasies.

@Pbmangan
Copy link
Author

Pbmangan commented Oct 4, 2023

Thank you yes I agree. Good to know about isRedirectError but agree on seemingly some extra code and sub-optimal behavior. It is clear in the documentation in hindsight but I overlooked the error-throwing meaning initially.

@dinosmajovic
Copy link

Adding an await fixed the problem for me on next 14.

async function createAndNavigateToPost(post) {
  try {
     const newPost = await insertPostIntoDb(post);
     await redirect(`/posts/${newPost.id}`); // add await
  } catch (e) {
    // make the error visible to the form
    return {
      message: e.message
    }
  }
} 

@oak93

This comment has been minimized.

@mGasiorek998
Copy link

Next v14.0.3
Same problem, with our without await.
Also redirect from next/navigation does not show up as async function so I get await has no effect on the type of this expression.

@sabovyan
Copy link

sabovyan commented Dec 6, 2023

I found this code block in my node_modules which is a bit strange
image

@renanleonel
Copy link

renanleonel commented Dec 25, 2023

getting this error too with action:

export const signup = async (prevState: any, formData: FormData) => {
	try {
		const email = formData.get('email');
		const password = formData.get('password');
		const confirmPassword = formData.get('confirmPassword');

		const validatedFields = signUpForm.safeParse({
			email: email,
			password: password,
			confirmPassword: confirmPassword,
		});

		if (!validatedFields.success) {
			return {
				message: 'validation error',
				errors: validatedFields.error.flatten().fieldErrors,
			};
		}

		redirect('/sign-up/info');
	} catch (error) {
		console.log(error);

		return {
			message: 'unknown error',
			errors: {
				...defaultSignUpValues,
				unknown: 'unknown error.',
			},
		};
	}
};

console.log(error); logs me

Error: NEXT_REDIRECT at getRedirectError

any news on this issue?

edit: currently doing this workaround:

export const signup = async (prevState: any, formData: FormData) => {
	let success = false;
	try {
		const email = formData.get('email');
		const password = formData.get('password');
		const confirmPassword = formData.get('confirmPassword');

		const validatedFields = signUpForm.safeParse({
			email: email,
			password: password,
			confirmPassword: confirmPassword,
		});

		if (!validatedFields.success) {
			return {
				message: 'validation error',
				errors: validatedFields.error.flatten().fieldErrors,
			};
		}

		success = true;
		redirect('/sign-up/info');
	} catch (error) {
		console.log(error);

		if (success) redirect('/sign-up/info');

		return {
			message: 'unknown error',
			errors: {
				...defaultSignUpValues,
				unknown: 'unknown error.',
			},
		};
	}
};

but this is not the ideal approach

@leerob
Copy link
Member

leerob commented Dec 25, 2023

This is intended – redirect() throws NEXT_REDIRECT and returns the TypeScript never type. It should be called outside of your try/catch block, or handled explicitly. We've updated the docs to change the code blocks for Server Actions to place the redirect() after the try/catch block.

https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations#redirecting

@leerob leerob closed this as completed Dec 25, 2023
@renanleonel
Copy link

thank you for the quick answer!

@chungweileong94
Copy link
Contributor

This is intended – redirect() throws NEXT_REDIRECT and returns the TypeScript never type. It should be called outside of your try/catch block, or handled explicitly. We've updated the docs to change the code blocks for Server Actions to place the redirect() after the try/catch block.

https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations#redirecting

Hmm... it's quite confusing. According to the NextJS authentication learning doc, the next-auth signIn function is being called in try/catch block, and the function contains redirect from next/navigation.

@chungweileong94
Copy link
Contributor

chungweileong94 commented Dec 26, 2023

This is intended – redirect() throws NEXT_REDIRECT and returns the TypeScript never type. It should be called outside of your try/catch block, or handled explicitly. We've updated the docs to change the code blocks for Server Actions to place the redirect() after the try/catch block.
https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations#redirecting

Hmm... it's quite confusing. According to the NextJS authentication learning doc, the next-auth signIn function is being called in try/catch block, and the function contains redirect from next/navigation.

Aha, I think I understand now. So technically, we can still call redirect within a try/catch block. However, we need to ensure that we throw the original error. Here's an example:

try {
  redirect('/~');
} catch (error) {
  throw error; // <-- We need to throw this error to ensure that the redirect works.
}

Instead of mentioning that redirect cannot be used in a try/catch block in the documentation, it would be helpful to explain why and provide the workaround.

(Edit: Didn't realize someone already mentioned the exact same thing above, my bad)

@monsterooo
Copy link

This is intended – redirect() throws NEXT_REDIRECT and returns the TypeScript never type. It should be called outside of your try/catch block, or handled explicitly. We've updated the docs to change the code blocks for Server Actions to place the redirect() after the try/catch block.
https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations#redirecting

Hmm... it's quite confusing. According to the NextJS authentication learning doc, the next-auth signIn function is being called in try/catch block, and the function contains redirect from next/navigation.

Aha, I think I understand now. So technically, we can still call redirect within a try/catch block. However, we need to ensure that we throw the original error. Here's an example:

try {
  redirect('/~');
} catch (error) {
  throw error; // <-- We need to throw this error to ensure that the redirect works.
}

Instead of mentioning that redirect cannot be used in a try/catch block in the documentation, it would be helpful to explain why and provide the workaround.

(Edit: Didn't realize someone already mentioned the exact same thing above, my bad)

I used next-auth v5, and I actually had to use try catch to catch the 'signIn' error

// dont work NEXT_REDIRECT
try {
    await signIn("credentials", {
      email,
      password,
      redirectTo: "/",
    });
  } catch (error) {
    console.log(error);
  }

Is there a better solution? Thank you

@chungweileong94
Copy link
Contributor

Is there a better solution?

You can throw the error if it's a redirect error, else handle the other errors

import { isRedirectError } from "next/dist/client/components/redirect";

try {
  await signIn("credentials", {
    email,
    password,
    redirectTo: "/",
  });
} catch (error) {
  if (isRedirectError(err)) {
    throw err;
  }
  
  // Handle other errors
  console.log(error);
}

@monsterooo
Copy link

Is there a better solution?

You can throw the error if it's a redirect error, else handle the other errors

import { isRedirectError } from "next/dist/client/components/redirect";

try {
  await signIn("credentials", {
    email,
    password,
    redirectTo: "/",
  });
} catch (error) {
  if (isRedirectError(err)) {
    throw err;
  }
  
  // Handle other errors
  console.log(error);
}

Thank you. It works fine, but emm, I've seen the repository code work very well (I've tested it locally) and it feels mysterious
https://github.com/AntonioErdeljac/next-auth-v5-advanced-guide/blob/master/actions/login.ts#L106

@monsterooo
Copy link

Is there a better solution?

You can throw the error if it's a redirect error, else handle the other errors

import { isRedirectError } from "next/dist/client/components/redirect";

try {
  await signIn("credentials", {
    email,
    password,
    redirectTo: "/",
  });
} catch (error) {
  if (isRedirectError(err)) {
    throw err;
  }
  
  // Handle other errors
  console.log(error);
}

Thank you. It works fine, but emm, I've seen the repository code work very well (I've tested it locally) and it feels mysterious https://github.com/AntonioErdeljac/next-auth-v5-advanced-guide/blob/master/actions/login.ts#L106

As long as' throw error 'everything is fine

try {
    await signIn("credentials", {
      email,
      password,
      redirectTo: "/",
    });
  } catch (error) {
    throw error;
  }

But the same code shows the NEXT_REDIRECT, Maybe I got a little dizzy. Thank you

@renanleonel
Copy link

currently doing like this, dont know if it's the best approach but it works for my needs

export async function fn(prevState: any, formData: FormData) {
	let success = false;

	try {
		const email = formData.get('email');

		const validatedFields = recoverSchema.safeParse({
			email: email,
		});

		if (!validatedFields.success) {
			return {
				message: 'validation error',
				errors: validatedFields.error.flatten().fieldErrors,
			};
		}

		success = true;
	} catch (error) {
		return {
			message: 'unknown error',
			errors: {
				...defaultRecoverValues,
				unknown: 'unknown error.',
			},
		};
	}

	if (success) redirect('/');
}

Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. locked
Projects
None yet
Development

No branches or pull requests