Skip to content
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

Jump to bottom

domain: improve deprecation warning text for DEP0097 #36136

Closed
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
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