Skip to content

Conversation

@ndom91
Copy link
Member

@ndom91 ndom91 commented May 31, 2024

☕️ Reasoning

  • Change CredentialsSignin error to use Error as base class
    • Avoids adding our "Read more at authjs.dev .." suffix to error.message when users extend this class for their own signin errors

Replaces #10231

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

📌 Resources

@ndom91 ndom91 requested a review from ThangHuuVu as a code owner May 31, 2024 10:49
@vercel
Copy link

vercel bot commented May 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 1, 2024 10:22am
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Jun 1, 2024 10:22am
proxy ⬜️ Ignored (Inspect) Visit Preview Jun 1, 2024 10:22am

@ndom91 ndom91 merged commit d089923 into main Jun 1, 2024
@ndom91 ndom91 deleted the ndom91/update-custom-signin-error-class branch June 1, 2024 10:15
@superafroman
Copy link

@ndom91 this breaks the logic that redirects these errors to the signin page (see isAuthError in src/index.ts). Now a CredentialsSignIn error shows the error page with error=Configuration

@dml0031
Copy link

dml0031 commented Jun 3, 2024

@ndom91 this breaks the logic that redirects these errors to the signin page (see isAuthError in src/index.ts). Now a CredentialsSignIn error shows the error page with error=Configuration

Also noticed this, left some more details on this in this comment on the other PR that added code to the client side.

@Kupinaaa
Copy link

Breaks the types on CredentialsSignin errors. The typing of CredentialsSignin is changed from extending SignInError to extending Error.

Screenshot 2024-06-14 at 11 12 13 AM

Copy link

@Kupinaaa Kupinaaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaks types on CredentialsSignin class #11155

* 2. If you throw this error in a framework that handles form actions server-side, this error is thrown, instead of redirecting the user, so you'll need to handle.
*/
export class CredentialsSignin extends SignInError {
export class CredentialsSignin extends Error {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaks types on CredentialsSignin #11155

@l0gicgate
Copy link

This PR caused a regression and now we are unable to throw custom errors during the sign in mechanism.

Is there a plan to address this? I can raise a PR to revert if necessary.

@ndom91

@ndom91
Copy link
Member Author

ndom91 commented Jun 24, 2024

This PR caused a regression and now we are unable to throw custom errors during the sign in mechanism.

Is there a plan to address this? I can raise a PR to revert if necessary.

@ndom91

Thanks for the reminder.

Check out my response here: #11155 (comment)

I'll take a look at fixing this shortly

@l0gicgate
Copy link

Thank you for the prompt response @ndom91!

ThangHuuVu added a commit that referenced this pull request Jul 27, 2024
Added a test to avoid regression
ThangHuuVu added a commit that referenced this pull request Jul 27, 2024
* fix(core): revert #11050

Added a test to avoid regression

* Update callback.test.ts

* Update index.ts
k3k8 pushed a commit to k3k8/next-auth that referenced this pull request Aug 1, 2024
* fix(core): revert nextauthjs#11050

Added a test to avoid regression

* Update callback.test.ts

* Update index.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Refers to `@auth/core`

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants