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.

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

util: add options to util.promisify() #43088

Closed
2 changes: 1 addition & 1 deletion doc/api/async_context.md
Original file line number Diff line number Diff line change
Expand Up @@ -816,4 +816,4 @@ const server = createServer((req, res) => {
[`EventEmitter`]: events.md#class-eventemitter
[`Stream`]: stream.md#stream
[`Worker`]: worker_threads.md#class-worker
[`util.promisify()`]: util.md#utilpromisifyoriginal
[`util.promisify()`]: util.md#utilpromisifyoriginal-options
2 changes: 1 addition & 1 deletion doc/api/child_process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1882,6 +1882,6 @@ or [`child_process.fork()`][].
[`subprocess.stdin`]: #subprocessstdin
[`subprocess.stdio`]: #subprocessstdio
[`subprocess.stdout`]: #subprocessstdout
[`util.promisify()`]: util.md#utilpromisifyoriginal
[`util.promisify()`]: util.md#utilpromisifyoriginal-options
[synchronous counterparts]: #synchronous-process-creation
[v8.serdes]: v8.md#serialization-api
2 changes: 1 addition & 1 deletion doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -6156,7 +6156,7 @@ See the [list of SSL OP Flags][] for details.
[`sign.update()`]: #signupdatedata-inputencoding
[`stream.Writable` options]: stream.md#new-streamwritableoptions
[`stream.transform` options]: stream.md#new-streamtransformoptions
[`util.promisify()`]: util.md#utilpromisifyoriginal
[`util.promisify()`]: util.md#utilpromisifyoriginal-options
[`verify.update()`]: #verifyupdatedata-inputencoding
[`verify.verify()`]: #verifyverifyobject-signature-signatureencoding
[`x509.fingerprint256`]: #x509fingerprint256
Expand Down
2 changes: 1 addition & 1 deletion doc/api/dns.md
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,6 @@ uses. For instance, _they do not use the configuration from `/etc/hosts`_.
[`dnsPromises.setDefaultResultOrder()`]: #dnspromisessetdefaultresultorderorder
[`dnsPromises.setServers()`]: #dnspromisessetserversservers
[`socket.connect()`]: net.md#socketconnectoptions-connectlistener
[`util.promisify()`]: util.md#utilpromisifyoriginal
[`util.promisify()`]: util.md#utilpromisifyoriginal-options
[supported `getaddrinfo` flags]: #supported-getaddrinfo-flags
[worker threads]: worker_threads.md
2 changes: 1 addition & 1 deletion doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -7588,7 +7588,7 @@ the file contents.
[`fsPromises.utimes()`]: #fspromisesutimespath-atime-mtime
[`inotify(7)`]: https://man7.org/linux/man-pages/man7/inotify.7.html
[`kqueue(2)`]: https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2
[`util.promisify()`]: util.md#utilpromisifyoriginal
[`util.promisify()`]: util.md#utilpromisifyoriginal-options
[bigints]: https://tc39.github.io/proposal-bigint
[caveats]: #caveats
[chcp]: https://ss64.com/nt/chcp.html
Expand Down
33 changes: 25 additions & 8 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -1020,13 +1020,21 @@ Otherwise, returns `false`.
See [`assert.deepStrictEqual()`][] for more information about deep strict
equality.

## `util.promisify(original)`
## `util.promisify(original[, options])`

<!-- YAML
added: v8.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/43088
description: options argument was added.
-->

* `original` {Function}
* `options` {Object}
* `callbackPosition` {integer|null} **Default:** `null`
* `resolveArray` {boolean} **Default:** `false`
* `resolveObject` {Array|null} **Default:** `null`
* Returns: {Function}

Takes a function following the common error-first callback style, i.e. taking
Expand Down Expand Up @@ -1060,13 +1068,22 @@ async function callStat() {
```

If there is an `original[util.promisify.custom]` property present, `promisify`
will return its value, see [Custom promisified functions][].
will return its value, see [Custom promisified functions][]. Setting `options`
to non-null value prevents this.

If `options.resolveArray` is truthy, the promise is resolved with an array of
arguments passed to callback. Otherwise it resolves only with the first one.

If `options.resolveObject` is an array, the promise is resolved with an object
having its values as keys and callback arguments as values.

`promisify()` assumes that `original` is a function taking a callback as its
final argument in all cases. If `original` is not a function, `promisify()`
will throw an error. If `original` is a function but its last argument is not
an error-first callback, it will still be passed an error-first
callback as its last argument.
By default, `promisify()` assumes that `original` is a function taking
a callback as its final argument. If `original` is not a function,
`promisify()` will throw an error. If `original` is a function but its last
argument is not an error-first callback, it will still be passed an error-first
callback as its last argument. If `options.callbackPosition` is a number in
range from 0 to `arguments.length`, argument on this position will be used
instead of final one.

Using `promisify()` on class methods or other methods that use `this` may not
work as expected unless handled specially:
Expand Down Expand Up @@ -2681,7 +2698,7 @@ util.log('Timestamped message.');
[`tty.hasColors()`]: tty.md#writestreamhascolorscount-env
[`util.format()`]: #utilformatformat-args
[`util.inspect()`]: #utilinspectobject-options
[`util.promisify()`]: #utilpromisifyoriginal
[`util.promisify()`]: #utilpromisifyoriginal-options
[`util.types.isAnyArrayBuffer()`]: #utiltypesisanyarraybuffervalue
[`util.types.isArrayBuffer()`]: #utiltypesisarraybuffervalue
[`util.types.isDate()`]: #utiltypesisdatevalue
Expand Down
51 changes: 37 additions & 14 deletions lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const {
ArrayIsArray,
ArrayPrototypePush,
ArrayPrototypeSlice,
ArrayPrototypeSplice,
ArrayPrototypeSort,
Error,
ObjectCreate,
Expand Down Expand Up @@ -320,15 +321,27 @@ const kCustomPromisifiedSymbol = SymbolFor('nodejs.util.promisify.custom');
const kCustomPromisifyArgsSymbol = Symbol('customPromisifyArgs');

let validateFunction;
let validateInteger;

function promisify(original) {
function promisify(original, options = undefined) {
// Lazy-load to avoid a circular dependency.
if (validateFunction === undefined)
({ validateFunction } = require('internal/validators'));
if (validateFunction === undefined || validateInteger === undefined) {
({
validateFunction,
validateInteger,
} = require('internal/validators'));
}

validateFunction(original, 'original');

if (original[kCustomPromisifiedSymbol]) {
// No validateObject so .map(util.promisify) can work
const useDefaultOptions = options == null || typeof options !== 'object';
const callbackPosition = useDefaultOptions ? null : options.callbackPosition;
const allowCustom = useDefaultOptions;
const resolveArray = useDefaultOptions ? false : !!options.resolveArray;
const resolveObject = useDefaultOptions ? null : options.resolveObject;

if (allowCustom && original[kCustomPromisifiedSymbol]) {
const fn = original[kCustomPromisifiedSymbol];

validateFunction(fn, 'util.promisify.custom');
Expand All @@ -340,32 +353,42 @@ function promisify(original) {

// Names to create an object from in case the callback receives multiple
// arguments, e.g. ['bytesRead', 'buffer'] for fs.read.
const argumentNames = original[kCustomPromisifyArgsSymbol];
const argumentNames = resolveObject || original[kCustomPromisifyArgsSymbol];

function fn(...args) {
return new Promise((resolve, reject) => {
ArrayPrototypePush(args, (err, ...values) => {
const callback = (err, ...values) => {
if (err) {
return reject(err);
}
if (argumentNames !== undefined && values.length > 1) {
if (resolveArray) {
return resolve(values);
}
if (argumentNames) {
const obj = {};
for (let i = 0; i < argumentNames.length; i++)
obj[argumentNames[i]] = values[i];
resolve(obj);
} else {
resolve(values[0]);
return resolve(obj);
}
});
return resolve(values[0]);
};
if (typeof callbackPosition === 'number') {
validateInteger(callbackPosition, 'options.callbackPosition', 0, args.length);
ArrayPrototypeSplice(args, callbackPosition, 0, callback);
} else {
ArrayPrototypePush(args, callback);
}
ReflectApply(original, this, args);
});
}

ObjectSetPrototypeOf(fn, ObjectGetPrototypeOf(original));

ObjectDefineProperty(fn, kCustomPromisifiedSymbol, {
value: fn, enumerable: false, writable: false, configurable: true
});
if (allowCustom) {
ObjectDefineProperty(fn, kCustomPromisifiedSymbol, {
value: fn, enumerable: false, writable: false, configurable: true
});
}
return ObjectDefineProperties(
fn,
ObjectGetOwnPropertyDescriptors(original)
Expand Down
198 changes: 198 additions & 0 deletions test/parallel/test-util-promisify-options.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { promisify } = require('util');

// Callback gets "error" when first two args are the same
function cbError(arg1, arg2) {
return arg1 === arg2 ? { code: 'CbError' } : null;
}

const cb = {
classic(arg1, arg2, arg3, callback) {
callback(cbError(arg1, arg2), arg3);
},
smart(arg1, arg2, arg3, callback) {
// Try penultimate argument if the last is not set
callback ??= arg3;
callback(cbError(arg1, arg2), arg3);
},
returnMultiple(arg1, arg2, arg3, callback) {
callback(cbError(arg1, arg2), arg1, arg2, arg3);
},
callbackFirst(callback, arg1 = 'default1', arg2 = 'default2', arg3 = 'default3') {
callback(cbError(arg1, arg2), arg3);
},
callbackSecond(arg1, callback, arg2 = 'default2', arg3 = 'default3') {
callback(cbError(arg1, arg2), arg3);
},
rest(callback, ...args) {
callback(cbError(args[0], args[1]), args[2]);
},
returnRest(callback, ...args) {
callback(cbError(args[0], args[1]), ...args);
},
hybrid(arg1, arg2, callback, ...args) {
callback(cbError(arg1, arg2), args.length);
},
returnHybrid(arg1, arg2, callback, ...args) {
callback(cbError(arg1, arg2), ...args);
},
};

// Test that function name and length are always retained
Object.entries(cb).forEach(([fnName, fn]) => {
const promisifiedFn = promisify(fn);
assert.strictEqual(fnName, promisifiedFn.name);
assert.strictEqual(fn.name, promisifiedFn.name);
assert.strictEqual(fn.length, promisifiedFn.length);
});

// Tests for invalid numbers
{
[
NaN, -Infinity, -1, 1.5, 4, Infinity,
].forEach((invalidCallbackPosition) => {
assert.rejects(
promisify(cb.classic, { callbackPosition: invalidCallbackPosition })(1, 2, 3),
{ code: 'ERR_OUT_OF_RANGE' }
);
});
}

// Various tests
(async () => {
assert.strictEqual(
await promisify(cb.classic)(1, 2, 3),
3
);
assert.strictEqual(
await promisify(cb.classic, { callbackPosition: 3 })(1, 2, 3),
3
);
assert.deepStrictEqual(
await promisify(cb.classic, { resolveArray: true })(1, 2, 3),
[3]
);
assert.deepStrictEqual(
await promisify(cb.classic, { resolveObject: ['kFoo'] })(1, 2, 3),
{ kFoo: 3 }
);
assert.rejects(
promisify(cb.classic)(1, 1, 3),
{ code: 'CbError' }
);
assert.rejects(
promisify(cb.classic)(1, 2),
TypeError
);

assert.strictEqual(
await promisify(cb.smart)(1, 2, 3),
3
);
assert.strictEqual(
typeof await promisify(cb.smart)(1, 2),
'function'
);
assert.strictEqual(
await promisify(cb.smart, { callbackPosition: 3 })(1, 2, 3),
3
);
assert.strictEqual(
typeof await promisify(cb.smart, { callbackPosition: 2 })(1, 2),
'function'
);
assert.rejects(
promisify(cb.smart)(1, 1, 3),
{ code: 'CbError' }
);
assert.rejects(
promisify(cb.smart, { callbackPosition: 3 })(1, 2),
{ code: 'ERR_OUT_OF_RANGE' }
);
assert.rejects(
promisify(cb.smart, { callbackPosition: 2 })(1, 2, 3),
TypeError
);

assert.strictEqual(
await promisify(cb.returnMultiple, { resolveArray: false })(1, 2, 3),
1
);
assert.deepStrictEqual(
await promisify(cb.returnMultiple, { resolveArray: true })(1, 2, 3),
[1, 2, 3]
);
assert.deepStrictEqual(
await promisify(cb.returnMultiple, { resolveObject: ['kFoo', 'kBar', 'kBaz'] })(1, 2, 3),
{ kFoo: 1, kBar: 2, kBaz: 3 }
);

assert.strictEqual(
await promisify(cb.callbackFirst, { callbackPosition: 0 })(1, 2, 3),
3
);
assert.strictEqual(
await promisify(cb.callbackFirst, { callbackPosition: 0 })(1, 2),
'default3'
);

assert.strictEqual(
await promisify(cb.callbackSecond, { callbackPosition: 1 })(1, 2, 3),
3
);
assert.strictEqual(
await promisify(cb.callbackSecond, { callbackPosition: 1 })(1, 2),
'default3'
);

assert.strictEqual(
await promisify(cb.rest, { callbackPosition: 0 })(1, 2, 3, 4),
3
);
assert.rejects(
promisify(cb.rest, { callbackPosition: 0 })(1, 1, 3, 4),
{ code: 'CbError' }
);
assert.strictEqual(
await promisify(cb.rest, { callbackPosition: 0 })(1, 2),
undefined
);

assert.deepStrictEqual(
await promisify(cb.returnRest, { callbackPosition: 0, resolveArray: true })(1, 2, 3, 4, 5),
[1, 2, 3, 4, 5]
);
assert.deepStrictEqual(
await promisify(cb.returnRest, {
callbackPosition: 0,
resolveObject: ['a', 'b', 'c', 'd', 'e']
})(1, 2, 3, 4, 5),
{ a: 1, b: 2, c: 3, d: 4, e: 5 }
);
assert.deepStrictEqual(
await promisify(cb.returnRest, {
callbackPosition: 0,
resolveObject: ['a', 'b', 'c', 'd', 'e', 'f'],
})(1, 2, 3, 4, 5),
{ a: 1, b: 2, c: 3, d: 4, e: 5, f: undefined }
);

assert.strictEqual(
await promisify(cb.hybrid, { callbackPosition: 2 })(1, 2, 3, 4, 5, 6),
4
);

assert.deepStrictEqual(
await promisify(cb.returnHybrid, { callbackPosition: 2, resolveArray: true })(1, 2, 3, 4, 5, 6),
[3, 4, 5, 6]
);
assert.deepStrictEqual(
await promisify(cb.returnHybrid, {
callbackPosition: 2,
resolveObject: ['a', 'b', 'c', 'd', 'e', 'f']
})(1, 2, 3, 4, 5, 6),
{ a: 3, b: 4, c: 5, d: 6, e: undefined, f: undefined }
);
})().then(common.mustCall());