Confused about promises. Do I need to return here?

Question

So I've recently started to learn Promises (Bluebird) and now I'm trying to use them as much as possible, but I'm a bit confused if I need to return promises in this case.

Here I have a Passport LocalStrategy that I made:

passport.use(new LocalStrategy(function(username, password, done) {
    users.get(username) // A
        .then(function(user) {
            if (!user) {
                return done(null, false, { message: 'Incorrect username.' });
            }
            bcrypt.compare(password, user.password).then(function(result) { // B
                if (result) {
                    return done(null, user);
                }
                return done(null, false, { message: 'Incorrect password.' });
            });
        })
        .catch(function(err) {
            return done(err);
        });
}));

users.get(username) on line A uses the pg-promise library to return a promise that will resolve into a user if one is found in the database and to false if the user was not found.

bcrypt.compare on line B uses bcrypt to check if the password and hash match. It returns a promise that will resolve to true or false.

The code works perfectly, I'm just confused if line A and B should return like so

return users.get(username) // A

return bcrypt.compare(password, user.password).then(function(result) { // B

The code works with and without returning the promises.

Is Passport/Node just waiting until it sees return done? Does this mean that this function is synchronous even though everything inside it is async? Normally you would return a promise and then use .then() on it but since LocalStrategy is not using .then() or .catch() I don't have to return anything? Any input is greatly appreciated. Thanks.

At least related: http://stackoverflow.com/questions/23920589/how-to-pass-a-third-argument-to-a-callback-using-bluebird-js-nodeify — Tomalak, Jan 02 '17 at 12:42
I found this to be a really good article explaining confusions with promises [We have a problem with promises](https://pouchdb.com/2015/05/18/we-have-a-problem-with-promises.html) — IVN, Jan 02 '17 at 13:44
You might want to have a look at [my rules of thumb](http://stackoverflow.com/a/25756564/1048572) for promises — Bergi, Jan 02 '17 at 14:20

score 0 · Accepted Answer · answered Jan 02 '17 at 12:42

Passport does not support Promise that is why you must call done in a callback. You could return users.get(username)but the return value (promise) is never used. Do not forget that you can chain promises like the following :

users.get(username)
    .then(function(user) {
        if (!user) {
            return done(null, false, { message: 'Incorrect username.' });
        }
        return bcrypt.compare(password, user.password);
   })
   .then(function(result) { // B
        if (result) {
            return done(null, user);
        }
        return done(null, false, { message: 'Incorrect password.' });
    })
    .catch(function(err) {
        return done(err);
    });

Ok thanks. I figured that it probably does not matter in this case since Passport does not care. I was just wondering if there was any harm in returning or not but I guess not. The reason I have not chained like in your example is because bcrypt.compare does not return the user, it just returns true or false so I can't return the user after I .then() users.get(username). — Nikokin, Jan 02 '17 at 13:52

Confused about promises. Do I need to return here?

1 Answers1