cehck the return type instead

nodejs · Sep 13, 2023 · d28fd83 · d28fd83
1 parent 954d2d0
commit d28fd83
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 18 deletions.
diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js
@@ -251,7 +251,6 @@ primordials.SymbolAsyncDispose ??= primordials.SymbolFor('nodejs.asyncDispose');
 // on the global object.
 // Refs: https://tc39.es/ecma262/#sec-%typedarray%-intrinsic-object
 [
-  { name: 'AsyncFunction', original: async function() {}.constructor },
   { name: 'TypedArray', original: Reflect.getPrototypeOf(Uint8Array) },
   { name: 'ArrayIterator', original: {
     prototype: Reflect.getPrototypeOf(Array.prototype[Symbol.iterator]()),

diff --git a/lib/internal/util.js b/lib/internal/util.js
@@ -7,7 +7,6 @@ const {
   ArrayPrototypePush,
   ArrayPrototypeSlice,
   ArrayPrototypeSort,
-  AsyncFunction,
   Error,
   FunctionPrototypeCall,
   ObjectDefineProperties,
@@ -64,7 +63,7 @@ const {
   sleep: _sleep,
   toUSVString: _toUSVString,
 } = internalBinding('util');
-const { isNativeError } = internalBinding('types');
+const { isNativeError, isPromise } = internalBinding('types');
 const { getOptionValue } = require('internal/options');
 
 const noCrypto = !process.versions.openssl;
@@ -391,16 +390,6 @@ function promisify(original) {
     });
   }
 
-  try {
-    const { constructor } = original;
-    if (constructor === AsyncFunction) {
-      process.emitWarning('Calling promisify on async functions is likely a mistake.');
-    }
-  } catch {
-    // If `constructor` was a getter that throws, ignore the error.
-  }
-
-
   // Names to create an object from in case the callback receives multiple
   // arguments, e.g. ['bytesRead', 'buffer'] for fs.read.
   const argumentNames = original[kCustomPromisifyArgsSymbol];
@@ -420,7 +409,9 @@ function promisify(original) {
           resolve(values[0]);
         }
       });
-      ReflectApply(original, this, args);
+      if (isPromise(ReflectApply(original, this, args))) {
+        process.emitWarning('Calling promisify on a function that returns a Promise is likely a mistake.');
+      }
     });
   }
 

diff --git a/test/parallel/test-util-promisify.js b/test/parallel/test-util-promisify.js
@@ -7,18 +7,21 @@ const vm = require('vm');
 const { promisify } = require('util');
 const { customPromisifyArgs } = require('internal/util');
 
-common.expectWarning('Warning', 'Calling promisify on async functions is likely a mistake.');
-promisify(async () => {});
-
 {
   const warningHandler = common.mustNotCall();
   process.on('warning', warningHandler);
   function foo() {}
-  foo.constructor = { name: 'AsyncFunction' };
+  foo.constructor = (async () => {}).constructor;
   promisify(foo);
   process.off('warning', warningHandler);
 }
 
+common.expectWarning('Warning', 'Calling promisify on a function that returns a Promise is likely a mistake.');
+promisify(async (callback) => { callback(); })().then(common.mustCall(() => {
+  common.expectWarning('Warning', 'Calling promisify on a function that returns a Promise is likely a mistake.');
+  promisify(async () => {})().then(common.mustNotCall());
+}));
+
 const stat = promisify(fs.stat);
 
 {