fix: catch errors from jwt callback so we can handle them consistently

[foxfirecodes]

☕️ Reasoning

if errors are thrown during the jwt callback when signing in with credentials, they don't actually get caught anywhere so the default next.js 500 error page is returned, which signIn then tries to parse as JSON and ultimately throws an exception rather than just returning an error.

this felt like unexpected behavior IMO, so I just quickly made this tweak but I'm not actually sure if this aligns with the expectations around error handling. as such I haven't added any test cases or documentation around this yet, but if y'all agree that this is a logical way to handle errors here I'm more than happy to look into adding tests for this case! but didn't want to invest too much time if you ended up not agreeing with this change in the first place :P

🧢 Checklist

• [ ] Documentation
• [ ] Tests
• [ ] Ready to be merged

🎫 Affected issues

couldn't find any!

[vercel[bot]]

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

1 Ignored Deployment

Name	Status	Preview	Updated
next-auth	⬜️ Ignored (Inspect)		Sep 21, 2022 at 6:26PM (UTC)

[foxfirecodes]

Sounds good, but instead of returning the actual message to the client, let's create an error code and document it here: https://next-auth.js.org/errors

We could use logger.error(error) to show the actual error to the developer server-side

oh geez, I totally missed your reply here! tysm for reviewing!

my rationale for returning the actual error instead of just a vague code is that this is the current behavior for the signIn callback and allows passing more meaningful messages from the backend to the frontend. it's clunky UX to be limited to just "Please try again" rather than being able to specifically tell the user what went wrong

that being said, I could see an alternative argument that says "you should do all the checks that the user can sign in first in the signIn callback, and jwt should never throw errors!" however I think it's still helpful to be able to handle and pass meaningful user-readable messages to the client from the jwt callback

I just double checked that this is how the signIn callback works:

// server:
async signIn() {
  throw new Error('oh god its a pickle')
}

// client:
const res = await signIn('credentials', {
  redirect: false,
  username: 'rayzr522',
  password: 'yooooo',
})
console.debug(res)
// => {"error":"oh god its a pickle","status":200,"ok":true,"url":null}

thoughts?

[ThangHuuVu]

my rationale for returning the actual error instead of just a vague code is that this is the current behavior for the signIn callback and allows passing more meaningful messages from the backend to the frontend. it's clunky UX to be limited to just "Please try again" rather than being able to specifically tell the user what went wrong

hey @rayzr522 any chances you can revisit this PR and wrap it up? I like your approach because it aligns with the signIn callback behavior. On the other hand, I think the end user will not need to know about the error in JWT creation which can be considered "too much detail", and an error code is sufficient in this case. Let me know what you think!

[ThangHuuVu]

Close for inactivity. If this issue still persists, please reopen the PR against the core package.