NodeJs, javascript: .forEach seems to be asynchronous? need synchronization

Question

I am currently working on a project with 3 friends using nodeJs, expressJs, MongoDB, html5,... Since we're fairly new to these technologies we bumped into some problems. A big problem that I can't find a solution for is the asynchronous execution of certain code.

I want a for each loop to finish, so that I have an updated online friends list, and than execute the res.render (in which I pass the online friends list), because currently it does the res.render before it finishes the loop. Code:

function onlineFriends(req, res) {
var onlinefriends = new Array();
onlinefriends.push("mark");
FriendList.findOne({
    owner: req.session.username
}, function (err, friendlist) {
    friendlist.friends.forEach(function (friend) { // here forEach starts
        OnlineUser.findOne({
            userName: friend
        }, function (err, onlineFriend) {
            if (onlineFriend != null) {
                onlinefriends.push(onlineFriend.userName);
                console.log("a loop");
            }
        });

    });  
        console.log("online friends: " + onlinefriends);
        console.log("redirecting");
        res.render('index', { // this is still inside the forEach function
            friendlist: friendlist.friends,
            onlinefriendlist: onlinefriends,
            username: req.session.username
        });// and here it ends
});

}

output will be as follows:

online friends: mark
redirecting
a loop
a loop
a loop
a loop
a loop
a loop
a loop

As discussed here ( JavaScript, Node.js: is Array.forEach asynchronous? ) , the answer is that the for-each is blocking, but in my example it seems to be non-blocking because it executes the res.render before it has finished looping? How can I make sure that the for each is finished so I have an up to date onlinefriends list (and friendlist) which I can than pass to the res.render instead of the res.render happening way before the for -each loop finishes (which gives me an incorrect list of online users) ?

Thanks very much!

Solution 1

The following console log:

console.log("a loop");

is inside a callback

I believe that the callback of the function OnlineUser.findOne() is called asynchronously, that is why the code will log "a loop" after the redirect log

You should put the redirection after all the loop callbacks have been executed

Something like:

var count = 0;
friendlist.friends.forEach(function (friend) { // here forEach starts
    OnlineUser.findOne({
        userName: friend
    }, function (err, onlineFriend) {
        count++;
        if (onlineFriend != null) {
            onlinefriends.push(onlineFriend.userName);
            console.log("a loop");
        }
        if(count == friendlist.friends.length) { // check if all callbacks have been called
            redirect();
        }
    });
}); 

function redirect() {
    console.log("online friends: " + onlinefriends);
    console.log("redirecting");
    res.render('index', { // this is still inside the forEach function
        friendlist: friendlist.friends,
        onlinefriendlist: onlinefriends,
            username: req.session.username
    });// and here it ends
}

Solution 2

I was able to solve something similar by adding the async package to my project and changing forEach() to async.each(). The advantage is that this provides a standard way to do synchronization for other parts of the application.

Something like this for your project:

function onlineFriends(req, res) {
  var onlinefriends = new Array();
  onlinefriends.push("mark");

  FriendList.findOne({owner: req.session.username}, function (err, friendlist) {
    async.each(friendlist.friends, function(friend, callback) {
      OnlineUser.findOne({userName: friend}, function (err, onlineFriend) {
        if (onlineFriend != null) {
          onlinefriends.push(onlineFriend.userName);
          console.log("a loop");
        }
        callback();
      });
    }, function(err) {
      console.log("online friends: " + onlinefriends);
      console.log("redirecting");
      res.render('index', { // this is still inside the forEach function
          friendlist: friendlist.friends,
          onlinefriendlist: onlinefriends,
          username: req.session.username
      });
    });
  });
}

Solution 3

Running your code through jsbeautifier indents it properly and shows you why that happens:

function onlineFriends(req, res) {
    var onlinefriends = new Array();
    onlinefriends.push("mark");
    FriendList.findOne({
        owner: req.session.username
    }, function (err, friendlist) {
        friendlist.friends.forEach(function (friend) { // here forEach starts
            console.log("vriend: " + friend);
            OnlineUser.findOne({
                userName: friend
            }, function (err, onlineFriend) {
                if (onlineFriend != null) {
                    onlinefriends.push(onlineFriend.userName);
                    console.log("online friends: " + onlinefriends);
                }
            });
            console.log("nu door verwijzen");
            res.render('index', { // this is still inside the forEach function
                friendlist: friendlist.friends,
                onlinefriendlist: onlinefriends,
                username: req.session.username
            });
        });  // and here it ends
    });

So... always indent your code properly and you won't have issues like this. Some editors such as Vim can indent your whole file with a single shortcut (gg=G in vim).

However, OnlineUser.findOne() is most likely asynchronous. so even if you move the call to the correct location it won't work. See ShadowCloud's answer on how to solve this.

Comments

[Jeroen]

thanks!! this works, but is this kind of programming in javascript considered as a "bad practice" ? or is it fully legit to work this way?

[BFil]

It's not a bad practice, it's how javascript works, you just need to get used to callbacks.. Anyway, it obviously isn't the cleanest way, you could buil your own function, wrap the functionality, or use something like: github.com/coolaj86/futures/tree/v2.0/forEachAsync , that also guarantees the order of the function callbacks (the code I provided doesn't)

[pehaada]

The solution works but most devs would use async.JS or a promise library (github.com/kriskowal/q). The other libraries can give you a lot more "leverage" in your code.

[sidonaldson]

This is the best solution to adopt moving forward, no variables to worry about or order the callbacks fire. thanks!