satray satray - 1 year ago 112
Node.js Question

Catching errors with ES6 promises and BookshelfJS

I'm adding a very basic login method to a Bookshelf User model in an ExpressJS app, but I can't catch the errors from the rejected promise that the login function in the User model returns. I'm looking at Bookshelf's example login in the docs at http://bookshelfjs.org/#Model-static-extend but that example uses Bluebird, whereas I'm trying to do the same with the built-in ES6 promises.

My login method in the User model:

userModel.js

function login(email, password) {
return new Promise((resolve, reject) => {
User.where('email', email)
.fetch({ require: true })
.then(user => {
bcrypt.compare(password, user.get('password'), (err, matched) => {
if (!matched) return reject(new Error('Password didn\'t match!'));
resolve(user);
});
});
});


The controller action that implements login and calls
User.login
from the Bookshelf
User
model:

usersAuthController.js

function logUserIn(req, res) {
new User().login(req.body.email, req.body.password)
.then(user => res.json({ message: 'Login succeeded!' }))
.catch(User.NotFoundError, () => res.status(404).json({ error: 'User not found!' }) // catch #1
.catch(err => res.status(401).json({ err: err.message })); // catch #2
}


My intention is that
login()
can return a rejected promise when Bookshelf's
User.fetch
method can't find the user with the given email. In that case, the
.catch(User.NotFoundError ...)
(catch #1) line should catch it and return a 404. I also intend
login()
to return a rejected Promise when
bcrypt
determines that the password passed to
login()
doesn't match the user's password, in which case the "catch-all" (catch #2) below the
User.NotFoundError
catch statement should return a 401.

If I put in the incorrect password, the
logUserIn()
controller action in the above code goes to catch #2 with the error message
{ error: "Cannot set property 'message' of undefined" }
instead of the message
'Password didn't match!'
message that I rejected in
login()
. If I put in a nonexistent email, the response is never sent and the error
Unhandled rejection CustomError: EmptyResponse
is thrown in the console. Only valid input works.

An attempt at fix: catching
User.NotFoundError
directly in the model instead.

I moved catch #1 to the User model so that the login method now looks like:

userModel.js

function login(email, password) {
return new Promise((resolve, reject) => {
User.where('email', email)
.fetch({ require: true })
.then(user => {
bcrypt.compare(password, user.get('password'), (err, matched) => {
if (!matched) return reject(new Error('Password didn\'t match!'));
resolve(user);
});
})
.catch(User.NotFoundError, () => reject({ error: 'User not found!' }));
});


This way, I can catch both errors (incorrect password and nonexistent email) correctly, but this way I can't specify the status code in the controller. If the user with the given email couldn't be found, then it should return a 404, but if the password was incorrect, then it should return a 401, but both errors go to the catch-all (catch #2) in the controller action (which always returns a 401).

To solve this problem, in the
User
model I could do
.catch(User.NotFoundError, () => reject({ name: 'NotFoundError', message: 'User not found!' }))
and in the controller action I can check for what kind of error I'm getting with
const statusCode = err.name === 'NotFoundError' ? 404 : 401
but that seems really messy and misses the point of having these
.catch
statements.

Is there a way to catch the
User.NotFoundError
from the model's login method and any other errors all in
logInUser
? Why doesn't the setup I had at first work, the one with both
catch
statements in usersAuthController.js, and what did the
Cannot set property 'message' of undefined'
and
CustomError: EmptyResponse
errors mean (does it have something to do with mixing up Bookshelf's Bluebird promises versus the built-in ES6 promises)? What's the best way to handle this?

Answer Source

In your initial implementation, you're not propagating any rejections that may be caused by User.fetch(). Also, because User.fetch() already returns a promise, wrapping it with a new promise is a bit of an anti-pattern (although you still need to wrap bcrypt.compare() with a promise, since that only works with callbacks).

Try this:

function login(email, password) {
  return User .where('email', email)
              .fetch({ require: true })
              .then(user => {
                return new Promise((resolve, reject) => {
                  bcrypt.compare(password, user.get('password'), (err, matched) => {
                    if (err)      return reject(err);
                    if (!matched) return reject(new Error('Password didn\'t match!'));
                    resolve(user);
                  });
                })
              });
}