Add early-exit to TracingChannel

Author: Stephen Belanger

checks.js

@@ -8,8 +8,11 @@ function hasFullSupport() {
 }
 module.exports.hasFullSupport = hasFullSupport;
 
+// TracingChannel _did_ exist before this, but we need to replace everything
+// anyway to get early-exit support on all the trace methods.
 function hasTracingChannel() {
-  return MAJOR >= 20;
+  return MAJOR >= 22
+    || (MAJOR === 21 && MINOR >= 8);
 }
 module.exports.hasTracingChannel = hasTracingChannel;
 

dc-polyfill.js

@@ -31,4 +31,3 @@ if (checks.hasSyncUnsubscribeBug()) {
 }
 
 module.exports = dc;
-

patch-tracing-channel.js

@@ -1,10 +1,11 @@
 const {
-  ReflectApply,
+  ArrayPrototypeAt,
+  ArrayPrototypeSplice,
+  ObjectDefineProperty,
+  PromisePrototypeThen,
   PromiseReject,
   PromiseResolve,
-  PromisePrototypeThen,
-  ArrayPrototypeSplice,
-  ArrayPrototypeAt,
+  ReflectApply,
 } = require('./primordials.js');
 
 const { ERR_INVALID_ARG_TYPE } = require('./errors.js');
@@ -17,40 +18,57 @@ const traceEvents = [
   'error',
 ];
 
