next-auth icon indicating copy to clipboard operation
next-auth copied to clipboard

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

Open rayzr522 opened this issue 1 year ago • 2 comments

☕️ 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!

rayzr522 avatar Sep 07 '22 23:09 rayzr522

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)

vercel[bot] avatar Sep 07 '22 23:09 vercel[bot]

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?

rayzr522 avatar Sep 19 '22 21:09 rayzr522