Skip to content
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.

Sign up for GitHub

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

asyncify promisified functions #840

Merged
merged 12 commits into from
Jul 9, 2015
Merged

asyncify promisified functions #840

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b89b1ac
asyncify Promises
zartdinov Jul 8, 2015
3c2fd71
callback moved out from try/catch block
zartdinov Jul 9, 2015
ec63006
Merge branch 'master' of https://github.com/caolan/async
zartdinov Jul 9, 2015
4be3c68
ahh, forgot some console.log
zartdinov Jul 9, 2015
725fdb2
unit test for bluebird promisified functions
zartdinov Jul 9, 2015
dbcde70
test names modified to "promisified by ..."
zartdinov Jul 9, 2015
6f2530b
npm install --save-dev bluebird
zartdinov Jul 9, 2015
479d36c
added "should not get an error here" messages
zartdinov Jul 9, 2015
edee839
don't repeat yourself (DRY)
zartdinov Jul 9, 2015
458c73c
not used Promise was fixed
zartdinov Jul 9, 2015
173e913
unit test for NPO
zartdinov Jul 9, 2015
97f9825
node-only
zartdinov Jul 9, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/async.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1186,10 +1186,18 @@
var result;
try {
result = func.apply(this, args);
// if result is Promise object
if (typeof result.then === "function") {
result.then(function(values) {
var args = [null].concat(values);
callback.apply(this, args);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There can be only one value from promise so can you do

callback.call(this, value)

}).catch(callback);
} else {
callback(null, result);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to have the callback call in the try/catch block.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep: The failing test is the one that guards against this: https://travis-ci.org/caolan/async/jobs/70115957

}
} catch (e) {
return callback(e);
}
callback(null, result);
});
};

Expand Down