Wierd async node.js

Question

I'm trying to find all musics my users are listening. I'v done it like this :

User.find().exec((err, users) => {
    users.forEach((user, index) => {
        Musics.count({ idReader: user._id }, (err, count) => {
            result.push({ mail: user.mail, count: count });
            if (index >= users.length - 1) {
                return res.json(result);
            }
        });
    });
});

And it work but not everytimes. Sometimes i'v only 30% of my users sometimes 100%. I think it's because of asynchronous. But i have no idea how to do differently. Thank you !

Users collection contains only 3 users. result is a var declared on top of User.find(). let result = new Array(); — Jeremy M.
– Jeremy M., Commented Mar 6, 2018 at 11:38
Yes, whenever your last count inside your forEach has completed it will return the result, this may or may not be the last count to be actually be completed. — Keith
– Keith, Commented Mar 6, 2018 at 11:39

Slim · Accepted Answer · 2018-03-06 11:46:27Z

1

Your intuition is correct. In your forEach loop you make count query for each user, and while you make the requests in order, they are not necessarily resolved in order. That's why checking for the index is not trust worthy. Here is a snippet that should make it:

User.find().exec((err, users) => {

    // get an array of promises for each count query
    var promises = users.map(user => 

        Musics.count({ idReader: user._id })
            // format ther result
            .then(count => ({
                mail: user.mail, 
                count: count
            }))
    );

    // after each request to the db is ready
    Promise.all(promises)
        .then(result => {
            res.json(result)
        })
        .catch( err => {
            console.log(err)
            res.status(500).send('Something went wrong')
        })
});

answered Mar 6, 2018 at 11:46

Slim

1,9641 gold badge12 silver badges20 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Jeremy M. Over a year ago

It work perfectly ! Thanks ! I will have to learn more about promises but your example explained me a lot. Ty again.

Muzammil Bilwani · Accepted Answer · 2018-03-06 11:40:37Z

0

You have to play with the promises to get the exact results for that.

musicListeners = async (req, res) => {
  try {
    const users = await Users.find({})
    try {
      const userPromises = [];
      for (let user in users)
        userPromises.push(Music.count({ idReader: user._id }))

      Promise.all(userPromises).then(res => {
        return res.send(res)

      }, err => {
        return res.status(500).send(err)
      })
    } catch (error) {
      return res.status(500).send('Something went wrong')
    }
  } catch (error) {
    return res.status(500).send('Something went wrong')

  }
}

Async/Await would be a good approach to write a good code but needs version of node to LTS one. It would not work with 6. of node.

answered Mar 6, 2018 at 11:40

Muzammil Bilwani

1464 silver badges14 bronze badges

2 Comments

Jeremy M. Over a year ago

My node version is the latest one.

Keith Over a year ago

Using async / await is certainly a good idea here. But you seem to have really over complicated this code.. especially since you are using async / await

Collectives™ on Stack Overflow

Wierd async node.js

2 Answers 2

1 Comment

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related