lib: unify bypass key validation in diagnostics_channel

Signed-off-by: Divyanshu Sharma <divyanshu88999@gmail.com>

Author: DivyanshuX9

lib/diagnostics_channel.js

@@ -49,6 +49,16 @@ function getSuppressionsStorage() {
 function withSuppressionsContext(set, fn, thisArg, args) {
   return getSuppressionsStorage().run(set, fn.bind(thisArg), ...args);
 }
+function validateBypassKey(value, name) {
+  if (value == null) {
+    throw new ERR_INVALID_ARG_TYPE(name, ['object', 'symbol'], value);
+  }
+  const type = typeof value;
+  if (type !== 'object' && type !== 'symbol') {
+    throw new ERR_INVALID_ARG_TYPE(name, ['object', 'symbol'], value);
+  }
+}
+
 // Can't delete when weakref count reaches 0 as it could increment again.
 // Only GC can be used as a valid time to clean up the channels map.
 class WeakRefMap extends SafeMap {
@@ -83,17 +93,24 @@ class WeakRefMap extends SafeMap {
 function markActive(channel) {
   // eslint-disable-next-line no-use-before-define
   ObjectSetPrototypeOf(channel, ActiveChannel.prototype);
-  channel._subscribers = [];
-  channel._stores = new SafeMap();
+  channel._subscribers = [];          // normal — plain functions
+  channel._bypassSubscribers = null;  // lazy — only if subscriberId used
+  channel._stores = new SafeMap();    // normal — plain transforms
+  channel._bypassStores = null;       // lazy — only if subscriberId used
 }
 
 function maybeMarkInactive(channel) {
   // When there are no more active subscribers or bound, restore to fast prototype.
-  if (!channel._subscribers.length && !channel._stores.size) {
+  if (!channel._subscribers.length &&
+      !channel._stores.size &&
+      !channel._bypassSubscribers?.length &&
+      !channel._bypassStores?.size) {
     // eslint-disable-next-line no-use-before-define
     ObjectSetPrototypeOf(channel, Channel.prototype);
     channel._subscribers = undefined;
     channel._stores = undefined;
+    channel._bypassSubscribers = undefined;
+    channel._bypassStores = undefined;
   }
 }
 
@@ -104,19 +121,33 @@ class RunStoresScope {
     // eslint-disable-next-line no-restricted-globals
     using stack = new DisposableStack();
 
-    // Enter stores using withScope
+    // Normal stores — exactly as before, zero extra cost
     if (activeChannel._stores) {
-      const activeKeys = getSuppressionsStorage().getStore();
       for (const entry of activeChannel._stores.entries()) {
         const store = entry[0];
-        const { transform, subscriberId = null } = entry[1];
-
-        // Skip this bound store if it opted into suppression and its key
-        // is active in the current async context.
-        if (subscriberId !== null && activeKeys?.has(subscriberId)) {
-          continue;
+        const transform = entry[1];
+        let newContext = data;
+        if (transform) {
+          try {
+            newContext = transform(data);
+          } catch (err) {
+            process.nextTick(() => {
+              triggerUncaughtException(err, false);
+            });
+            continue;
+          }
         }
+        stack.use(store.withScope(newContext));
+      }
+    }
 
+    // Bypass stores — only entered if bypass stores exist
+    if (activeChannel._bypassStores) {
+      const activeKeys = getSuppressionsStorage().getStore();
+      for (const entry of activeChannel._bypassStores.entries()) {
+        const store = entry[0];
+        const { transform, subscriberId } = entry[1];
+        if (activeKeys?.has(subscriberId)) continue;
         let newContext = data;
         if (transform) {
           try {
@@ -128,7 +159,6 @@ class RunStoresScope {
             continue;
           }
         }
-
         stack.use(store.withScope(newContext));
       }
     }
@@ -149,95 +179,156 @@ class RunStoresScope {
 class ActiveChannel {
   subscribe(subscription, options = {}) {
     validateFunction(subscription, 'subscription');
-    const subscriberId = options && options.subscriberId !== undefined ? options.subscriberId : null;
-    if (subscriberId !== null) {
-      const t = typeof subscriberId;
-      if (t === 'string' || t === 'number' || t === 'bigint' || t === 'boolean') {
-        throw new ERR_INVALID_ARG_TYPE('subscriberId', ['object', 'symbol', 'function'], subscriberId);
+    const subscriberId = options?.subscriberId;
+
+    if (subscriberId !== undefined) {
+      validateBypassKey(subscriberId, 'subscriberId');
+      // Bypass path — lazy separate array
+      if (this._bypassSubscribers === null) {
+        this._bypassSubscribers = [];
       }
+      this._bypassSubscribers = ArrayPrototypeSlice(this._bypassSubscribers);
+      ArrayPrototypePush(this._bypassSubscribers, { handler: subscription, subscriberId });
+    } else {
+      // Normal path — plain function, zero extra cost
+      this._subscribers = ArrayPrototypeSlice(this._subscribers);
+      ArrayPrototypePush(this._subscribers, subscription);
     }
 
-    const handler = subscription;
-    this._subscribers = ArrayPrototypeSlice(this._subscribers);
-    ArrayPrototypePush(this._subscribers, { handler, subscriberId });
     channels.incRef(this.name);
     if (this._index !== undefined) subscriberCounts[this._index]++;
   }
 
   unsubscribe(subscription) {
-    // Find subscriber entry by handler identity.
+    // Check normal subscribers first
     let index = -1;
-    for (let i = 0; i < (this._subscribers?.length || 0); i++) {
-      if (this._subscribers[i].handler === subscription) {
+    for (let i = 0; i < this._subscribers.length; i++) {
+      if (this._subscribers[i] === subscription) {
         index = i;
         break;
       }
     }
-    if (index === -1) return false;
 
-    const before = ArrayPrototypeSlice(this._subscribers, 0, index);
-    const after = ArrayPrototypeSlice(this._subscribers, index + 1);
-    this._subscribers = before;
-    ArrayPrototypePushApply(this._subscribers, after);
+    if (index !== -1) {
+      const before = ArrayPrototypeSlice(this._subscribers, 0, index);
+      const after = ArrayPrototypeSlice(this._subscribers, index + 1);
+      this._subscribers = before;
+      ArrayPrototypePushApply(this._subscribers, after);
+      channels.decRef(this.name);
+      if (this._index !== undefined) subscriberCounts[this._index]--;
+      maybeMarkInactive(this);
+      return true;
+    }
 
-    channels.decRef(this.name);
-    if (this._index !== undefined) subscriberCounts[this._index]--;
-    maybeMarkInactive(this);
+    // Check bypass subscribers
+    if (this._bypassSubscribers !== null) {
+      let bypassIndex = -1;
+      for (let i = 0; i < this._bypassSubscribers.length; i++) {
+        if (this._bypassSubscribers[i].handler === subscription) {
+          bypassIndex = i;
+          break;
+        }
+      }
+      if (bypassIndex !== -1) {
+        const before = ArrayPrototypeSlice(this._bypassSubscribers, 0, bypassIndex);
+        const after = ArrayPrototypeSlice(this._bypassSubscribers, bypassIndex + 1);
+        this._bypassSubscribers = before;
+        ArrayPrototypePushApply(this._bypassSubscribers, after);
+        if (this._bypassSubscribers.length === 0) {
+          this._bypassSubscribers = null;
+        }
+        channels.decRef(this.name);
+        if (this._index !== undefined) subscriberCounts[this._index]--;
+        maybeMarkInactive(this);
+        return true;
+      }
+    }
 
-    return true;
+    return false;
   }
 
   bindStore(store, transform, options = {}) {
-    const subscriberId = options && options.subscriberId !== undefined ? options.subscriberId : null;
-    if (subscriberId !== null) {
-      const t = typeof subscriberId;
-      if (t === 'string' || t === 'number' || t === 'bigint' || t === 'boolean') {
-        throw new ERR_INVALID_ARG_TYPE('subscriberId', ['object', 'symbol', 'function'], subscriberId);
-      }
-    }
+    const subscriberId = options?.subscriberId;
 
-    const replacing = this._stores.has(store);
-    if (!replacing) {
-      channels.incRef(this.name);
-      if (this._index !== undefined) subscriberCounts[this._index]++;
+    if (subscriberId !== undefined) {
+      validateBypassKey(subscriberId, 'subscriberId');
+      // Bypass path — lazy separate SafeMap
+      if (this._bypassStores === null) {
+        this._bypassStores = new SafeMap();
+      }
+      const replacing = this._bypassStores.has(store);
+      if (!replacing) {
+        channels.incRef(this.name);
+        if (this._index !== undefined) subscriberCounts[this._index]++;
+      }
+      this._bypassStores.set(store, { transform, subscriberId });
+    } else {
+      // Normal path — plain transform, zero extra cost
+      const replacing = this._stores.has(store);
+      if (!replacing) {
+        channels.incRef(this.name);
+        if (this._index !== undefined) subscriberCounts[this._index]++;
+      }
+      this._stores.set(store, transform);
     }
-    this._stores.set(store, { transform, subscriberId });
   }
 
   unbindStore(store) {
-    if (!this._stores.has(store)) {
-      return false;
+    if (this._stores.has(store)) {
+      this._stores.delete(store);
+      channels.decRef(this.name);
+      if (this._index !== undefined) subscriberCounts[this._index]--;
+      maybeMarkInactive(this);
+      return true;
     }
 
-    this._stores.delete(store);
-
-    channels.decRef(this.name);
-    if (this._index !== undefined) subscriberCounts[this._index]--;
-    maybeMarkInactive(this);
+    if (this._bypassStores?.has(store)) {
+      this._bypassStores.delete(store);
+      if (this._bypassStores.size === 0) {
+        this._bypassStores = null;
+      }
+      channels.decRef(this.name);
+      if (this._index !== undefined) subscriberCounts[this._index]--;
+      maybeMarkInactive(this);
+      return true;
+    }
 
-    return true;
+    return false;
   }
 
   get hasSubscribers() {
     return true;
   }
 
   publish(data) {
+    // Normal path — no ALS lookup, plain function call, zero overhead
     const subscribers = this._subscribers;
-    const activeKeys = getSuppressionsStorage().getStore();
-    for (let i = 0; i < (subscribers?.length || 0); i++) {
+    for (let i = 0; i < subscribers.length; i++) {
       try {
-        const { handler, subscriberId = null } = subscribers[i];
-        if (subscriberId !== null && activeKeys?.has(subscriberId)) {
-          continue;
-        }
-        handler(data, this.name);
+        subscribers[i](data, this.name);
       } catch (err) {
         process.nextTick(() => {
           triggerUncaughtException(err, false);
         });
       }
     }
+
+    // Bypass path — only entered if bypass subscribers exist
+    if (this._bypassSubscribers !== null) {
+      const activeKeys = getSuppressionsStorage().getStore();
+      const bypassSubscribers = this._bypassSubscribers;
+      for (let i = 0; i < bypassSubscribers.length; i++) {
+        try {
+          const { handler, subscriberId } = bypassSubscribers[i];
+          if (activeKeys?.has(subscriberId)) continue;
+          handler(data, this.name);
+        } catch (err) {
+          process.nextTick(() => {
+            triggerUncaughtException(err, false);
+          });
+        }
+      }
+    }
   }
 
   withStoreScope(data) {
@@ -684,13 +775,7 @@ dc_binding.linkNativeChannel((name) => channel(name));
 function suppressed(key, fn, thisArg, ...args) {
   validateFunction(fn, 'fn');
 
-  if (key === null || key === undefined) {
-    throw new ERR_INVALID_ARG_TYPE('key', ['object', 'symbol', 'function'], key);
-  }
-  const t = typeof key;
-  if (t === 'string' || t === 'number' || t === 'bigint' || t === 'boolean') {
-    throw new ERR_INVALID_ARG_TYPE('key', ['object', 'symbol', 'function'], key);
-  }
+  validateBypassKey(key, 'key');
 
   const currentSet = getSuppressionsStorage().getStore();
   const next = currentSet ? new SafeSet(currentSet) : new SafeSet();