Node.js/Async - How to avoid callback hell with async?

Question

I'm new to Node.Js and JavaScript web development on the backend. I see that callbacks inside callbacks could be a pain and there are modules to avoid that. One of these modules is async, https://github.com/caolan/async

I've read the documentation but it is hard to start and understand how to do it.

For example, I've this function "check_aut_user", How can I convert this code using async?

function check_auth_user(username, password, done) {

    var client = new pg.Client("pg://user:[email protected]/database");
    client.connect(function(err) {
        // If not get the connection
        if(err) { return console.error('could not connect to postgres', err); }

        // Query the user table
        client.query('select * from "user" where username = $1 and password = $2', [username, password], function(err, result) {
            if(err) { return console.error('error running query', err); }

            if (result.rowCount > 0) {
                var res = result.rows[0];
                console.log(res);

                passport.serializeUser(function(res, done) {
                    //console.log("serializer: " + res);
                    done(null, res);
                });

                passport.deserializeUser(function(user, done) {
                    //console.log("deserializer: " + user['password']);
                    done(null, res);
                }); 

                return done(null, res);
            } else {
                return done(null, false);
            }               
        });     
    });
}

Best Regards,

Dorcasdorcea

in a hurry, but here's a basic parallel example; should get you started: pastebin.com/z2M7RmeH – Vagrant

Your callback, done() can be called multiple times in your code. Here's some info on understanding callbacks and how they work: javascriptissexy.com/… – Deedradeeds

Im not too sure async is going to clean up this example all that much. But where you have a serial dependance in your exection, that is, you need to complete the connect() before running the query, you should look at async.series rather than async.parallel. – Wahkuna

Maybe this should be moved to stackexchange code review since it is not really about solving a programming problem. – Tenderfoot

lol I have what you call "callback hell" in my code. +1 – Gennygeno

I've seen worse, but jumping to using the async lib isn't the first thing you should be trying to do. Read that link I posted. – Deedradeeds

Every library has its own learning curve. I would say learn by practising the basic control flow functions in async. When you learn what each one does, then try solving the problem at hand. Also you can check this Q library. I find it much cleaner(clearer) to work with, than with just callbacks. It will require more learning though. – Gassy

Callback is the beauty/backbone of JS, just looks like hell, but works like a charm. thinking asynchronous :). As said above it can be moved to review forum. – Vanwinkle

I know this is not directly related to your question, but have you looked at iced coffeescript? coffeescript is a language that compiles into javascript, the iced version adds 2 powerful keywords, await and defer, which make callback hell much more readable. You have to learn a new language though, but I have to say that it helped me a lot – Scibert

Answer 1

There are ways to combat runaway nesting using functional programming techniques. I use the curry module to break loop bodies out into stand-alone routines; usually this carries a very minor performance hit versus nesting (study curry for why). Example:

var curry = require('curry'),
    async = require('async');

var eachBody = curry(function(context, item, callback) {
  callback(null, context + item);  // first argument is error indicator
});

function exampleLoop(data, callback) {
  // use the curried eachBody instead of writing the function inline
  async.map(data, eachBody(data.length), callback);
}

function print(err, result) {
  console.log(result);
}

exampleLoop([2, 4, 6], print);  // prints [5, 7, 9]
exampleLoop([2, 4, 6, 7], print);  // prints [6, 8, 10, 11]

Instead of:

var async = require('async');

function exampleLoop(data) {
  async.map(data, function(item, callback) {
    callback(null, data.length + item);  
  }, function(err, result) {
    console.log(result);
  });
}

exampleLoop([2, 4, 6]);  // prints [5, 7, 9]
exampleLoop([2, 4, 6, 7]);  // prints [6, 8, 10, 11]

The second example is more compact, but the more nesting you add, the less readable it becomes. Also, by splitting up the implementation, you can reuse component functions in multiple ways, and you can write unit tests for the individual component functions, not only for the high-level functionality.

Beelzebub

Answer 2

In my view, callback hell is really a mixture of two problems:

• Anonymous inline functions.
• Indentation.

Either one in small amounts is fine, but together they make code rigid and unmaintainable. The solution to avoiding callback hell is to avoid these two things by:

• Naming your functions and pulling them out of function calls.
• Returning early to avoid intendation.

Going by these principles, your code can be rewritten as:

