util: runtime deprecate promisify-ing a function returning a Promise

PR-URL: nodejs#49609
Fixes: nodejs#49607
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: aduh95

doc/api/deprecations.md

@@ -3387,12 +3387,15 @@ Consider using alternatives such as the [`mock`][] helper function.
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/49609
+    description: Runtime deprecation.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/49647
     description: Documentation-only deprecation.
 -->
 
-Type: Documentation-only
+Type: Runtime
 
 Calling [`util.promisify`][] on a function that returns a <Promise> will ignore
 the result of said promise, which can lead to unhandled promise rejections.

lib/internal/util.js

@@ -63,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;
@@ -409,7 +409,10 @@ 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.',
+                            'DeprecationWarning', 'DEP0174');
+      }
     });
   }
 

test/parallel/test-util-promisify.js

@@ -7,6 +7,29 @@ const vm = require('vm');
 const { promisify } = require('util');
 const { customPromisifyArgs } = require('internal/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);
 
 {