How to call a function returning a promise on a result of a promise?

Question

I have a basic mongoose authentication, with bcryptjs to hash passwords. Both bcrypt and mongoose return promises. In my routes.js I have the following script which gets stuck after finding the User in the db:

routes.post('/auth', (req, res)=> {
    User.findOne({'local.username': req.body.username})
        .then(
            user=> Promise.all([user, user.validate(req.body.password)])
        )
        .then(
            results => {
                console.log(results);
                res.json({token: jwt.sign({id: results[0]._id}, config.secret)});
            }
        )
        .catch(
            err=> console.log(err)
        );
});

As you can see I find the user, and then try to call its validate method (which gets called), but it won't resolve the promise nor throw an error. In my user.js which defines my UserSchema I have this code to compare passwords:

UserSchema.methods.validate = function (password) {
    return bcrypt.compare(password, this.local.password);
};

This is called, but the returned promise seems like it vanishes, it is not resolved, the results variable is never logged.

One more thing, if I edit user validation code to this:

UserSchema.methods.validate = function (password) {
    return bcrypt.compare(password, this.local.password).then(
        results => {
            console.log(results)
        }
    )
};

I get true logged to console, so it must be working but I don't want to resolve my promise here, I want to attach the .then(...) in my router, isn't it possible?

What am I doing wrong?

UPDATE:

If I put the compare method in the routes.js it works, but that is not what I want to do, I want to keep it in the user.js, but I thought this might point out the problem which I still cannot see. I guess I have to call then() immediately on the promise, but I don't see why.

 User.findOne({'local.username': req.body.username})
        .then(
            user=> Promise.all([user, bcrypt.compare(req.body.password,user.local.password)])
        )
        .then(
            results => {
                console.log(results);
                res.json({token: jwt.sign({id: results[0]._id}, config.secret)});
            }
        )
        .catch(
            err=> console.log(err)
        );

``Promise.all([user, user.validate(req.body.password)])`` should be ``Promise.all([Promise.resolve(user), user.validate(req.body.password)])`` — MatthieuLemoine, Jan 02 '17 at 11:14
@MatthieuLemoine Yes this seems reasonable, but tried it and it does not work :S — godzsa, Jan 02 '17 at 11:19
Not sure but I think your validate method should also resolve the promise. — Amit Jamwal, Jan 02 '17 at 11:22
Try `return bcrypt.compare(password, this.local.password)` only in `validate` function. — Mukesh Sharma, Jan 02 '17 at 11:33
@MatthieuLemoine No need for that, `Promise.resolve` is called implicitly by `Promise.all`. — Bergi, Jan 02 '17 at 11:57
What promise library are you using for mongoose? Is it Promises/A+ compatible? It might have a problem with assimilating the native promise. — Bergi, Jan 02 '17 at 12:03
@Bergi I am using global.Promise, I guess that is ES6 Promise in node v6.9.2 — godzsa, Jan 02 '17 at 12:10

score 1 · Answer 1 · answered Jan 02 '17 at 11:45

1

First of all why using Promise.all here? Especially i don't see the need for doing something like Promise.resolve(user). Without knowing how user.validate is working, i would write it like

routes.post('/auth', (req, res)=> {
    let userId

    User.findOne({'local.username': req.body.username})
    .then(user => {
      userId = user._id
      return user.validate(req.body.password)
    })
    .then(results => {
      console.log(results);
      res.json({token: jwt.sign({id: userId}, config.secret)});
    })
    .catch(err => console.log(err))
});

answered Jan 02 '17 at 11:45

Srle

10,366
8
34
63

I've tried this. It does not work for me, that's why I tried Promise.all() later, that does not work either – godzsa Jan 02 '17 at 11:49
Why `Promise.all`? Because that's a [much better pattern](http://stackoverflow.com/a/28250704/1048572) than [the ugly global `userId` variable](http://stackoverflow.com/a/28250700/1048572). – Bergi Jan 02 '17 at 12:00
Would partially agree with you. Would argue on what is more ugly, creating scoped variable to route handler or creating a promise when it is not needed. – Srle Jan 02 '17 at 12:07

score 0 · Accepted Answer · answered Jan 02 '17 at 13:01

I found out, that the problem is with mongoose. It wraps the module methods, and promises "get lost" somewhere. The solution to this is to use a sync compare method, or provide a callback.

Also I created an issue with this on github: https://github.com/Automattic/mongoose/issues/4856

score -1 · Answer 3 · answered Jan 02 '17 at 11:13

-1

You are not doing anything with the Promise.all you call in the then.

Instead of

user=> Promise.all([user, user.validate(req.body.password)])

You should then it:

user.validate(req.body.password)
.then(results => {
    // Do stuff with results here...
});

answered Jan 02 '17 at 11:13

Sheikz

317
1
8

1

I think this is the same, because Promise.all() returns a promise which is handled by .then() – godzsa Jan 02 '17 at 11:15
1

He is returning the `Promise.all(…)` from the callback, that's enough to do. – Bergi Jan 02 '17 at 11:58

How to call a function returning a promise on a result of a promise?

3 Answers3