-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
async.apply doesn't work as documented. #129
Comments
Apparently this is because |
I figured out the issue. When async.auto calls the continuation created by async.apply, it includes an extra 'results' argument (the results of the previous functions) as expected, which is passed on to the original function passed to apply. So when we create a continuation with var f = async.apply(fs.readFile, 'hello.txt', 'utf-8'); And then it is called by async.auto like f(callback, {}); The result is as if we called fs.readFile('hello.txt', 'utf-8', callback, {}); Which happens never to call the callback, as you can test for yourselves. Whether or not this design decision on the part of the authors of fs is something we agree with, this phenomenon may reappear in any number of places, the crux of the issue being that it isn't necessarily safe to pass extra arguments around to arbitrary functions. Therefore, in order to use the functionality of async.auto safely, all functions MUST be manually wrapped in a way that prevents the results argument from being passed around unintentionally. I propose adding something to this effect to the documentation of async.auto. |
yep optional arguments cause all kinds of issues with partial application :( ...I think adding a warning to the readme is the best we can do. |
Despite what the documentation implies, it cannot be used with
async.auto
.Same result with
f = fs.readFile.bind(null, 'hello.txt', 'utf-8')
.The text was updated successfully, but these errors were encountered: