Skip to content

Commit

Permalink
domain: fix unintentional deprecation warning
Browse files Browse the repository at this point in the history
646e5a4 changed the way that the domain hook callback
is called. Previously, the callback was only used in the case that
async_hooks were *not* being used (since domains already integrate
with async hooks the way they should), and the corresponding
deprecation warning also only emitted in that case.

However, that commit didn’t move that condition along when the code
was ported from C++ to JS. As a consequence, the domain hook callback
was used when it wasn’t necessary to use it, and the deprecation
warning emitted accidentally along with it.

Refs: 646e5a4#diff-9f21ce1b9d6d46fdd07b969e8a04e140L192
Refs: 646e5a4#diff-e6db408e12db906ead6ddfac3de15a6fR119
Refs: #33801 (comment)

PR-URL: #34245
Fixes: #34069
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
  • Loading branch information
addaleax authored and codebytere committed Jul 14, 2020
1 parent c7c713a commit df592c4
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 4 deletions.
6 changes: 3 additions & 3 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,18 @@ function useDomainTrampoline(fn) {
}

function callbackTrampoline(asyncId, cb, ...args) {
if (asyncId && hasHooks(kBefore))
if (asyncId !== 0 && hasHooks(kBefore))
emitBeforeNative(asyncId);

let result;
if (typeof domain_cb === 'function') {
if (asyncId === 0 && typeof domain_cb === 'function') {
ArrayPrototypeUnshift(args, cb);
result = ReflectApply(domain_cb, this, args);
} else {
result = ReflectApply(cb, this, args);
}

if (asyncId && hasHooks(kAfter))
if (asyncId !== 0 && hasHooks(kAfter))
emitAfterNative(asyncId);

return result;
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-domain-http-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
const common = require('../common');
const domain = require('domain');
const http = require('http');
const assert = require('assert');
const debug = require('util').debuglog('test');

process.on('warning', common.mustNotCall());

const objects = { foo: 'bar', baz: {}, num: 42, arr: [1, 2, 3] };
objects.baz.asdf = objects;

Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-domain-implicit-binding.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const domain = require('domain');
const fs = require('fs');
const isEnumerable = Function.call.bind(Object.prototype.propertyIsEnumerable);

process.on('warning', common.mustNotCall());

{
const d = new domain.Domain();

Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-domain-implicit-fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const common = require('../common');
const assert = require('assert');
const domain = require('domain');

process.on('warning', common.mustNotCall());

const d = new domain.Domain();

d.on('error', common.mustCall(function(er) {
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-domain-multi.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const common = require('../common');
const domain = require('domain');
const http = require('http');

process.on('warning', common.mustNotCall());

const a = domain.create();
a.enter(); // This will be our "root" domain

Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-domain-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const domain = require('domain');
const fs = require('fs');
const vm = require('vm');

process.on('warning', common.mustNotCall());

{
const d = domain.create();

Expand Down

0 comments on commit df592c4

Please sign in to comment.