+function validateFunction(func, name) {
+  if (typeof func !== 'function') {
+    throw new ERR_INVALID_ARG_TYPE(name, ['function'], func);
+  }
+}
+
+function assertChannel(value, name) {
+  if (!(value instanceof Channel)) {
+    throw new ERR_INVALID_ARG_TYPE(name, ['Channel'], value);
+  }
+}
+
 module.exports = function (unpatched) {
   const { channel } = unpatched;
 
   const dc = { ...unpatched };
 
+  function tracingChannelFrom(nameOrChannels, name) {
+    if (typeof nameOrChannels === 'string') {
+      return channel(`tracing:${nameOrChannels}:${name}`);
+    }
+
+    if (typeof nameOrChannels === 'object' && nameOrChannels !== null) {
+      const channel = nameOrChannels[name];
+      assertChannel(channel, `nameOrChannels.${name}`);
+      return channel;
+    }
+
+    throw new ERR_INVALID_ARG_TYPE('nameOrChannels',
+                                   ['string', 'object', 'TracingChannel'],
+                                   nameOrChannels);
+  }
+
   class TracingChannel {
     constructor(nameOrChannels) {
-      if (typeof nameOrChannels === 'string') {
-        this.start = channel(`tracing:${nameOrChannels}:start`);
-        this.end = channel(`tracing:${nameOrChannels}:end`);
-        this.asyncStart = channel(`tracing:${nameOrChannels}:asyncStart`);
-        this.asyncEnd = channel(`tracing:${nameOrChannels}:asyncEnd`);
-        this.error = channel(`tracing:${nameOrChannels}:error`);
-      } else if (typeof nameOrChannels === 'object') {
-        const { start, end, asyncStart, asyncEnd, error } = nameOrChannels;
-
-        // assertChannel(start, 'nameOrChannels.start');
-        // assertChannel(end, 'nameOrChannels.end');
-        // assertChannel(asyncStart, 'nameOrChannels.asyncStart');
-        // assertChannel(asyncEnd, 'nameOrChannels.asyncEnd');
-        // assertChannel(error, 'nameOrChannels.error');
-
-        this.start = start;
-        this.end = end;
-        this.asyncStart = asyncStart;
-        this.asyncEnd = asyncEnd;
-        this.error = error;
-      } else {
-        throw new ERR_INVALID_ARG_TYPE('nameOrChannels',
-                                       ['string', 'object', 'Channel'],
-                                       nameOrChannels);
+      for (const eventName of traceEvents) {
+        ObjectDefineProperty(this, eventName, {
+          __proto__: null,
+          value: tracingChannelFrom(nameOrChannels, eventName),
+        });
       }
     }
 
+    get hasSubscribers() {
+      return this.start.hasSubscribers ||
+        this.end.hasSubscribers ||
+        this.asyncStart.hasSubscribers ||
+        this.asyncEnd.hasSubscribers ||
+        this.error.hasSubscribers;
+    }
+
     subscribe(handlers) {
       for (const name of traceEvents) {
         if (!handlers[name]) continue;
@@ -74,6 +92,10 @@ module.exports = function (unpatched) {
     }
 
     traceSync(fn, context = {}, thisArg, ...args) {
+      if (!this.hasSubscribers) {
+        return ReflectApply(fn, thisArg, args);
+      }
+
       const { start, end, error } = this;
 
       return start.runStores(context, () => {
@@ -92,6 +114,10 @@ module.exports = function (unpatched) {
     }
 
     tracePromise(fn, context = {}, thisArg, ...args) {
+      if (!this.hasSubscribers) {
+        return ReflectApply(fn, thisArg, args);
+      }
+
       const { start, end, asyncStart, asyncEnd, error } = this;
 
       function reject(err) {
@@ -130,6 +156,10 @@ module.exports = function (unpatched) {
     }
 
     traceCallback(fn, position = -1, context = {}, thisArg, ...args) {
+      if (!this.hasSubscribers) {
+        return ReflectApply(fn, thisArg, args);
+      }
+
       const { start, end, asyncStart, asyncEnd, error } = this;
 
       function wrappedCallback(err, res) {
@@ -153,9 +183,7 @@ module.exports = function (unpatched) {
       }
 
       const callback = ArrayPrototypeAt(args, position);
-      if (typeof callback !== 'function') {
-        throw new ERR_INVALID_ARG_TYPE('callback', ['function'], callback);
-      }
+      validateFunction(callback, 'callback');
       ArrayPrototypeSplice(args, position, 1, wrappedCallback);
 
       return start.runStores(context, () => {

test/test-diagnostics-channel-tracing-channel-async.spec.js

@@ -57,7 +57,7 @@ test('test-diagnostics-channel-tracing-channel-async', (t) => {
   try {
     channel.traceCallback(common.mustNotCall(), 0, input, thisArg, 1, 2, 3);
   } catch (err) {
-    if (MAJOR >= 20) {
+    if (MAJOR >= 22 || (MAJOR === 21 && MINOR >= 8)) {
       // By default, this error message is used for all of v20
       // However, patch-sync-unsubscribe-bug causes the error to change to the older version mentioning Array
       t.ok(/"callback" argument must be of type function/.test(err.message));

test/test-diagnostics-channel-tracing-channel-callback-early-exit.spec.js

@@ -0,0 +1,25 @@
+'use strict';
+
+const test = require('tape');
+const common = require('./common.js');
+const dc = require('../dc-polyfill.js');
+
+test('test-diagnostics-channel-tracing-channel-callback-early-exit', (t) => {
+  t.plan(1);
+  const channel = dc.tracingChannel('test');
+
+  const handlers = {
+    start: common.mustNotCall(),
+    end: common.mustNotCall(),
+    asyncStart: common.mustNotCall(),
+    asyncEnd: common.mustNotCall(),
+    error: common.mustNotCall()
+  };
+
+  // While subscribe occurs _before_ the callback executes,
+  // no async events should be published.
+  channel.traceCallback(setImmediate, 0, {}, null, common.mustCall());
+  channel.subscribe(handlers);
+
+  t.ok(true);
+});

test/test-diagnostics-channel-tracing-channel-promise-early-exit.spec.js

@@ -0,0 +1,27 @@
+'use strict';
+
+const test = require('tape');
+const common = require('./common.js');
+const dc = require('../dc-polyfill.js');
+
+test('test-diagnostics-channel-tracing-channel-promise-early-exit', (t) => {
+  t.plan(1);
+  const channel = dc.tracingChannel('test');
+
+  const handlers = {
+    start: common.mustNotCall(),
+    end: common.mustNotCall(),
+    asyncStart: common.mustNotCall(),
+    asyncEnd: common.mustNotCall(),
+    error: common.mustNotCall()
+  };
+
+  // While subscribe occurs _before_ the promise resolves,
+  // no async events should be published.
+  channel.tracePromise(() => {
+    return new Promise(setImmediate);
+  }, {});
+  channel.subscribe(handlers);
+
+  t.ok(true);
+});

test/test-diagnostics-channel-tracing-channel-sync-early-exit.spec.js

@@ -0,0 +1,26 @@
+'use strict';
+
+const test = require('tape');
+const common = require('./common.js');
+const dc = require('../dc-polyfill.js');
+
+test('test-diagnostics-channel-tracing-channel-sync-early-exit', (t) => {
+  t.plan(1);
+  const channel = dc.tracingChannel('test');
+
+  const handlers = {
+    start: common.mustNotCall(),
+    end: common.mustNotCall(),
+    asyncStart: common.mustNotCall(),
+    asyncEnd: common.mustNotCall(),
+    error: common.mustNotCall()
+  };
+
+  // While subscribe occurs _before_ the sync call ends,
+  // no end event should be published.
+  channel.traceSync(() => {
+    channel.subscribe(handlers);
+  }, {});
+
+  t.ok(true);
+});