fix(node): Defensively check inheritableHandlers in LangChain dedupe

In practice CallbackManager keeps `inheritableHandlers ⊆ handlers`
(both `addHandler` and `setHandlers` maintain the invariant), so
checking `handlers` alone suffices. But an externally-constructed
manager subclass could in theory leak the Sentry handler onto
`inheritableHandlers` without mirroring it. Checking both lists
costs nothing and pre-empts the duplicate-span class of bug Sentry's
own reviewer flagged.

Author: mdnanocom

packages/node/src/integrations/tracing/langchain/instrumentation.ts

@@ -94,10 +94,16 @@ function augmentCallbackHandlers(handlers: unknown, sentryHandler: unknown): unk
     const copied = handlers.copy() as {
       addHandler: (handler: unknown, inherit?: boolean) => void;
       handlers?: unknown[];
+      inheritableHandlers?: unknown[];
     };
-    // Avoid double-registering if the caller already added us.
-    const existing = copied.handlers ?? [];
-    if (!existing.includes(sentryHandler)) {
+    // Avoid double-registering on nested invocations. CallbackManager's own
+    // `addHandler` keeps `inheritableHandlers ⊆ handlers`, so checking
+    // `handlers` alone is normally enough — but we check both as a defensive
+    // guard against externally-constructed managers that bypass `addHandler`.
+    const alreadyRegistered =
+      (copied.handlers?.includes(sentryHandler) ?? false) ||
+      (copied.inheritableHandlers?.includes(sentryHandler) ?? false);
+    if (!alreadyRegistered) {
       copied.addHandler(sentryHandler, true);
     }
     return copied;

packages/node/test/integrations/tracing/langchain.test.ts

@@ -8,14 +8,18 @@ const sentryHandler = { name: 'SentryCallbackHandler' };
  * (`addHandler` + `copy`) for the production code to recognize this as a
  * `CallbackManager` rather than fall through to the "unknown" branch.
  */
-function makeFakeCallbackManager(existingHandlers: unknown[] = []) {
+function makeFakeCallbackManager(existingHandlers: unknown[] = [], existingInheritableHandlers?: unknown[]) {
   const manager = {
     handlers: [...existingHandlers],
-    addHandler: vi.fn(function (this: any, handler: unknown, _inherit?: boolean) {
+    inheritableHandlers: [...(existingInheritableHandlers ?? existingHandlers)],
+    addHandler: vi.fn(function (this: any, handler: unknown, inherit?: boolean) {
       this.handlers.push(handler);
+      if (inherit !== false) {
+        this.inheritableHandlers.push(handler);
+      }
     }),
     copy: vi.fn(function (this: any) {
-      return makeFakeCallbackManager(this.handlers);
+      return makeFakeCallbackManager(this.handlers, this.inheritableHandlers);
     }),
   };
   return manager;
@@ -79,6 +83,21 @@ describe('augmentCallbackHandlers', () => {
     expect(result.addHandler).not.toHaveBeenCalled();
   });
 
+  test('does not double-register when the handler lives only on inheritableHandlers', () => {
+    // Defensive: a CallbackManager subclass or externally-constructed
+    // instance might keep the Sentry handler on `inheritableHandlers`
+    // without mirroring it onto `handlers`. We must still recognize it
+    // as already-registered to avoid duplicate spans on nested calls.
+    const manager = makeFakeCallbackManager([], [sentryHandler]);
+    const result = _INTERNAL_augmentCallbackHandlers(manager, sentryHandler) as {
+      handlers: unknown[];
+      inheritableHandlers: unknown[];
+      addHandler: ReturnType<typeof vi.fn>;
+    };
+    expect(result.addHandler).not.toHaveBeenCalled();
+    expect(result.inheritableHandlers).toEqual([sentryHandler]);
+  });
+
   test('returns the value unchanged when it is neither an array nor a CallbackManager', () => {
     const opaque = { name: 'NotAManager' };
     const result = _INTERNAL_augmentCallbackHandlers(opaque, sentryHandler);