domain: fix unintentional deprecation warning

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: nodejs@646e5a4#diff-9f21ce1b9d6d46fdd07b969e8a04e140L192 Refs: nodejs@646e5a4#diff-e6db408e12db906ead6ddfac3de15a6fR119 Refs: nodejs#33801 (comment) PR-URL: nodejs#34245 Fixes: nodejs#34069 Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Andrey Pechkurov <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>
codebytere · Jul 21, 2020 · 93928ac · 93928ac
1 parent 248f9af
commit 93928ac
Show file tree

Hide file tree

Showing 6 changed files with 14 additions and 4 deletions.
diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js
@@ -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;

diff --git a/test/parallel/test-domain-http-server.js b/test/parallel/test-domain-http-server.js
@@ -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;
 

diff --git a/test/parallel/test-domain-implicit-binding.js b/test/parallel/test-domain-implicit-binding.js
@@ -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();
 

diff --git a/test/parallel/test-domain-implicit-fs.js b/test/parallel/test-domain-implicit-fs.js
@@ -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) {

diff --git a/test/parallel/test-domain-multi.js b/test/parallel/test-domain-multi.js
@@ -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
 

diff --git a/test/parallel/test-domain-promise.js b/test/parallel/test-domain-promise.js
@@ -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();