HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Node.js async callback hell

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
asyncnodehellcallback

Problem

I'm attempting to create a final array with data pulled from 3 functions. The first two functions have all the data but then the 3rd function needs to run looping through one of the fields from function 2 in order to have the complete array at the end.

I'm looking for some advice and examples on how to fix this and or do it better. I have been advised promises would be a good solution, but could someone throw together a quick example so I can better get my head around it?

-
In my services controller I have this as the page here and checks if the user is logged in.

exports.all = function(req, res){
    if (!req.isAuthenticated()){
        res.redirect('login');
    } else {


-
It uses the services database schema in the model to connect to the database and pull out the info I need:

Database
    .find()
    .exec(function (err, services) {
      if (err) {
        return console.log(err);
      }
      if (!services) {
        console.log('Failed to load any Services');
      }


-
Once the services are found (multiple RPC/API services that have the same commands but different data returned), I async through each and connect.

async.mapSeries(services, function(service, cb) {
                connect(service);
                client.getBalance(req.user._id, 6, function(err, balance) {
                    console.log(balance);
                    if (err) balance = "Offline";
                    client.getAddressesByAccount(req.user._id, function(err, address) {
                        console.log(address);
                        if (err) address = "Offline";


-
Once the first two lots of data is returned (the first is just a string number and the second is an array of addresses then for each address I want too async again and run a third command and then map the result of each to the corresponding address from above and output a final array to pass to the view.

```
if(address != "Offline") {
async.mapSeries(

Solution

With Q promises it would look more or less like this (taking this chat into account):

var mongoose = require('mongoose') 
, Q = require('q')
, Wallet = mongoose.model('Wallet')
, _ = require('underscore')
, bitcoin = require('bitcoin');

function connect(coin) { 
  return new bitcoin.Client({ 
    host: coin.host, 
    port: coin.port, 
    user: coin.user, 
    pass: coin.pass, 
  }); 
}

var wallet = Wallet.find(),
    exec = Q.denodeify(wallet.exec.bind(wallet));

function getServiceBalance(service, userID) {
  var client = connect(service)
  , cmd = Q.denodeify(client.cmd.bind(client))
  , getBalance = Q.denodeify(client.getBalance.bind(client))
  , getAddressesByAccount = Q.denodeify(client.getAddressesByAccount.bind(client));

  // We're going to fill this in step by step...
  var result = {
    name: service.name,
    display: service.display,
    abr: service.abr
  };

  return getBalance(userID, 6)
    .then(function (balance) {
      result.balance = balance;
      console.log("Balance:" + balance);
      console.log("User ID: " + userID);
      return getAddressesByAccount(userID);
    })
    .then(function (addresses) {
      console.log("Addresses:", address);

      if (!_.isArray(addresses)) {
        // Throwing errors inside `then` is fine: they will get caught in `catch` below
        throw new Error('Could not get addresses array');
      }

      // For each address, call getreceivedbyaddress and gather results in array
      return Q.all(_.map(addresses, function (address) {
        return cmd('getreceivedbyaddress', address, 6).then(function (received) {
          return [address, received];
        });
      }));
    }).then(function (addressesReceived) {
      result.address = _.object(addressesReceived);
    }).catch(function (err) {
      // Here's the catch method--the *one and only* place all errors propagate to.
      console.log(err);

      result.balance = 'Offline';
      result.address = 'Offline';
    }).thenResolve(result);
}

exports.all = function (req, res) {
  if (!req.isAuthenticated()){
      res.redirect('login');
  } else {
    exec().then(function (services) {
      if (!services) {
        throw new Error('Failed to load any Services');
      }

      var promises = _.map(services, function (service) {
          return getServiceBalance(service, req.user._id);
      });

      return Q.all(promises);
    }).then(function (serviceBalances) {
      req.breadcrumbs('Services', '/services/all');
      res.render('services/index', {
        title: 'Services',
        icon: 'iconfa-cog',
        summary: 'Services information',
        services: serviceBalances,
        path : req.path,
        breadcrumbs: req.breadcrumbs(),
        udata : req.session.user
      });
    }).catch(function (err) {
      console.log('Error:', err);
      // TODO: render an error message?
    }).done();
  }
}


Note that we could've gone side-effect-free and remove captured result variable from getServiceBalance but I don't think it's worth the hassle, and in fact debugging with state is more convenient, as long as the state is properly isolated.

If you're looking to Q-style alternative to mapSeries, check out Q.all and Q.allSettled.

Code Snippets

var mongoose = require('mongoose') 
, Q = require('q')
, Wallet = mongoose.model('Wallet')
, _ = require('underscore')
, bitcoin = require('bitcoin');

function connect(coin) { 
  return new bitcoin.Client({ 
    host: coin.host, 
    port: coin.port, 
    user: coin.user, 
    pass: coin.pass, 
  }); 
}

var wallet = Wallet.find(),
    exec = Q.denodeify(wallet.exec.bind(wallet));

function getServiceBalance(service, userID) {
  var client = connect(service)
  , cmd = Q.denodeify(client.cmd.bind(client))
  , getBalance = Q.denodeify(client.getBalance.bind(client))
  , getAddressesByAccount = Q.denodeify(client.getAddressesByAccount.bind(client));

  // We're going to fill this in step by step...
  var result = {
    name: service.name,
    display: service.display,
    abr: service.abr
  };

  return getBalance(userID, 6)
    .then(function (balance) {
      result.balance = balance;
      console.log("Balance:" + balance);
      console.log("User ID: " + userID);
      return getAddressesByAccount(userID);
    })
    .then(function (addresses) {
      console.log("Addresses:", address);

      if (!_.isArray(addresses)) {
        // Throwing errors inside `then` is fine: they will get caught in `catch` below
        throw new Error('Could not get addresses array');
      }

      // For each address, call getreceivedbyaddress and gather results in array
      return Q.all(_.map(addresses, function (address) {
        return cmd('getreceivedbyaddress', address, 6).then(function (received) {
          return [address, received];
        });
      }));
    }).then(function (addressesReceived) {
      result.address = _.object(addressesReceived);
    }).catch(function (err) {
      // Here's the catch method--the *one and only* place all errors propagate to.
      console.log(err);

      result.balance = 'Offline';
      result.address = 'Offline';
    }).thenResolve(result);
}

exports.all = function (req, res) {
  if (!req.isAuthenticated()){
      res.redirect('login');
  } else {
    exec().then(function (services) {
      if (!services) {
        throw new Error('Failed to load any Services');
      }

      var promises = _.map(services, function (service) {
          return getServiceBalance(service, req.user._id);
      });

      return Q.all(promises);
    }).then(function (serviceBalances) {
      req.breadcrumbs('Services', '/services/all');
      res.render('services/index', {
        title: 'Services',
        icon: 'iconfa-cog',
        summary: 'Services information',
        services: serviceBalances,
        path : req.path,
        breadcrumbs: req.breadcrumbs(),
        udata : req.session.user
      });
    }).catch(function (err) {
      console.log('Error:', err);
      // TODO: render an error message?
    }).done();
  }
}

Context

StackExchange Code Review Q#38543, answer score: 2

Revisions (0)

No revisions yet.