Async parallel bug?

Question

Here I'm trying to make an array of functions with arguments to Async.js.

The array consists of instances of RunRequest that are supposed to be set inside the loop in MakeRequest, right before I try pass the function array to Async.

So the request in request[i] is fine when I pass it to RunRequest, but inside RunRequest function its undefined?

// Process Requests
function RunRequest(db, collection, request, requestHandler, callback) {
    console.log('this happening?')
    // Connect to the database
    db.open(function(err, db) {
        if(err) callback(err, null);
        // Connect to the collection
        db.collection(collection, function(err, collection) {
            if (err) callback(err, null);
            // Process the correct type of command
            requestHandler(db, collection, request, callback);  
        });
    });
}

function MakeRequest(request, requestHandler, collection, callback) {
    var data = [];
    var doneRequest = function(err, results) {
        console.log('done was called')
        if (err) callback(err, null);
        else if(results) data = data.concat(results);
    }
    // Make Request Array
    var requestArray = [];
    for(var i = 0; i < request.length; i++) {
        console.log('run request was called')
        var dbConnection = new Db('KidzpaceDB', new Server(Host, Port, {auto_reconnect: true}))
        requestArray.push(function() {RunRequest(dbConnection, collection, request[i], requestHandler, doneRequest)});
    }
    // Make all requests in Parallel then invoke callback
    Async.parallel(requestArray, function(err, results) {
        console.log('Step WORKS')
        if(data) {
            var uniqueResults = [];
            for(var i = 0; i < data.length; i++) {
                if( !uniqueResults[data[i]['_id']] ) {
                    uniqueResults[uniqueResults.length] = data[i];
                    uniqueResults[data[i]['_id']] = true;
                }
                callback (null, uniqueResults);
            }
        }
    });
}


// Request Handlers
var FindHandler = function(db, collection, request, callback) {
    console.log('FindHandler was called')
    console.log('Request Query' + request);
    collection.find(request.query, function(err, cursor) {
        if (err) callback(err, null);
        cursor.toArray(function(err, docs) {
            if (err) callback(err, null);
            if(docs.length <= 0) console.log("No documents match your query");
            var requestResults = [];
            for(var i=0; i<docs.length; i++) {
                requestResults[requestResults.length] = docs[i]; 
            }
            db.close();
            callback(null, requestResults);
        });
    });
}

score 1 · Answer 1 · answered Sep 13 '12 at 07:48

This is just a shot in the dark:

I think the problem is how you call RunRequest inside MakeRequest. Inside the first for-loop you are iterating over request and use request[i] inside an anonymous function, but i changes in the next iteration and the current scope gets lost when RunRequest is actually executed.

It's hard to reproduce, but try this:

var requestArray = [];
for(var i = 0; i < request.length; i++) {
    console.log('run request was called')
    var dbConnection = new Db('KidzpaceDB', new Server(Host, Port, {auto_reconnect: true}))

    function wrap(dbConnection, collection, request, requestHandler, doneRequest) {
        return function() {
            RunRequest(dbConnection, collection, request, requestHandler, doneRequest);
        }
    }
    requestArray.push(wrap(dbConnection, collection, request[i], requestHandler, doneRequest));
}

freakish · Accepted Answer · 2012-09-14T05:07:08.230

This is a scoping issue. When the loop finishes variable i is set to request.length, so request[i] is undefined.

Wrap your code with anonymous function like that:

var requestArray = [];
for(var i = 0; i < request.length; i++) {
    (function(i) {
        console.log('run request was called');
        var dbConnection = ...;
        requestArray.push( ... );
    })(i);
}

or even better ( avoids unnecessary overhead when creating anonymous functions ):

var requestArray = [];
request.forEach( function( el ) {
    console.log('run request was called');
    // the other code goes here, use el instead of request[i]
});

EDIT The callback is not called, because you don't define functions in arrays correctly. You will have to refactor your code a bit, so let me just show you how it should be:

requestArray.push(function(callback) { // <---- note the additional parameter here
    // do some stuff, for example call db
    db.open(function(err, db) {
        if (err) {
            callback( err );
        } else {
            callback( );
        }
    });
});

If you want to use RunRequest, then you need to pass callback as an additional parameter to RunRequest ( so use callback instead of doneRequest ).

That worked much better but unfortunately the Async.parallel callback is still never called... — Josh Elias, Sep 14 '12 at 00:25

Async parallel bug?

2 Answers2