function check_auth_user(username, password, done) {

    // Make a new client and open the connection.
    function connect(callback) {
        var client = new pg.Client("pg://user:[email protected]/database");

        client.connect(function (err) {
            if (err) {
                console.error('could not connect to postgres', err);
                return callback(err);
            }

            callback(null, client);
        });
    }

    // Query the database.
    function query(callback, results) {
        var client = results.client;
        var q = 'select * from "user" where username = $1 and password = $2';
        var params = [username, password];

        client.query(q, params, function (err, result) {
            if (err) {
                console.error('error running query', err.stack || err.message);
                return callback(err);
            }

            callback(null, result);
        });
    }

    // Do stuff with the result of the query.
    function handleQueryResult(callback, results) {
        var result = results.query;

        if (result.rowCount === 0) {
            return callback();
        }

        var row = result.rows[0];

        console.log(row);

        passport.serializeUser(function(res, done) {
            done(null, res);
        });

        passport.deserializeUser(function(user, done) {
            done(null, res);
        }); 

        callback(null, row);
    }

    // Let async handle the order. Allows remixing.
    async.auto({
        connect: [connect],
        query: ['connect', query],
        row: ['query', handleQueryResult]
    }, function (err, results) {
        if (err) {
            return done(err);
        }

        // Callback with the row.
        done(null, results.row);
    });
}

I've used async.auto here, even if async.waterfall would do. The reasoning behind that is that it's difficult to move, add or remove steps in a waterfall, and that's been a source of bugs. The auto lets you add steps without worrying and the order/parallelism is handled by async.

This is obviously using a lot more vertical space, but I think that's a small price to pay for the modularity.

Zoril

Answer 3

Here is the code modified to use async whenever needed.

function check_auth_user(username, password) {
    var client = new pg.Client("pg://user:[email protected]/database");
    async.waterfall([
      client.connect,
      function(callback) {
        client.query('select * from "user" where username = $1 and password = $2', [username, password], callback);
      },
      function(result, callback) {
        if(result.rowCount > 0) {
                var res = result.rows[0];

                async.series([
                   function(callback) {
                     passport.serializeUser(res, function(res, done) {
                       callback(null, res);
                     }
                   },
                   function(res, callback){
                     if(res) {
                       passport.deserializeUser(res, function(res, done) {
                          callback(null, res);
                       });
                     } else {
                       callback(new Error('SerializeUser failed'));
                     }
                   }
                ], function(err, res) {
                    if(err) {
                       callback(err);
                    } else {
                       callback(null);
                    }
                });
        }
        else {
           callback(new Error("No result"));
        }
      }
    ], function(err, result) {
       if(err)
          console.err(err);
    });
}

Obviously, it does not make the code any more readable. I would suggest additional changes:

• Querying the database should be separated in its own method (it is indeed a model method), thus allowing error checking to happen in its own "dominion".
• Passport serialization/deserialization should be done in a separate method, for the same reasons.

These two methods would both take callbacks. Here is a re-write:

// Given a client, queries for user
function retrieveUser( client, username, password, cb) {
  client.query('select * from "user" where username = $1 and password = $2', [username, password], function(err, users){
    if(err) {
      cb(err);
    }
    else if(users.rowCount < 0) {
      cb(new Error("No user found for username ="+username));
    }
    else {
      cb(null, result.rows[0]);
  });
}

//Uses passport to serialize/deserializeUser
function passportSerialize(user, cb){
      async.series([
       function(callback) {
         passport.serializeUser(user, function(res, done) {
           callback(null, res);
         }
       },
       function(res, callback){
         if(res) {
           passport.deserializeUser(res, function(res, done) {
              if(res) {
                 callback(null, res);
              } else {
                 callback(new Error('deserializeUser failed'));
              } 
           });
         } else {
           callback(new Error('SerializeUser failed'));
         }
       }
    ], cb);
}

Thus, our main method now becomes:

function check_auth_user(username, password) {
    var client = new pg.Client("pg://user:[email protected]/database");
    async.waterfall([
      client.connect,
      function(callback) {
        retrieveUser(client, username, password, callback);
      },
      function(user, callback) {
            passportSerialize(user, callback);
      }
    ], function(err, result) {
       if(err)
          console.err(err);
        else
          console.log("User authenticated, let her in");
    });
}

I hope you can see how this is much much better.

Tenderfoot