From 2fb3d5153f66b5c455a3d554b80a874ebc7d9314 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 11 Sep 2023 16:44:10 +0200 Subject: [PATCH 1/3] util: runtime deprecate `promisify`-ing a function returning a `Promise` --- lib/internal/util.js | 6 ++++-- test/parallel/test-util-promisify.js | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 3586084ba7b8bd..48724374b0eec0 100644 --- a/lib/internal/util.js +++ b/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,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 index 0fc2d650ee9272..82e7218eabcc46 100644 --- a/test/parallel/test-util-promisify.js +++ b/test/parallel/test-util-promisify.js @@ -7,6 +7,21 @@ 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('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); { From f9726f9e647528b5cf2a86309baae6eff32f1d25 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 16 Sep 2023 12:02:30 +0200 Subject: [PATCH 2/3] fixup! util: runtime deprecate `promisify`-ing a function returning a `Promise` --- lib/internal/util.js | 3 ++- test/parallel/test-util-promisify.js | 12 ++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 48724374b0eec0..3d63b983a2cbed 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -410,7 +410,8 @@ function promisify(original) { } }); if (isPromise(ReflectApply(original, this, args))) { - process.emitWarning('Calling promisify on a function that returns a Promise is likely a mistake.'); + process.emitWarning('Calling promisify on a function that returns a Promise is likely a mistake.', + 'DeprecationWarning', 'DEP0174'); } }); } diff --git a/test/parallel/test-util-promisify.js b/test/parallel/test-util-promisify.js index 82e7218eabcc46..e2318a4973b972 100644 --- a/test/parallel/test-util-promisify.js +++ b/test/parallel/test-util-promisify.js @@ -16,9 +16,17 @@ const { customPromisifyArgs } = require('internal/util'); process.off('warning', warningHandler); } -common.expectWarning('Warning', 'Calling promisify on a function that returns a Promise is likely a mistake.'); +common.expectWarning( + 'DeprecationWarning', + 'Calling promisify on a function that returns a Promise is likely a mistake.', + 'DEP0174'); promisify(async (callback) => { callback(); })().then(common.mustCall(() => { - common.expectWarning('Warning', 'Calling promisify on a function that returns a Promise is likely a mistake.'); + // 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()); })); From ad10dde08a095039acce8f10118ae7383f6110fb Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 16 Sep 2023 12:05:46 +0200 Subject: [PATCH 3/3] fixup! fixup! util: runtime deprecate `promisify`-ing a function returning a `Promise` --- doc/api/deprecations.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 7cabc90b278fd8..8b9c9e59adc7b5 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3387,12 +3387,15 @@ Consider using alternatives such as the [`mock`][] helper function. -Type: Documentation-only +Type: Runtime Calling [`util.promisify`][] on a function that returns a will ignore the result of said promise, which can lead to unhandled promise rejections.