1

I am new to Nodejs and need some guidance for writing better code. Here is my problem.

I have a function which is which I am using async water fall model. I want to call that function in a loop and break the loop if some thing goes wrong in the middle other wise notify some result at the end of for loop. But for some reason I am getting an undefined response.

function myFunc (arg1) {
     async.waterfall(
           [
               function() {
                        //do something
                        callback(null, data);
               },
               function(data, callback) {
                       //do something
                       callback(null, 'done');
               }
           ],
           function(err, result) {
               return {"error" : err, "res" : result};
           }
}

//Here I am calling my function 
for (var d in mydata) {
    var retdata =  myFunc (mydata[d]); //retdata has undefined in it as this line executes before the last function of water fall
    if (retdata.error !== 200) {
          return //  break loop
     }
  }
//Other wise if every thing is fine nofify after the for loop end
console.log ("done");

In short what is the correct and best way of notify the result (if true) at the end or break loop when last function of waterfall gives error.

user2243651
  • 325
  • 5
  • 13
  • you are using return inside the for loop. You should use a break instead. return exits the current execution context (i.e the function) break exits loops. – skarist Aug 05 '14 at 21:52
  • Yes I will do it. Thanks. Its a typo... But the main issue is some thing else here the retdata is always undefined because some how it is executing before the waterfall – user2243651 Aug 05 '14 at 21:56
  • well perhaps it is like you want to have it? Anyway, if this is running inside another function I guess you should at least return the error code. – skarist Aug 05 '14 at 21:56
  • I tried using callback as the last argument to myFunc but I need this callback to be executed only for last iteration and I am unable to find a cleaner way to do it. – user2243651 Aug 05 '14 at 21:59

2 Answers2

2

You can't use synchronous methods (like a for loop) and expect your code to wait till the asynchronous task completes. You can't use return to get data from an asynchronous function.

Here is a way to restructure your code using async.map. Take note of the callback structure. Also be sure you are referring to the async documentation.

//Async's methods take arrays:

var mydata = {a: 'foo', b: 'bar'};
var myarr;
for (var d in mydata) {
  myarr.push(mydata[d]);    
  // Beware, you might get unwanted Object properties
  // see e.g. http://stackoverflow.com/questions/3010840/loop-through-array-in-javascript/3010848#3010848
}

async.map(myarr, iterator, mapCallback);

function iterator(d, done){
  // async.map will call this once per myarr element, until completion or error
  myFunc(d, function(err, data){
    done(err, data);    
  });
};

function mapCallback(err, mapped){
  // async.map will call this after completion or error
  if(err){
    // Async ALREADY broke the loop for you.
    // Execution doesn't continue if the iterator function callsback with an
    // error.
  };
  console.log("Asynchronous result of passing mydata to myfunc:");
  console.log(mapped);
  // mapped is an array of returned data, in order, matching myarr.
};


function myFunc (arg1, callback) {
  async.waterfall(
  [
    function(done) {
      //do something
      done(null, data);
    },
    function(data, done) {
      //do something
      done(null, 'done');
    }
  ],
  function(err, result) {
    if (result !== 200) {
      return callback('Non-200');  
      // This return is important to end execution so you don't call the callback twice
    }

    callback(err, result);
  }
}
Plato
  • 10,812
  • 2
  • 41
  • 61
  • Thanks for your response @Plato. I have tried your method but one thing is not working for me in the mapCallBack function you have mentioned that ASYNC will break the loop if the iterator will return an error. But this is not happening in my case although error is being return, and it continues the execution – user2243651 Aug 06 '14 at 01:37
  • Also as you have mentioned that mapCallBack will be called only after the completion or in case of error. But for me it is being call after every iteration (basically done is calling it) – user2243651 Aug 06 '14 at 01:44
  • So I have noticed that myfunc is being called for myarr first element and before it goes to water fall functions it is being call for the 2nd element of myarr. which is causing an issue. Ideally for my case 2nd iteration should start once the first on is finished – user2243651 Aug 06 '14 at 01:59
  • @user2243651 To solve the issue in your most recent comment, use `async.mapSeries` instead of `async.map`. If mapCallback is really being called multiple times, then you must be invoking map multiple times. – Plato Aug 06 '14 at 22:38
  • @user2243651 And for the first issue, you can manually check for errors, and pass them to the appropriate callback. The callback's first argument is `null` if there is no error, and something truthy (like a string, an object, an Error) if there is one. – Plato Aug 06 '14 at 22:39
1

You are trying to mix both synchronous and async control flows. The problem is that your call to myFunc will return immediately before any of the waterfall functions inside myFunc executes.

Here is a real example that will work. It iterates through the array and terminates with an error if it sees a 5:

var async = require('async');

function myFunc(data, cb) {
   async.waterfall(
       [
           function(callback) {
             //do something
             callback(null, data);
           },
           function(data, callback) {
             //do something
             if (data === 5) {
                callback("Five", null);   // Throw error if we see 5
             } else {
                callback(null, 'done');
             }
           }
       ],
       function(err, result) {
           cb(err, result);
       }
   );
}

var mydata = [1,2,3,4,5,6];
async.eachSeries(mydata, function(d, cb) {
   console.log('Processing ' + d);
   myFunc(d, cb);
}, function(err) {
   // error happened in processing d
   // handle error
   console.log('Error ' + err);
});