From b3b0c4347426094afe48c9e2562881e3e2b0c186 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 16 Nov 2020 17:50:30 +0100 Subject: [PATCH] domain: improve deprecation warning text for DEP0097 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because the following gives basically no actionable information on its own, neither in the error message nor in the stack trace: (node:3187) [DEP0097] DeprecationWarning: Using a domain property in MakeCallback is deprecated. Use the async_context variant of MakeCallback or the AsyncResource class instead. at emitMakeCallbackDeprecation (domain.js:123:13) at process.topLevelDomainCallback (domain.js:135:5) at process.callbackTrampoline (internal/async_hooks.js:124:14) PR-URL: https://github.com/nodejs/node/pull/36136 Reviewed-By: Colin Ihrig Reviewed-By: Gerhard Stöbich Reviewed-By: Rich Trott --- lib/domain.js | 9 ++++++--- test/addons/make-callback-domain-warning/test.js | 14 +++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/domain.js b/lib/domain.js index 1d6d75ee91527a..5c96cb43790760 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -124,12 +124,15 @@ process.setUncaughtExceptionCaptureCallback = function(fn) { let sendMakeCallbackDeprecation = false; -function emitMakeCallbackDeprecation() { +function emitMakeCallbackDeprecation({ target, method }) { if (!sendMakeCallbackDeprecation) { process.emitWarning( 'Using a domain property in MakeCallback is deprecated. Use the ' + 'async_context variant of MakeCallback or the AsyncResource class ' + - 'instead.', 'DeprecationWarning', 'DEP0097'); + 'instead. ' + + `(Triggered by calling ${method?.name ?? ''} ` + + `on ${target?.constructor?.name}.)`, + 'DeprecationWarning', 'DEP0097'); sendMakeCallbackDeprecation = true; } } @@ -137,7 +140,7 @@ function emitMakeCallbackDeprecation() { function topLevelDomainCallback(cb, ...args) { const domain = this.domain; if (exports.active && domain) - emitMakeCallbackDeprecation(); + emitMakeCallbackDeprecation({ target: this, method: cb }); if (domain) domain.enter(); diff --git a/test/addons/make-callback-domain-warning/test.js b/test/addons/make-callback-domain-warning/test.js index 0377415e1269aa..e6eaa9d337c179 100644 --- a/test/addons/make-callback-domain-warning/test.js +++ b/test/addons/make-callback-domain-warning/test.js @@ -6,7 +6,7 @@ const domain = require('domain'); const binding = require(`./build/${common.buildType}/binding`); function makeCallback(object, cb) { - binding.makeCallback(object, () => setImmediate(cb)); + binding.makeCallback(object, function someMethod() { setImmediate(cb); }); } let latestWarning = null; @@ -16,8 +16,14 @@ process.on('warning', (warning) => { const d = domain.create(); +class Resource { + constructor(domain) { + this.domain = domain; + } +} + // When domain is disabled, no warning will be emitted -makeCallback({ domain: d }, common.mustCall(() => { +makeCallback(new Resource(d), common.mustCall(() => { assert.strictEqual(latestWarning, null); d.run(common.mustCall(() => { @@ -26,7 +32,9 @@ makeCallback({ domain: d }, common.mustCall(() => { assert.strictEqual(latestWarning, null); // Warning is emitted when domain property is used and domain is enabled - makeCallback({ domain: d }, common.mustCall(() => { + makeCallback(new Resource(d), common.mustCall(() => { + assert.match(latestWarning.message, + /Triggered by calling someMethod on Resource\./); assert.strictEqual(latestWarning.name, 'DeprecationWarning'); assert.strictEqual(latestWarning.code, 'DEP0097'); }));