'Maximum call stack size exceeded' using async.forEachLimit

[bbigras]

async = require('async');

var documents = ['a', 'b', 'c', 'd', 'e', 'f'];

async.forEachLimit(documents, 2, function(item, callback) {
  console.log(".", item);
  callback(null);
}, function(err){
  console.log("end", err);
});

Log

$ node test.js
. a
. a
. a
. a
. a
. a
end undefined
. b
. b
. b
. b
. b
. b
. b
. b
. b
. b
. b
. b
. b
. b
. b
. b
[...]

node.js:134
        throw e; // process.nextTick error, or 'error' event on first tick
        ^
RangeError: Maximum call stack size exceeded

[Kami]

forEachLimit looks wrong.

started += 1 running += 1

should be moved in front of the iterator call. arr[started] should also be changed to arr[started - 1]

Note: You will only encounter this bug if code inside iterator iterator calls the callback in the same tick.

[bobrik]

STOP calling SYNC functions in async.js

[jacobrask]

I encountered this problem when I had a conditional return callback(null); in my iterator. It meant I had to do the smarter thing, which is to filter the array using Array.filter before applying async.forEach, so it was actually kind of helpful...

If you really think you should, you can work around this by using process.nextTick(function() { callback(); }).

[kontinuity]

@jacobrask That worked well. Thanks

[chesles]

Doing callback(null); is synchronous. You need to call it asynchronously, ala process.nextTick(callback), or process.nextTick(function() { callback(null); });. Like @bobrik said, stop calling sync functions in async :)

[thepatrick]

Saying "stop calling sync functions in async" is missing the point - it's broken, and it's only in forEachLimit that this appears to be an issue.

async.forEachLimit(['a','b'], 1, function(item, callback){
  if(something) {
    doStuff(function(){ callback(); });
  } else {
    callback();
  }
});

While I can fairly easy protect against that very specific example (as @jacobrask mentions by filtering), what if doStuff() (which might be in a library outside my control) decides to call my callback without a process.nextTick()?

[caolan]

I've added some code to call async.nextTick if it detects a synchronous iterator which should avoid a stack overflow in these cases. But seriously, STOP CALLING SYNC FUNCTIONS IN ASYNC! ...make your functions consistently synchronous or consistently asynchronous instead ;)

[webjay]

This also happened for me with async.eachSeries.

[alsotang]

Also happen in mapLimit, and yes my function is sync because it's a mock function. Then I wrap it in the setImmediate, the problem solved.

[webcaetano]

Happen with me on async.forEachOfLimit

[raeesaa]

Facing same issue with async.waterfall.

[tim-kos]

async.queue, too, of course.

[lipsumar]

I was doing a sync operation. I fixed it by changing from:

callback();

to

setTimeout(callback, 0);

[hitdrumhard]

This happened to me when I had a catch attached to a promise. Something like:
async.forEach(items, (item:any, callback:Function) => { someAsyncPromise(item).then(() => { callback(); }).catch((err) => { //console error or something here callback(); }) });

catch must be thrown before async bit in promise, because this error thrown only when exception occured in promise.

`

[ktamaral]

I understand that we should stop calling sync functions in async.

But, if calling the sync function callback() inside the async body is wrong, why does the async documentation show that in the examples?

Can someone help me understand what is the 'proper' way to work with the async module instead?

Here's an example directly from the async documentation:

// assuming openFiles is an array of file names
async.each(openFiles, function(file, callback) {

    // Perform operation on file here.
    console.log('Processing file ' + file);

    if( file.length > 32 ) {
      console.log('This file name is too long');
      callback('File name too long');
    } else {
      // Do work to process file here
      console.log('File processed');
      callback();
    }
}, function(err) {
    // if any of the file processing produced an error, err would equal that error
    if( err ) {
      // One of the iterations produced an error.
      // All processing will now stop.
      console.log('A file failed to process');
    } else {
      console.log('All files have been processed successfully');
    }
});

https://caolan.github.io/async/docs.html#each

Thanks!