util: add options to util.promisify()

doc/api/util.md

@@ -1024,9 +1024,17 @@ equality.
 
 <!-- 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}
+  * `resolveArray` {boolean} **Default:** `false`
+  * `resolveObject` {Array|null} **Default:** `null`
+  * `callbackPosition` {integer|null} **Default:** `null`
 * Returns: {Function}
 
 Takes a function following the common error-first callback style, i.e. taking
@@ -1062,11 +1070,19 @@ async function callStat() {
 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. 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.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.
+
+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:

lib/internal/util.js

@@ -5,6 +5,7 @@ const {
   ArrayIsArray,
   ArrayPrototypePush,
   ArrayPrototypeSlice,
+  ArrayPrototypeSplice,
   ArrayPrototypeSort,
   Error,
   ObjectCreate,
@@ -320,14 +321,29 @@ const kCustomPromisifiedSymbol = SymbolFor('nodejs.util.promisify.custom');
 const kCustomPromisifyArgsSymbol = Symbol('customPromisifyArgs');
 
 let validateFunction;
+let validateInteger;
 
-function promisify(original) {
+function promisify(original, options) {
   // 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');
 
+  // No validateObject so .map(util.promisify) can work
+  if (options == null || typeof options !== 'object') {
+    options = ObjectCreate(null);
+  }
+  const {
+    resolveArray = false,
+    resolveObject = null,
+    callbackPosition = null,
+  } = options;
+
   if (original[kCustomPromisifiedSymbol]) {
     const fn = original[kCustomPromisifiedSymbol];
 
@@ -340,23 +356,31 @@ 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);
     });
   }

test/parallel/test-util-promisify-options.js

@@ -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());