From 059f2960503eec1418c32226646f5a2af6ae85f0 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 14 Apr 2017 18:27:46 +0200 Subject: [PATCH 1/8] util: add internal bindings for promise handling Add methods for creating, resolving and rejecting promises using the V8 C++ API that does not require creation of extra `resolve` and `reject` functions to `process.binding('util')`. PR-URL: https://github.com/nodejs/node/pull/12442 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Myles Borins Reviewed-By: Evan Lucas Reviewed-By: William Kapke Reviewed-By: Timothy Gu Reviewed-By: Teddy Katz --- src/node_util.cc | 35 ++++++++++++++++ .../test-promise-internal-creation.js | 41 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 test/parallel/test-promise-internal-creation.js diff --git a/src/node_util.cc b/src/node_util.cc index 5f3de643d4c2f3..3424739cc96688 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -12,6 +12,7 @@ using v8::Context; using v8::FunctionCallbackInfo; using v8::Integer; using v8::Local; +using v8::Maybe; using v8::Object; using v8::Private; using v8::Promise; @@ -147,6 +148,36 @@ void WatchdogHasPendingSigint(const FunctionCallbackInfo& args) { } +void CreatePromise(const FunctionCallbackInfo& args) { + Local context = args.GetIsolate()->GetCurrentContext(); + auto maybe_resolver = Promise::Resolver::New(context); + if (!maybe_resolver.IsEmpty()) + args.GetReturnValue().Set(maybe_resolver.ToLocalChecked()); +} + + +void PromiseResolve(const FunctionCallbackInfo& args) { + Local context = args.GetIsolate()->GetCurrentContext(); + Local promise = args[0]; + CHECK(promise->IsPromise()); + if (promise.As()->State() != Promise::kPending) return; + Local resolver = promise.As(); // sic + Maybe ret = resolver->Resolve(context, args[1]); + args.GetReturnValue().Set(ret.FromMaybe(false)); +} + + +void PromiseReject(const FunctionCallbackInfo& args) { + Local context = args.GetIsolate()->GetCurrentContext(); + Local promise = args[0]; + CHECK(promise->IsPromise()); + if (promise.As()->State() != Promise::kPending) return; + Local resolver = promise.As(); // sic + Maybe ret = resolver->Reject(context, args[1]); + args.GetReturnValue().Set(ret.FromMaybe(false)); +} + + void Initialize(Local target, Local unused, Local context) { @@ -192,6 +223,10 @@ void Initialize(Local target, env->SetMethod(target, "startSigintWatchdog", StartSigintWatchdog); env->SetMethod(target, "stopSigintWatchdog", StopSigintWatchdog); env->SetMethod(target, "watchdogHasPendingSigint", WatchdogHasPendingSigint); + + env->SetMethod(target, "createPromise", CreatePromise); + env->SetMethod(target, "promiseResolve", PromiseResolve); + env->SetMethod(target, "promiseReject", PromiseReject); } } // namespace util diff --git a/test/parallel/test-promise-internal-creation.js b/test/parallel/test-promise-internal-creation.js new file mode 100644 index 00000000000000..7142c872d9452e --- /dev/null +++ b/test/parallel/test-promise-internal-creation.js @@ -0,0 +1,41 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { + createPromise, promiseResolve, promiseReject +} = process.binding('util'); +const { inspect } = require('util'); + +common.crashOnUnhandledRejection(); + +{ + const promise = createPromise(); + assert.strictEqual(inspect(promise), 'Promise { }'); + promiseResolve(promise, 42); + assert.strictEqual(inspect(promise), 'Promise { 42 }'); + promise.then(common.mustCall((value) => { + assert.strictEqual(value, 42); + })); +} + +{ + const promise = createPromise(); + const error = new Error('foobar'); + promiseReject(promise, error); + assert(inspect(promise).includes(' Error: foobar')); + promise.catch(common.mustCall((value) => { + assert.strictEqual(value, error); + })); +} + +{ + const promise = createPromise(); + const error = new Error('foobar'); + promiseReject(promise, error); + assert(inspect(promise).includes(' Error: foobar')); + promiseResolve(promise, 42); + assert(inspect(promise).includes(' Error: foobar')); + promise.catch(common.mustCall((value) => { + assert.strictEqual(value, error); + })); +} From 99da8e8e02a874a0a044889f863c45700509d02c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 14 Apr 2017 18:28:16 +0200 Subject: [PATCH 2/8] util: add util.promisify() Add `util.promisify(function)` for creating promisified functions. Includes documentation and tests. Fixes: https://github.com/nodejs/CTC/issues/12 PR-URL: https://github.com/nodejs/node/pull/12442 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Myles Borins Reviewed-By: Evan Lucas Reviewed-By: William Kapke Reviewed-By: Timothy Gu Reviewed-By: Teddy Katz --- doc/api/util.md | 82 ++++++++++++++++++++++++++++ lib/internal/util.js | 61 +++++++++++++++++++++ lib/util.js | 2 + src/node_util.cc | 1 + test/parallel/test-util-promisify.js | 76 ++++++++++++++++++++++++++ 5 files changed, 222 insertions(+) create mode 100644 test/parallel/test-util-promisify.js diff --git a/doc/api/util.md b/doc/api/util.md index 9de5abd7a85304..7a9a779cbab935 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -399,6 +399,86 @@ util.inspect.defaultOptions.maxArrayLength = null; console.log(arr); // logs the full array ``` +## util.promisify(original) + + +* `original` {Function} + +Takes a function following the common Node.js callback style, i.e. taking a +`(err, value) => ...` callback as the last argument, and returns a version +that returns promises. + +For example: + +```js +const util = require('util'); +const fs = require('fs'); + +const stat = util.promisify(fs.stat); +stat('.').then((stats) => { + // Do something with `stats` +}).catch((error) => { + // Handle the error. +}); +``` + +Or, equivalently using `async function`s: + +```js +const util = require('util'); +const fs = require('fs'); + +const stat = util.promisify(fs.stat); + +async function callStat() { + const stats = await stat('.'); + console.log(`This directory is owned by ${stats.uid}`); +} +``` + +If there is an `original[util.promisify.custom]` property present, `promisify` +will return its value, see [Custom promisified functions][]. + +`promisify()` assumes that `original` is a function taking a callback as its +final argument in all cases, and the returned function will result in undefined +behaviour if it does not. + +### Custom promisified functions + +Using the `util.promisify.custom` symbol one can override the return value of +[`util.promisify()`][]: + +```js +const util = require('util'); + +function doSomething(foo, callback) { + // ... +} + +doSomething[util.promisify.custom] = function(foo) { + return getPromiseSomehow(); +}; + +const promisified = util.promisify(doSomething); +console.log(promisified === doSomething[util.promisify.custom]); + // prints 'true' +``` + +This can be useful for cases where the original function does not follow the +standard format of taking an error-first callback as the last argument. + +### util.promisify.custom + + +* {symbol} + +A Symbol that can be used to declare custom promisified variants of functions, +see [Custom promisified functions][]. + ## Deprecated APIs The following APIs have been deprecated and should no longer be used. Existing @@ -878,7 +958,9 @@ Deprecated predecessor of `console.log`. [`console.error()`]: console.html#console_console_error_data_args [`console.log()`]: console.html#console_console_log_data_args [`util.inspect()`]: #util_util_inspect_object_options +[`util.promisify()`]: #util_util_promisify_original [Custom inspection functions on Objects]: #util_custom_inspection_functions_on_objects [Customizing `util.inspect` colors]: #util_customizing_util_inspect_colors +[Custom promisified functions]: #util_custom_promisified_functions [constructor]: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/constructor [semantically incompatible]: https://github.com/nodejs/node/issues/4179 diff --git a/lib/internal/util.js b/lib/internal/util.js index 1633a920d55209..4dfcf81837b912 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -4,6 +4,8 @@ const errors = require('internal/errors'); const binding = process.binding('util'); const signals = process.binding('constants').os.signals; +const { createPromise, promiseResolve, promiseReject } = binding; + const kArrowMessagePrivateSymbolIndex = binding['arrow_message_private_symbol']; const kDecoratedPrivateSymbolIndex = binding['decorated_private_symbol']; const noCrypto = !process.versions.openssl; @@ -217,3 +219,62 @@ module.exports = exports = { // default isEncoding implementation, just in case userland overrides it. kIsEncodingSymbol: Symbol('node.isEncoding') }; + +const kCustomPromisifiedSymbol = Symbol('util.promisify.custom'); +const kCustomPromisifyArgsSymbol = Symbol('customPromisifyArgs'); + +function promisify(orig) { + if (typeof orig !== 'function') { + const errors = require('internal/errors'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'original', 'function'); + } + + if (orig[kCustomPromisifiedSymbol]) { + const fn = orig[kCustomPromisifiedSymbol]; + if (typeof fn !== 'function') { + throw new TypeError('The [util.promisify.custom] property must be ' + + 'a function'); + } + Object.defineProperty(fn, kCustomPromisifiedSymbol, { + value: fn, enumerable: false, writable: false, configurable: true + }); + return fn; + } + + // Names to create an object from in case the callback receives multiple + // arguments, e.g. ['stdout', 'stderr'] for child_process.exec. + const argumentNames = orig[kCustomPromisifyArgsSymbol]; + + function fn(...args) { + const promise = createPromise(); + try { + orig.call(this, ...args, (err, ...values) => { + if (err) { + promiseReject(promise, err); + } else if (argumentNames !== undefined && values.length > 1) { + const obj = {}; + for (var i = 0; i < argumentNames.length; i++) + obj[argumentNames[i]] = values[i]; + promiseResolve(promise, obj); + } else { + promiseResolve(promise, values[0]); + } + }); + } catch (err) { + promiseReject(promise, err); + } + return promise; + } + + Object.setPrototypeOf(fn, Object.getPrototypeOf(orig)); + + Object.defineProperty(fn, kCustomPromisifiedSymbol, { + value: fn, enumerable: false, writable: false, configurable: true + }); + return Object.defineProperties(fn, Object.getOwnPropertyDescriptors(orig)); +} + +promisify.custom = kCustomPromisifiedSymbol; + +exports.promisify = promisify; +exports.customPromisifyArgs = kCustomPromisifyArgsSymbol; diff --git a/lib/util.js b/lib/util.js index 4e626a3accc3f8..d73b12dcfd59b3 100644 --- a/lib/util.js +++ b/lib/util.js @@ -1057,3 +1057,5 @@ exports._exceptionWithHostPort = function(err, // process.versions needs a custom function as some values are lazy-evaluated. process.versions[exports.inspect.custom] = (depth) => exports.format(JSON.parse(JSON.stringify(process.versions))); + +exports.promisify = internalUtil.promisify; diff --git a/src/node_util.cc b/src/node_util.cc index 3424739cc96688..50de94bfb2bf3a 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -21,6 +21,7 @@ using v8::Value; #define VALUE_METHOD_MAP(V) \ + V(isAsyncFunction, IsAsyncFunction) \ V(isDataView, IsDataView) \ V(isDate, IsDate) \ V(isExternal, IsExternal) \ diff --git a/test/parallel/test-util-promisify.js b/test/parallel/test-util-promisify.js new file mode 100644 index 00000000000000..23b3d400375239 --- /dev/null +++ b/test/parallel/test-util-promisify.js @@ -0,0 +1,76 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const vm = require('vm'); +const { promisify } = require('util'); + +common.crashOnUnhandledRejection(); + +const stat = promisify(fs.stat); + +{ + const promise = stat(__filename); + assert(promise instanceof Promise); + promise.then(common.mustCall((value) => { + assert.deepStrictEqual(value, fs.statSync(__filename)); + })); +} + +{ + const promise = stat('/dontexist'); + promise.catch(common.mustCall((error) => { + assert(error.message.includes('ENOENT: no such file or directory, stat')); + })); +} + +{ + function fn() {} + function promisifedFn() {} + fn[promisify.custom] = promisifedFn; + assert.strictEqual(promisify(fn), promisifedFn); + assert.strictEqual(promisify(promisify(fn)), promisifedFn); +} + +{ + function fn() {} + fn[promisify.custom] = 42; + assert.throws( + () => promisify(fn), + (err) => err instanceof TypeError && + err.message === 'The [util.promisify.custom] property must ' + + 'be a function'); +} + +{ + const fn = vm.runInNewContext('(function() {})'); + assert.notStrictEqual(Object.getPrototypeOf(promisify(fn)), + Function.prototype); +} + +{ + function fn(callback) { + callback(null, 'foo', 'bar'); + } + promisify(fn)().then(common.mustCall((value) => { + assert.deepStrictEqual(value, 'foo'); + })); +} + +{ + function fn(callback) { + callback(null); + } + promisify(fn)().then(common.mustCall((value) => { + assert.strictEqual(value, undefined); + })); +} + +{ + function fn(callback) { + callback(); + } + promisify(fn)().then(common.mustCall((value) => { + assert.strictEqual(value, undefined); + })); +} From 3ea2301e38677a61af0cb3093138869294358339 Mon Sep 17 00:00:00 2001 From: Madara Uchiha Date: Tue, 18 Apr 2017 20:09:29 +0300 Subject: [PATCH 3/8] test: add a bunch of tests from bluebird Take tests from Bluebird's promisify's tests and adapted them to the format in use here. Add tests making sure things work with async functions. Add basic usability tests. PR-URL: https://github.com/nodejs/node/pull/12442 Reviewed-By: Anna Henningsen Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Myles Borins Reviewed-By: Evan Lucas Reviewed-By: William Kapke Reviewed-By: Timothy Gu Reviewed-By: Teddy Katz --- test/parallel/test-util-promisify.js | 94 ++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/test/parallel/test-util-promisify.js b/test/parallel/test-util-promisify.js index 23b3d400375239..04e7647ff9893d 100644 --- a/test/parallel/test-util-promisify.js +++ b/test/parallel/test-util-promisify.js @@ -74,3 +74,97 @@ const stat = promisify(fs.stat); assert.strictEqual(value, undefined); })); } + +{ + function fn(err, val, callback) { + callback(err, val); + } + promisify(fn)(null, 42).then(common.mustCall((value) => { + assert.strictEqual(value, 42); + })); +} + +{ + function fn(err, val, callback) { + callback(err, val); + } + promisify(fn)(new Error('oops'), null).catch(common.mustCall((err) => { + assert.strictEqual(err.message, 'oops'); + })); +} + +{ + function fn(err, val, callback) { + callback(err, val); + } + + (async () => { + const value = await promisify(fn)(null, 42); + assert.strictEqual(value, 42); + })(); +} + +{ + const o = {}; + const fn = promisify(function(cb) { + + cb(null, this === o); + }); + + o.fn = fn; + + o.fn().then(common.mustCall(function(val) { + assert(val); + })); +} + +{ + const err = new Error('Should not have called the callback with the error.'); + const stack = err.stack; + + const fn = promisify(function(cb) { + cb(null); + cb(err); + }); + + (async () => { + await fn(); + await Promise.resolve(); + return assert.strictEqual(stack, err.stack); + })(); +} + +{ + function c() { } + const a = promisify(function() { }); + const b = promisify(a); + assert.notStrictEqual(c, a); + assert.strictEqual(a, b); +} + +{ + let errToThrow; + const thrower = promisify(function(a, b, c, cb) { + errToThrow = new Error(); + throw errToThrow; + }); + thrower(1, 2, 3) + .then(assert.fail) + .then(assert.fail, (e) => assert.strictEqual(e, errToThrow)); +} + +{ + const err = new Error(); + + const a = promisify((cb) => cb(err))(); + const b = promisify(() => { throw err; })(); + + Promise.all([ + a.then(assert.fail, function(e) { + assert.strictEqual(err, e); + }), + b.then(assert.fail, function(e) { + assert.strictEqual(err, e); + }) + ]); +} From e965ed16c111f219116a5e8ea6847f5b98b5d0be Mon Sep 17 00:00:00 2001 From: Gil Tayar Date: Tue, 18 Apr 2017 20:58:33 +0300 Subject: [PATCH 4/8] test: add test for promisify customPromisifyArgs PR-URL: https://github.com/nodejs/node/pull/12442 Reviewed-By: Anna Henningsen Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Myles Borins Reviewed-By: Evan Lucas Reviewed-By: William Kapke Reviewed-By: Timothy Gu Reviewed-By: Teddy Katz --- test/parallel/test-util-promisify.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/parallel/test-util-promisify.js b/test/parallel/test-util-promisify.js index 04e7647ff9893d..84919714154b28 100644 --- a/test/parallel/test-util-promisify.js +++ b/test/parallel/test-util-promisify.js @@ -1,9 +1,11 @@ 'use strict'; +// Flags: --expose-internals const common = require('../common'); const assert = require('assert'); const fs = require('fs'); const vm = require('vm'); const { promisify } = require('util'); +const { customPromisifyArgs } = require('internal/util'); common.crashOnUnhandledRejection(); @@ -42,6 +44,21 @@ const stat = promisify(fs.stat); 'be a function'); } +{ + const firstValue = 5; + const secondValue = 17; + + function fn(callback) { + callback(null, firstValue, secondValue); + } + + fn[customPromisifyArgs] = ['first', 'second']; + + promisify(fn)().then(common.mustCall((obj) => { + assert.deepStrictEqual(obj, {first: firstValue, second: secondValue}); + })); +} + { const fn = vm.runInNewContext('(function() {})'); assert.notStrictEqual(Object.getPrototypeOf(promisify(fn)), From e7c51454b0de0a6b33c15a4d77006a17ced4eeff Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 14 Apr 2017 21:42:47 +0200 Subject: [PATCH 5/8] timers: add promisify support Add support for `util.promisify(setTimeout)` and `util.promisify(setImmediate)` as a proof-of-concept implementation. `clearTimeout()` and `clearImmediate()` do not work on those Promises. Includes documentation and tests. PR-URL: https://github.com/nodejs/node/pull/12442 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Myles Borins Reviewed-By: Evan Lucas Reviewed-By: William Kapke Reviewed-By: Timothy Gu Reviewed-By: Teddy Katz --- doc/api/timers.md | 38 ++++++++++++++++++++++ lib/timers.js | 26 +++++++++++++-- test/parallel/test-timers-promisified.js | 40 ++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-timers-promisified.js diff --git a/doc/api/timers.md b/doc/api/timers.md index 2b6af6f66f41fe..8abcdcb5cb6890 100644 --- a/doc/api/timers.md +++ b/doc/api/timers.md @@ -85,6 +85,27 @@ next event loop iteration. If `callback` is not a function, a [`TypeError`][] will be thrown. +*Note*: This method has a custom variant for promises that is available using +[`util.promisify()`][]: + +```js +const util = require('util'); +const setImmediatePromise = util.promisify(setImmediate); + +setImmediatePromise('foobar').then((value) => { + // value === 'foobar' (passing values is optional) + // This is executed after all I/O callbacks. +}); + +// or with async function +async function timerExample() { + console.log('Before I/O callbacks'); + await setImmediatePromise(); + console.log('After I/O callbacks'); +} +timerExample(); +``` + ### setInterval(callback, delay[, ...args])