Node.js looping async calls

Question

I'm busy working on an endpoint for a reporting system. Node being async is giving me issues, although I'd rather not force it to be synchronous.

We're using MongoDB and Mongoose. I've got to query regex over collection A, then for each document that gets returned, query multiple contained documents to populate a JSON object/array to be returned.

I can use populate for most of the data, except the final looped queries which is where the async kicks in and returns my report early. Is there an elegant way to do this? Or should I be splitting into a different function and calling that multiple times to stick to the functions should do only one thing rule?

Example Code:

A.find({ name: regex }).populate({ path: 'B', populate: { path: 'B.C', model: 'C' } }).exec(function(err, A) {
            var report = [];

            A.map(function(a)){
                report[a.name] = [];

                D.aggregate([
                    {
                        $match: {
                            id: B._id
                        }
                    },
                    {
                        $group: {
                            _id: null,
                            count: { $sum: 1 }
                        }
                    }
                ], function(err, result) {
                    C.map(function(c){
                        report[a.name].push({
                            'field1': c.field1,
                            'field2': c.field2,
                            'field3': c.field3,
                            'count': result.count
                        });
                    });                        
                });
            }

            return report;
        });

The issue here is with the logic / async. Not with the syntax, hence the semi-pseudo code.

Any help or advice would be greatly appreciated.

LetterEh · Accepted Answer · 2017-01-31 09:59:08Z

You need to familiarize yourself with promises, and with async in general. Because you are returning an array, that's the value you are going to get.

You have a few options when dealing with Async, but in your case, you want to look at two solutions:

// callbacks
getSetOfIDs((err, ids) => {
  let remaining = ids.length;
  let things = [];
  let failed = false;

  ids.forEach(id => {
    getThingByID(id, (err, thing) => {
      if (failed) { return; }
      if (err) {
        failed = true;
        handleFailure(err);
      } else {
        remaining -= 1;
        things.push(thing);
        if (!remaining) {
          handleSuccess(things);
        }
      }
    });
  });
});

Note, I'm not returning things, I'm passing it into a callback.

You can use higher-order functions to clean this sort of thing up.

// cleaned up callbacks
function handleNodeCallback (succeed, fail) {
  return function (err, data) {
    if (err) {
      fail(err);
    } else {
      succeed(data);
    }
  };
}

function handleAggregateCallback (succeed, fail, count) {
  let items = [];
  let failed = false;

  const ifNotFailed = cb => data => {
    if (!failed) { cb(data); }
  };

  const handleSuccess = ifNotFailed((item) => {
    items.push(item);
    if (items.length === count) { succeed(items); }
  });

  const handleFailure = ifNotFailed((err) => {
    failed = true;
    fail(err);
  });

  return handleNodeCallback(handleSuccess, handleFailure);
}

A little helper code later, and we're ready to go:

// refactored callback app code (note that it's much less scary)
getSetOfIDs((err, ids) => {
  const succeed = (things) => app.display(things);
  const fail = err => app.apologize(err);
  if (err) { return fail(err); }

  let onThingResponse = handleAggregateCallback(succeed, fail, ids.length);
  ids.forEach(id => getThingByID(id, onThingResponse));
});

Note that aside from higher-order functions, I'm never returning anything, I'm always passing continuations (things to do next, with a value).

The other method is Promises

// Promises
getSetOfIDs()
  .then(ids => Promise.all(ids.map(getThingByID)))
  .then(things => app.display(things))
  .catch(err => app.apologize(err));

To really get what's going on here, learn Promises, the Promise.all static method, and array.map().

Both of these sets of code theoretically do the exact same thing, except that in this last case getSetOfIDs and getThingByID don't take callbacks, they return promises instead.

Rahul Kumar · Accepted Answer · 2017-01-31 09:21:26Z

usually in async calls, after return statement any operations are cancelled.

maybe you can return report object only when all is done and well.

A.find({ name: regex }).populate({ path: 'B', populate: { path: 'B.C', model: 'C' } }).exec(function(err, A) {
            var report = [];

            A.map(function(a)){
                report[a.name] = D.aggregate([
                    {
                        $match: {
                            id: B._id
                        }
                    },
                    {
                        $group: {
                            _id: null,
                            count: { $sum: 1 }
                        }
                    }
                ], function(err, result) {
                    if(err){
                      return [];
                    }
                    var fields = []
                    C.map(function(c){
                        fields.push({
                            'field1': c.field1,
                            'field2': c.field2,
                            'field3': c.field3,
                            'count': result.count
                        });
                    });
                    return fields;                       
                });     
            }
            return report;
        });

In this code, the report will be returned after the first iteration of the C documents returned?
Oops, you are right, but you know where to return report object, I would correct the answer
That's the problem that I'm having. If I return report at the end of the function, it returns empty. This is because as soon as the aggregate function is started, async kicks in and carries on processing the current function. This ends up hitting the return statement before report is populated.
I got the problem, in that case one trick is to assign the async return to the object values. Other way would be to use promises though.

Adriano Spadoni · Accepted Answer · 2017-01-31 09:39:30Z

Just use promises:

A.find({ name: regex }).populate({ path: 'B', populate: { path: 'B.C', model: 'C' } }).exec(function(err, A) {

   var report = [];
   return Promise.all([
      A.map(function(a)){
         return new Promise(function(resolve, reject) {

            report[a.name] = [];

            D.aggregate([{ $match: { id: B._id }},{$group: {_id: null,count: { $sum: 1 }}}], 
              function(err, result) {
                if(err) {
                  reject(err)
                } else {
                  C.map(function(c){
                    report[a.name].push({
                        'field1': c.field1,
                        'field2': c.field2,
                        'field3': c.field3,
                        'count': result.count
                    });
                  }); 
                  resolve(report)
                }  
            });
        }
    })])
  })
  .then(function(report){
     console.log(report)
   })
  .catch(function(err){
     console.log(err)
   })

Is that resolve going to happen several times (and thus finish with 1 element, instead of 0)?

Collectives™ on Stack Overflow

Node.js looping async calls

3 Answers 3

Comments

5 Comments

2 Comments

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

5 Comments

2 Comments

Related