2

I've followed this tutorial and now I'm building an Express js application connected to a Mysql database with a working connection pool; then i'm using Async.js for a waterfall of instructions in my server.

I was just refactoring my code avoiding callback hells. I'm very confused because I had a working code with nested forEach functions (callback hell situation) which perfectly acquired my MySql connection and now nothing appears to work.

The ISSUE: On GET request received, it prints until "Action Two" and it stucks.

This is my current MWE:

apiRoutes.get('/endpoint', function(req, res) {
async.waterfall([
function actionOne(callback){
    connection.acquire(function(err, con){
        con.query( myQuery , function(err, result){
            con.release();
            if(err){
                console.log(err);
                callback(err);
            } else {
                console.log("Action One Success");
                callback(null, result);
            }
        });
    });
},
function actionTwo(list, callback){
    console.log("Action Two");
    var arr = [];
    list.forEach(function(item, index, array){
        item.arr = [];
        connection.acquire(function(err, con){
            con.query( otherQuery , function(err, result){
                con.release();
                if(err){
                    console.log("SQL ERROR: "+err);
                    callback(err);
                } else {
                    item.arr = result;
                    arr.push(cult);
                    if(index === array.length-1){
                        console.log("Action Two Success");
                        callback(null, arr);
                    }
                }
            }
        });
    })
},
function actionThree(item, callback){
         ....
    res.json('success');
}],function(err){
     if(err)
      console.log(err);
});

}




SOLVED: Finally I've found the best readable solution for chain asynchronous instructions using Promises .

apiRoutes.get('/endpoint', function(req, res){

    //Define 1st function
    function actionOne(){
        return new Promise(function(fulfill, reject){
            myAsyncQueryFunction(err, result){
                if(err) { reject(err); }
                else { fulfill(result); }
            }
        }
    };

    //Define 2nd function
    function actionTwo(){
        return new Promise(function(fulfill, reject){
            actionOne().then(function(result){
                my2ndQueryFun(err, result){
                   if(err) { reject(err); }
                   else { fulfill(result); }
                }
            }, reject);
        }
    };

    //Execute async chained tasks, then send result to client
    actionTwo().then(function(result){
        res.json(result);
    };
}
Nicolò
  • 186
  • 1
  • 16

1 Answers1

1

If, by some reason, index === array.length-1 is never true, your code will be stuck forever at actionTwo since its callback function would never be called.

Also, it seems you're controlling when to call the actionTwo callback based on the index of the array you're querying. But that could lead to errors. Both connection.acquire and con.query are asynchronous so being at the last index does not guarantee that it is the last query to return. What if the query for the last item of the list is the fastest one to run?

Consider declaring all the SQL query tasks first and then controlling their flow with another async.js function like async.parallel or async.series

function actionTwo(list, callback){
    console.log("Action Two");


    var tasks = [];

    list.forEach(function(item, index, array){
        item.arr = [];
        tasks.push(async.apply(getDataFromSQL, item));
    })

    // could also be async.series(tasks, function (err, results) {
    // it depends if you want to run the queries in parallel or not
    async.parallel(tasks, function (err, results) {
        if (err) {
            console.log("Action Two Error")
            return callback(err);
        }
        console.log("Action Two Success");
        return callback(null, results); //results is an array of the "cult" objects returned at each getDataFromSQL callback
    });


    function getDataFromSQL(item, sqlCallback) {
        // otherQuery = generate_query_based_on_item(item);
        connection.acquire(function(err, con){
            con.query( otherQuery , function(err, result){
                con.release();
                if(err){
                    console.log("SQL ERROR: "+err);
                    sqlCallback(err);
                } else {
                    item.arr = result;
                    sqlCallback(null, cult); //not sure what cult meant. maybe result?
                }
            }
        });
    }
}
fmello
  • 563
  • 2
  • 9
  • SOLVED: The problem then was the asynchronous methods connection.acquire and con.query, which had no problem as first call, but the second never had the time to run since the entire method returns before the 2nd SQL result. Thank you very much! – Nicolò Oct 19 '16 at 12:41
  • Refering to using other async function. I chosed to use async.waterfall because each function need to pass his result to the next one. Look [here](http://stackoverflow.com/questions/9258603/what-is-the-difference-between-async-waterfall-and-async-series) – Nicolò Oct 20 '16 at 08:58