Node.js Async waterfall() callback was already called

Question

I am using async module on node.js from yesterday to handle the order of asynchronous tasks. Below code works, But the error:

Callback was already called

shown about 1 to 5 times. I don't know where is the problem, maybe the callback in forEach loop on getNewsTitles() called not just one time. So I put console.log in here but this log printed only one time whether error is shown or not.

async.waterfall([ function(callback) { callback(); }, function(callback) { // When error, below log doesn't show. console.log('getting news titles...'); getNewsTitles(arr_uri, arr_subs, function() { // * The problem is here => "Callback was already called" callback(null); }); } ], function(err, result) { if (err) return next(); else res.send(arr_subs); }); function getNewsTitles(targets, subs, callback) { targets.forEach(function(current, index) { request.get({ uri: current, encoding: null }, function(err, response, body) { if (!err && response.statusCode == 200) { var $ = cheerio.load(iconv.decode(body, 'EUC-KR')); var subject = $('.articleSubject a'); for (var i = 0; i < subject.length; i++) { subs.push(subject[i].attribs.title); } if (subs.length == (targets.length - 2) * 20 + 2) { // when error or not, below log shows one time. console.log('doubt here too'); callback(); } } }); }) }

Did I miss something..?

Ben · Accepted Answer · 2016-05-26 13:50:57Z

request.get() is an async call. Regular for loop would not work. In the code above, callback() gets called every successful request.get(). You need something that you can control the flow such as async.each(), async.eachLimit(), async.eachSeries() and etc, so that callback() is called only once.

I would recommend using async.eachLimit() over async.each() in this scenario to throttle the maximum number of request.get() so you don't flush the server with too many request.get() at a time. In the example below, I use 5 as a max number of requests processing concurrently but you can change the value your server ecan handle:

function getNewsTitles(targets, subs, callback) { async.eachLimit(targets, 5, function (current, eachCb) { request.get({ uri: current, encoding: null }, function(err, response, body) { if (!err && response.statusCode == 200) { var $ = cheerio.load(iconv.decode(body, 'EUC-KR')); var subject = $('.articleSubject a'); for (var i = 0; i < subject.length; i++) { subs.push(subject[i].attribs.title); } if (subs.length == (targets.length - 2) * 20 + 2) { // when error or not, below log shows one time. console.log('doubt here too'); } } eachCb(null); // must be called for every iteration of async.eachLimit() }); }, function (err) { callback(null); // all items have been processed, call this callback only once }); }

Thanks ^-^. It works well. But I wonder, you said callback() gets called every successful request.get().. But I intended callback() called only once by this if (subs.length == (targets.length - 2) * 20 + 2) { callback(); } This condition will match only once in for loop. Your code works well and I feel very thanks but I don't understand why my code shown errors... Is it better to make a new question? Thanks.
I wrote fiddle jsfiddle.net/bexoss/1xqLhqrr . I think this is same to question's code but question not works well but fiddle code works well... haha.... maybe I need to make a new question if same problem happens to me
Actually, you're right... it's not every successful request.get()... I looked at the 1st if (check for error) but didn't have a closer look at the 2nd if you have there.
The reason the fiddle code works because the callback you call is your own callback, which is function(){ counter++; if(counter == uris.length) console.log('All done !');}.... this code here doesn't work because the callback you're calling is async.waterfall's callback. async.waterfall's callback can be called only once. If you call more than once, you'll get the error "Callback was already called"

Collectives™ on Stack Overflow

Node.js Async waterfall() callback was already called

1 Answer 1

4 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Related