Skip to content

Commit

Permalink
domain: improve deprecation warning text for DEP0097
Browse files Browse the repository at this point in the history
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: #36136
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
addaleax authored and nodejs-github-bot committed Nov 18, 2020
1 parent ed26808 commit b3b0c43
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 6 deletions.
9 changes: 6 additions & 3 deletions lib/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,20 +124,23 @@ 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 ?? '<anonymous>'} ` +
`on ${target?.constructor?.name}.)`,
'DeprecationWarning', 'DEP0097');
sendMakeCallbackDeprecation = true;
}
}

function topLevelDomainCallback(cb, ...args) {
const domain = this.domain;
if (exports.active && domain)
emitMakeCallbackDeprecation();
emitMakeCallbackDeprecation({ target: this, method: cb });

if (domain)
domain.enter();
Expand Down
14 changes: 11 additions & 3 deletions test/addons/make-callback-domain-warning/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(() => {
Expand All @@ -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');
}));
Expand Down

0 comments on commit b3b0c43

Please sign in to comment.