Node.js using multiple promises

Question

I currently try to use two kind of promises. One for the overall convensation, and one to get the userids to usernames.

The code works when its ran first time, and is looking something like this:

The output above is what i wanted.

But when i run the query again, i get this:

what is wrong with my code? what do i need to do to make the promises go correctly?

code:

function getconvensations(userid) {
    return new Promise(function(resolve, reject) {

        convensations.find({members: userid}).lean().then(function (convensations) {
            var promises = convensations.map(function (currentConvensation) {
                return new Promise(function (resolve, reject) {
                    var realnames = [];
                    var usernames = currentConvensation.members.map(function (CurrentUser) {
                        Core.userDetails(CurrentUser).then(function (user) {
                            realnames.push(user.username);
                            console.log("RESOLVING USERNAMES");
                           resolve();
                        });
                    });

                    Promise.all(usernames).then(function () {
                        console.log("GET LATEST MESSAGE");
                        latestMessage(currentConvensation._id).then(function (message) {
                            console.log("Messages: ");
                            console.log(message);
                            currentConvensation.spesificmessage = message;
                            currentConvensation.users = realnames;
                            currentConvensation.otherend = (currentConvensation.members[0] == userid ? realnames[0] : realnames[1]);

                            console.log("RESOLVE LATEST MESSAGE");


                            resolve();
                        });
                    });
                });
            });

            Promise.all(promises).then(function () {
                console.log("FINISHED CONVENSATIONS");
                console.log("-------------------------");
                console.log(convensations);
                console.log("-------------------------");
                return resolve(convensations);
            }).catch(console.error);
        });
    });
}

caller:

  } else if(action == 'getconvensations') {
        Messages_model.getconvensations(req.user._id).then(function (response) {
            res.json(response);
        });
    }

Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, Jun 13 '17 at 20:51
@Bergi im reading about it now. kevin: isnt i in this case resolving the promise two times for the 2nd and the main only one? — maria, Jun 13 '17 at 20:55
It looks like because of currentConvensation.members.map you will call resolve multipl times. I doubt this is what you want. — jas7457, Jun 13 '17 at 20:55
@jas7457 isnt the holder of members.map another promise im making? that is resolving twise before resolving the final? — maria, Jun 13 '17 at 20:56
.map doesn't make a promise, it makes an array. for each in the source array, you're creating a new promise but doing nothing with it. Then, when each of those resolve, you're resolving the parent promise. each time. that's incorrect. the promise.all that follows is just doing Promise.all([undefined,undefined,undefined,...]) — Kevin B, Jun 13 '17 at 20:58
so what do i need to do to fix this? @KevinB. thanks for clearing that up. — maria, Jun 13 '17 at 20:59
Btw, what's a convensation? I know conversations and conventions. — Bergi, Jun 13 '17 at 21:04

Bergi · Accepted Answer · 2017-06-13T21:11:20.480

You have a race condition here:

new Promise(function (resolve, reject) {
    …
    currentConvensation.members.map(function (CurrentUser) {
        Core.userDetails(CurrentUser).then(function (user) {
            resolve();
        });
    });
    …
    latestMessage(currentConvensation._id).then(function (message) {
        resolve();
    });
    …
});

There are arbitrarily many tasks executing concurrently, and the first promise that fulfills will call resolve().

The solution is to avoid the Promise constructor antipattern and never ever call new Promise or resolve manually! Instead, chain promise callbacks to each other using the then method, and return new promises from every function to allow the caller to wait for them.

function getconvensations(userid) {
    return convensations.find({members: userid}).lean().then(function (convensations) {
//  ^^^^^^
        var promises = convensations.map(function (currentConvensation) {
            var usernamepromises = currentConvensation.members.map(function (CurrentUser) {
                console.log("GETTING USERNAME");
                return Core.userDetails(CurrentUser).then(function (user) {
//              ^^^^^^
                    console.log("FULFILLED USERNAME");
                    return user.username;
//                  ^^^^^^
                });
            });
            return Promise.all(usernamepromises).then(function (realnames) {
//          ^^^^^^
                console.log("FULFILLED ALL USERNAMES");
                currentConvensation.users = realnames;
                currentConvensation.otherend = (currentConvensation.members[0] == userid ? realnames[0] : realnames[1]);

                console.log("GETTING LATEST MESSAGE");
                return latestMessage(currentConvensation._id);
//              ^^^^^^
            }).then(function (message) {
                console.log("FULFILLED LATEST MESSAGE");
                console.log("Message: ", message);
                currentConvensation.spesificmessage = message;
            });
        });
        return Promise.all(promises).then(function () {
//      ^^^^^^
            console.log("FINISHED ALL CONVENSATIONS");
            console.log("-------------------------");
            console.log(convensations);
            console.log("-------------------------");
            return convensations;
//          ^^^^^^
        });
    });
}

I tought that the members.map thats using usernames are resolving the other promise that im using. That after usernames resolves it resolves the final one. what do i need to do to fix it? — maria, Jun 13 '17 at 20:57
how can i make the called do .then function on this? Cannot read property 'then' of undefined I will take this answer soon. thanks . — maria, Jun 13 '17 at 21:07
Where exactly do you get that error about not being able to call `.then` on `undefined`? — Bergi, Jun 13 '17 at 21:25

Node.js using multiple promises

1 Answers1