node: fix test-util-promisify.js

src/js/internal/promisify.ts

@@ -1,3 +1,7 @@
+const { validateFunction } = require("internal/validators");
+
+const ArrayPrototypePush = Array.prototype.push;
+
 const kCustomPromisifiedSymbol = Symbol.for("nodejs.util.promisify.custom");
 const kCustomPromisifyArgsSymbol = Symbol("customPromisifyArgs");
 
@@ -18,55 +22,74 @@ function defineCustomPromisifyArgs(target, args) {
   return args;
 }
 
-var promisify = function promisify(original) {
-  if (typeof original !== "function") throw new TypeError('The "original" argument must be of type Function');
-  const custom = original[kCustomPromisifiedSymbol];
-  if (custom) {
-    if (typeof custom !== "function") {
-      throw new TypeError('The "util.promisify.custom" argument must be of type Function');
-    }
-    // ensure that we don't create another promisified function wrapper
-    return defineCustomPromisify(custom, custom);
+function promisify(original) {
+  validateFunction(original, "original");
+
+  if (original[kCustomPromisifiedSymbol]) {
+    const fn = original[kCustomPromisifiedSymbol];
+
+    validateFunction(fn, "util.promisify.custom");
+
+    Object.defineProperty(fn, kCustomPromisifiedSymbol, {
+      __proto__: null,
+      value: fn,
+      enumerable: false,
+      writable: false,
+      configurable: true,
+    });
+    Object.defineProperty(fn, "name", { value: original.name });
+    return fn;
   }
 
-  const callbackArgs = original[kCustomPromisifyArgsSymbol];
-  function fn(...originalArgs) {
-    const { promise, resolve, reject } = Promise.withResolvers();
-    try {
-      original.$apply(this, [
-        ...originalArgs,
-        function (err, ...values) {
-          if (err) {
-            return reject(err);
-          }
+  // Names to create an object from in case the callback receives multiple
+  // arguments, e.g. ['bytesRead', 'buffer'] for fs.read.
+  const argumentNames = original[kCustomPromisifyArgsSymbol];
 
-          if (callbackArgs !== undefined) {
-            // if (!Array.isArray(callbackArgs)) {
-            //   throw new TypeError('The "customPromisifyArgs" argument must be of type Array');
-            // }
-            // if (callbackArgs.length !== values.length) {
-            //   throw new Error("Mismatched length in promisify callback args");
-            // }
-            const result = {};
-            for (let i = 0; i < callbackArgs.length; i++) {
-              result[callbackArgs[i]] = values[i];
-            }
-            resolve(result);
-          } else {
-            resolve(values[0]);
-          }
-        },
-      ]);
-    } catch (err) {
-      reject(err);
-    }
-
-    return promise;
+  function fn(...args) {
+    return new Promise((resolve, reject) => {
+      ArrayPrototypePush.$call(args, (err, ...values) => {
+        if (err) {
+          return reject(err);
+        }
+        if (argumentNames !== undefined && values.length > 1) {
+          const obj = {};
+          for (let i = 0; i < argumentNames.length; i++) obj[argumentNames[i]] = values[i];
+          resolve(obj);
+        } else {
+          resolve(values[0]);
+        }
+      });
+      if ($isPromise(original.$apply(this, args))) {
+        process.emitWarning(
+          "Calling promisify on a function that returns a Promise is likely a mistake.",
+          "DeprecationWarning",
+          "DEP0174",
+        );
+      }
+    });
   }
+
   Object.setPrototypeOf(fn, Object.getPrototypeOf(original));
-  defineCustomPromisify(fn, fn);
-  return Object.defineProperties(fn, Object.getOwnPropertyDescriptors(original));
-};
+
+  Object.defineProperty(fn, kCustomPromisifiedSymbol, {
+    __proto__: null,
+    value: fn,
+    enumerable: false,
+    writable: false,
+    configurable: true,
+  });
+
+  const descriptors = Object.getOwnPropertyDescriptors(original);
+  const propertiesValues = Object.values(descriptors);
+  for (let i = 0; i < propertiesValues.length; i++) {
+    // We want to use null-prototype objects to not rely on globally mutable
+    // %Object.prototype%.
+    Object.setPrototypeOf(propertiesValues[i], null);
+  }
+  Object.defineProperties(fn, descriptors);
+  Object.defineProperty(fn, "name", { value: original.name });
+  return fn;
+}
 promisify.custom = kCustomPromisifiedSymbol;
 
 // Lazily load node:timers/promises promisified functions onto the global timers.

src/js/internal/stream.ts

@@ -71,7 +71,9 @@ Stream.pipeline = pipeline;
 const { addAbortSignal } = require("internal/streams/add-abort-signal");
 Stream.addAbortSignal = addAbortSignal;
 Stream.finished = eos;
+Object.defineProperty(Stream.finished, "name", { value: "finished" });
 Stream.destroy = destroyer;
+Object.defineProperty(Stream.destroy, "name", { value: "destroy" });
 Stream.compose = compose;
 Stream.setDefaultHighWaterMark = setDefaultHighWaterMark;
 Stream.getDefaultHighWaterMark = getDefaultHighWaterMark;

test/js/node/test/parallel/test-util-promisify-custom-names.mjs

@@ -0,0 +1,40 @@
+import '../common/index.mjs';
+import assert from 'node:assert';
+import { promisify } from 'node:util';
+
+// Test that customly promisified methods in [util.promisify.custom]
+// have appropriate names
+
+import fs from 'node:fs';
+import readline from 'node:readline';
+import stream from 'node:stream';
+import timers from 'node:timers';
+
+
+assert.strictEqual(
+  promisify(fs.exists).name,
+  'exists'
+);
+
+assert.strictEqual(
+  promisify(readline.Interface.prototype.question).name,
+  'question',
+);
+
+assert.strictEqual(
+  promisify(stream.finished).name,
+  'finished'
+);
+assert.strictEqual(
+  promisify(stream.pipeline).name,
+  'pipeline'
+);
+
+assert.strictEqual(
+  promisify(timers.setImmediate).name,
+  'setImmediate'
+);
+assert.strictEqual(
+  promisify(timers.setTimeout).name,
+  'setTimeout'
+);

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

@@ -0,0 +1,215 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const vm = require('vm');
+const { promisify } = require('util');
+
+{
+  const warningHandler = common.mustNotCall();
+  process.on('warning', warningHandler);
+  function foo() {}
+  foo.constructor = (async () => {}).constructor;
+  promisify(foo);
+  process.off('warning', warningHandler);
+}
+
+common.expectWarning(
+  'DeprecationWarning',
+  'Calling promisify on a function that returns a Promise is likely a mistake.',
+  'DEP0174');
+promisify(async (callback) => { callback(); })().then(common.mustCall(() => {
+  // We must add the second `expectWarning` call in the `.then` handler, when
+  // the first warning has already been triggered.
+  common.expectWarning(
+    'DeprecationWarning',
+    'Calling promisify on a function that returns a Promise is likely a mistake.',
+    'DEP0174');
+  promisify(async () => {})().then(common.mustNotCall());
+}));
+
+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() {}
+
+  function promisifiedFn() {}
+
+  // util.promisify.custom is a shared symbol which can be accessed
+  // as `Symbol.for("nodejs.util.promisify.custom")`.
+  const kCustomPromisifiedSymbol = Symbol.for('nodejs.util.promisify.custom');
+  fn[kCustomPromisifiedSymbol] = promisifiedFn;
+
+  assert.strictEqual(kCustomPromisifiedSymbol, promisify.custom);
+  assert.strictEqual(promisify(fn), promisifiedFn);
+  assert.strictEqual(promisify(promisify(fn)), promisifiedFn);
+}
+
+{
+  function fn() {}
+  fn[promisify.custom] = 42;
+  assert.throws(
+    () => promisify(fn),
+    { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError' }
+  );
+}
+
+{
+  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.strictEqual(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);
+  }));
+}
+
+{
+  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);
+  })().then(common.mustCall());
+}
+
+{
+  const o = {};
+  const fn = promisify(function(cb) {
+
+    cb(null, this === o);
+  });
+
+  o.fn = fn;
+
+  o.fn().then(common.mustCall((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);
+  })().then(common.mustCall());
+}
+
+{
+  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);
+    }),
+  ]);
+}
+
+[undefined, null, true, 0, 'str', {}, [], Symbol()].forEach((input) => {
+  assert.throws(
+    () => promisify(input),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError',
+      message: 'The "original" argument must be of type function.' +
+               common.invalidArgTypeHelper(input)
+    });
+});