Skip to content

domain: improve deprecation warning text for DEP0097 #36136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
addaleax wants to merge 1 commit into from
Closed

domain: improve deprecation warning text for DEP0097 #36136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
domain: improve deprecation warning text for DEP0097 
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)
  • Loading branch information
@addaleax
addaleax committed Nov 16, 2020
commit f194b438022b49296d3fcdc234368240d4990c37
9 changes: 6 additions & 3 deletions lib/domain.js
View file
Open in desktop
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
View file
Open in desktop
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