satray satray - 4 months ago 32
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

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);
                  });
                })
              });
}