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!

score 15 · Accepted Answer · answered May 14 '12 at 08:36

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
}

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? — Jeroen, May 14 '12 at 08:50
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: https://github.com/coolaj86/futures/tree/v2.0/forEachAsync , that also guarantees the order of the function callbacks (the code I provided doesn't) — BFil, May 14 '12 at 08:55
The solution works but most devs would use async.JS or a promise library (https://github.com/kriskowal/q). The other libraries can give you a lot more "leverage" in your code. — pehaada, Jul 23 '14 at 19:14

score 6 · Answer 2 · answered Mar 31 '14 at 02:38

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
      });
    });
  });
}

This is the best solution to adopt moving forward, no variables to worry about or order the callbacks fire. thanks! — sidonaldson, May 13 '15 at 10:40

score 2 · Answer 3 · edited May 23 '17 at 12:18

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.

as I have been testing for a solution, I already tried putting the code ad the end of the loop, does not make a difference — Jeroen, May 14 '12 at 08:05
Why don't you put the `res.render(...)` call **after** the end of the foreach but inside the first callback. Than the redirect will be executed after the foreach is finished. — jsbeckr, May 14 '12 at 08:50
@graydsl I don't know where you mean exactly but I think I tried most places, it keeps doing the render before the looping :o — Jeroen, May 14 '12 at 09:11
@Jeroen Forget what I said. I was wrong, because of the `OnlineUser.findOne(...)` call. Hm.. if the answer from ShadowCload works for you, it's fine. :) You could read about [fibers](https://github.com/laverdet/node-fibers), which will give you the ability to use async functions in a sync way. But they aren't that easy to use. ;) — jsbeckr, May 14 '12 at 10:16

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

3 Answers3