ace196 ace196 - 1 month ago 6
Javascript Question

Async function and callbacks inside a forEach loop

Inside this function:

function Example(array, callback){
var toReturn = [];

// variable 'array' has 2 elements in this case
array.forEach(function(el){
MongooseModel.AsyncFunction(el, function(err, doc){
if(err || !doc){ return callback('Error'); }
toReturn.push(doc)
});
});

return callback(null, toReturn)
}


A few things to note:


  1. toReturn.push(doc)
    doesn't work and on the last callback an empty
    array is returned. Why is that?

  2. If there's an error, or !docs is true,
    return callback('Error')
    is never called.
    return callback(null, toReturn)
    is always called.



Also, if I put a callback above the async function, like this:

function Example(array, callback){
var toReturn = [];

// variable 'array' has 2 elements in this case
array.forEach(function(el){
// another callback here
return callback('Error')
MongooseModel.AsyncFunction(el, function(err, doc){
if(err || !doc){ return callback('Error'); }
toReturn.push(doc)
});
});

return callback(null, toReturn)
}


the script crashes because multiple callbacks have been called. Why is forEach doing that and how to avoid it?

Kos Kos
Answer

Your questions

toReturn.push(doc) doesn't work and on the last callback an empty array is returned. Why is that?

You're calling callback before MongooseModel.AsyncFunction gets any chance to execute your function(err,doc).

If there's an error, or !docs is true, return callback('Error') is never called. return callback(null, toReturn) is always called.

The successful return is called immediately after scheduling your async functions. It's still possible that the callback('Error') will be called sometime later by MongooseModel.AsyncFunction.

the script crashes because multiple callbacks have been called

That's exactly the behaviour you asked for! Your second code literally says: "For each element, call the callback once".


Some pointers

All in all, I think the single thing you have wrong is that calling the callback somewhat resembles returning from the function.

That's not the case! Your function Example is supposed to schedule some async things to happen, then return immediately (returning nothing) but promising that either callback(error) or callback(null, success) will be called sometime later in the future.

Therefore, it doesn't make sense to ever say return callback() - just callback() will do. By the time that you have some data to call callback with, the function Example will already have finished executing. The function that eventually calls callback will the an anonymous function passed to MongooseModel.AsyncFunction, not Example itself.


Now what?

You could try an approach like this:

function Example(array, callback){
    var toReturn = [];
    var previousError = false;

    array.forEach(function(el){
        MongooseModel.AsyncFunction(el, function(err, doc){
            if (previousError) {
                return;
            }
            if(err || !doc) {
                previousError = true;
                callback('Error');
            }
            toReturn.push(doc)
            if (toReturn.length === array.length) {
                // that was the last push - we have completed
                callback(null, toReturn);
            }
        });
    });
}

Stuff that happens here:

  • Every time your AsyncFunction completes, you aggregate the result. If that was the last one, you can finally call callback and pass the whole data. (Otherwise, we're waiting for a few more functions, so no callback)

  • If there was an error somewhere along the way, we report it immediately, but also take a note that the error has been reported already, so that further executions of AsyncFunction that have been scheduled don't do anything in particular. This prevents your callback from potentially being called twice or more.

  • Be careful though - the order of elements inside toReturn will be random, depending on which async tasks complete first.


But this is messy

Oh yes. That's why we don't really do callbacks anymore. There's a pattern called promises that makes it significantly easier to work with async callback spaghetti, I suggest to read up a little on this topic before moving forward.