New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix "RangeError: Maximum call stack size exceeded" #1514
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! It looks good, except for a couple of small changes. We implemented something similar for queue
s in #1334, so I don't see an issue in adding this to eachOfLimit
(and the collection methods that use eachOfLimit
internally).
mocha_test/eachOf.js
Outdated
if (err) throw err; | ||
expect(count).to.equal(1024 * 1024); | ||
}); | ||
setTimeout(done, 25); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you call done
inside of the final callback instead?
mocha_test/eachOf.js
Outdated
@@ -274,6 +274,18 @@ describe("eachOf", function() { | |||
setTimeout(done, 25); | |||
}); | |||
|
|||
it('forEachOfLimit 1024 * 1024 synchronous call', function(done) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is minor, but we already have a forEachOfLimit synchronous
test. Could you rename this to something like forEachOfLimit no stackoverflow
or forEachOfLimit no call stack size exceed error
just to differentiate between the two?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add a similar test for forEachOf
and forEachOfSeries
to make sure the behaviour is consistent?
…it no call stack size exceed error' done called inside of the final callback of 'forEachOfLimit no call stack size exceed error' added 'forEachOf no call stack size exceed error' and 'forEachOfSeries no call stack size exceed error' tests
I don't know why the build initially failed, but it appears to be working now. LGTM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A clever, simple solution to a common problem.
👍 I was originally going to comment that this might be a misuse of async, but to be honest I've hit this case in my own personal use enough times to think its justified. That this solution is so simple is just icing on the cake. |
eachOfLimit
throws aRangeError: Maximum call stack size exceeded
when the collection is too big and the iteratee is synchronous.The usual workaround is to use
setImmediate(next)
or http://caolan.github.io/async/docs.html#ensureAsync.I have a data analysis program. The
setImmediate(next)
workaround made the whole program slow when there are few new data to analyse.With a small modification, it was possible to make the program lasting from minutes to seconds.
Here is an exemple of time duration with and without
setImmediate