From e2b872a2896b2802e94c3e75d3146759c23412bb Mon Sep 17 00:00:00 2001 From: mdnanocom Date: Tue, 12 May 2026 20:56:09 +0200 Subject: [PATCH 1/5] fix(node): Preserve CallbackManager handlers in LangChain instrumentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `augmentCallbackHandlers` previously wrapped a `CallbackManager` instance into `[manager, sentryHandler]` whenever `options.callbacks` was a single object. LangChain downstream then treats the whole manager as one opaque handler — its inheritable children (notably LangGraph's `StreamMessagesHandler` installed by `streamMode: ['messages']`, plus the LangSmith tracer) are never unpacked, so per-token streaming events and nested tracing silently disappear. Detect `CallbackManager` via duck-typing (avoids coupling to a specific `@langchain/core` resolution) and register Sentry's handler as an inheritable child on a copy, so the manager's existing handlers continue to receive `handleLLMNewToken` and friends. Repro: LangGraph compiled graph + `ChatOpenAI` (or any provider with `bindTools(...)`), `graph.stream(..., { streamMode: ['values','messages'] })` piped through `@ai-sdk/langchain`'s `toUIMessageStream`. Without the fix, the SSE output collapses to a single aggregated `text-delta`. With the fix, every token is delivered as the model produces it. --- .../tracing/langchain/instrumentation.ts | 54 +++++++++++++++++-- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/packages/node/src/integrations/tracing/langchain/instrumentation.ts b/packages/node/src/integrations/tracing/langchain/instrumentation.ts index fb3e80b48583..6e91505df854 100644 --- a/packages/node/src/integrations/tracing/langchain/instrumentation.ts +++ b/packages/node/src/integrations/tracing/langchain/instrumentation.ts @@ -28,7 +28,42 @@ interface PatchedLangChainExports { } /** - * Augments a callback handler list with Sentry's handler if not already present + * Duck-types a LangChain `CallbackManager` instance. We can't `instanceof` + * check because `@langchain/core` may be bundled, deduped, or absent from + * our module graph; checking the shape avoids that coupling. + */ +function isCallbackManager(value: unknown): value is { + addHandler: (handler: unknown, inherit?: boolean) => void; + copy: () => unknown; + handlers?: unknown[]; +} { + if (!value || typeof value !== 'object') { + return false; + } + const candidate = value as { addHandler?: unknown; copy?: unknown }; + return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function'; +} + +/** + * Augments a callback handler list with Sentry's handler if not already present. + * + * `options.callbacks` may be one of three shapes (per LangChain's RunnableConfig): + * - `undefined` → no callbacks configured + * - `BaseCallbackHandler[]` → list of handler instances + * - `CallbackManager` → a manager that already holds (potentially + * inheritable) child handlers + * + * The `CallbackManager` case is the load-bearing one: when LangGraph sets up + * a run with `streamMode: ['messages']`, it puts a `StreamMessagesHandler` + * onto a `CallbackManager` and passes that manager through `options.callbacks`. + * If we naively wrap the manager into `[manager, sentryHandler]`, LangChain + * downstream treats the whole manager as a single opaque handler — its + * inheritable children (`StreamMessagesHandler`, the tracer, etc.) are never + * unpacked, and per-token streaming events silently disappear. + * + * Instead, when we receive a `CallbackManager`, we copy it (so we don't + * mutate the caller's manager across invocations) and register Sentry's + * handler as an inheritable child via `.addHandler()`. */ function augmentCallbackHandlers(handlers: unknown, sentryHandler: unknown): unknown { // Handle null/undefined - return array with just our handler @@ -46,9 +81,20 @@ function augmentCallbackHandlers(handlers: unknown, sentryHandler: unknown): unk return [...handlers, sentryHandler]; } - // If it's a single handler object, convert to array - if (typeof handlers === 'object') { - return [handlers, sentryHandler]; + // CallbackManager: register our handler as an inheritable child on a copy + // so we preserve any handlers already attached (notably LangGraph's + // StreamMessagesHandler used by `streamMode: ['messages']`). + if (isCallbackManager(handlers)) { + const copied = handlers.copy() as { + addHandler: (handler: unknown, inherit?: boolean) => void; + handlers?: unknown[]; + }; + // Avoid double-registering if the caller already added us. + const existing = copied.handlers ?? []; + if (!existing.includes(sentryHandler)) { + copied.addHandler(sentryHandler, true); + } + return copied; } // Unknown type - return original From 4c74bfa5fa2cdf8ac42601d759932975157d21eb Mon Sep 17 00:00:00 2001 From: mdnanocom Date: Tue, 12 May 2026 21:00:27 +0200 Subject: [PATCH 2/5] test(node): Add unit tests + changelog for LangChain callback fix --- CHANGELOG.md | 1 + .../tracing/langchain/instrumentation.ts | 6 ++ .../integrations/tracing/langchain.test.ts | 87 +++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 packages/node/test/integrations/tracing/langchain.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a0b161d35104..884eb0bed1eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- fix(node): Preserve `CallbackManager` children when augmenting LangChain callbacks - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott ## 10.53.1 diff --git a/packages/node/src/integrations/tracing/langchain/instrumentation.ts b/packages/node/src/integrations/tracing/langchain/instrumentation.ts index 6e91505df854..22f4665b3a27 100644 --- a/packages/node/src/integrations/tracing/langchain/instrumentation.ts +++ b/packages/node/src/integrations/tracing/langchain/instrumentation.ts @@ -44,6 +44,12 @@ function isCallbackManager(value: unknown): value is { return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function'; } +/** + * Exported for testing. + * @internal + */ +export const _INTERNAL_augmentCallbackHandlers = augmentCallbackHandlers; + /** * Augments a callback handler list with Sentry's handler if not already present. * diff --git a/packages/node/test/integrations/tracing/langchain.test.ts b/packages/node/test/integrations/tracing/langchain.test.ts new file mode 100644 index 000000000000..cca557328ffd --- /dev/null +++ b/packages/node/test/integrations/tracing/langchain.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, test, vi } from 'vitest'; +import { _INTERNAL_augmentCallbackHandlers } from '../../../src/integrations/tracing/langchain/instrumentation'; + +const sentryHandler = { name: 'SentryCallbackHandler' }; + +/** + * Minimal `CallbackManager` stand-in. We only need the duck-typed shape + * (`addHandler` + `copy`) for the production code to recognize this as a + * `CallbackManager` rather than fall through to the "unknown" branch. + */ +function makeFakeCallbackManager(existingHandlers: unknown[] = []) { + const manager = { + handlers: [...existingHandlers], + addHandler: vi.fn(function (this: any, handler: unknown, _inherit?: boolean) { + this.handlers.push(handler); + }), + copy: vi.fn(function (this: any) { + return makeFakeCallbackManager(this.handlers); + }), + }; + return manager; +} + +describe('augmentCallbackHandlers', () => { + test('wraps Sentry handler in an array when no callbacks are configured', () => { + const result = _INTERNAL_augmentCallbackHandlers(undefined, sentryHandler); + expect(result).toEqual([sentryHandler]); + }); + + test('appends Sentry handler when callbacks is already an array', () => { + const other = { name: 'OtherHandler' }; + const result = _INTERNAL_augmentCallbackHandlers([other], sentryHandler); + expect(result).toEqual([other, sentryHandler]); + }); + + test('is idempotent when Sentry handler is already in the array', () => { + const result = _INTERNAL_augmentCallbackHandlers([sentryHandler], sentryHandler); + expect(result).toEqual([sentryHandler]); + }); + + test('preserves inheritable handlers when callbacks is a CallbackManager', () => { + // Reproduces the LangGraph `streamMode: ['messages']` setup: a + // CallbackManager carrying a StreamMessagesHandler is passed via + // options.callbacks. Without this fix, the manager would be wrapped as + // `[manager, sentryHandler]`, dropping all its inheritable children. + const streamMessagesHandler = { + name: 'StreamMessagesHandler', + lc_prefer_streaming: true, + }; + const manager = makeFakeCallbackManager([streamMessagesHandler]); + + const result = _INTERNAL_augmentCallbackHandlers(manager, sentryHandler) as { + handlers: unknown[]; + }; + + // The result is a manager (object), not a wrapping array. + expect(Array.isArray(result)).toBe(false); + // The original child handler is still there alongside Sentry's. + expect(result.handlers).toEqual([streamMessagesHandler, sentryHandler]); + }); + + test('copies the manager rather than mutating the caller-supplied one', () => { + // If we mutated the original manager, repeated invocations would + // accumulate Sentry handlers (and tracers from prior runs would leak + // into subsequent unrelated runs). + const manager = makeFakeCallbackManager([]); + _INTERNAL_augmentCallbackHandlers(manager, sentryHandler); + expect(manager.copy).toHaveBeenCalledTimes(1); + expect(manager.handlers).toEqual([]); + }); + + test('does not double-register Sentry handler when copy already contains it', () => { + const manager = makeFakeCallbackManager([sentryHandler]); + const result = _INTERNAL_augmentCallbackHandlers(manager, sentryHandler) as { + handlers: unknown[]; + addHandler: ReturnType; + }; + expect(result.handlers).toEqual([sentryHandler]); + expect(result.addHandler).not.toHaveBeenCalled(); + }); + + test('returns the value unchanged when it is neither an array nor a CallbackManager', () => { + const opaque = { name: 'NotAManager' }; + const result = _INTERNAL_augmentCallbackHandlers(opaque, sentryHandler); + expect(result).toBe(opaque); + }); +}); From b33352b6bffbe1ddd11290185afab1c802c0e5b2 Mon Sep 17 00:00:00 2001 From: mdnanocom Date: Tue, 12 May 2026 21:50:48 +0200 Subject: [PATCH 3/5] fix(node): Defensively check inheritableHandlers in LangChain dedupe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../tracing/langchain/instrumentation.ts | 12 ++++++--- .../integrations/tracing/langchain.test.ts | 25 ++++++++++++++++--- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/packages/node/src/integrations/tracing/langchain/instrumentation.ts b/packages/node/src/integrations/tracing/langchain/instrumentation.ts index 22f4665b3a27..34bfdf008534 100644 --- a/packages/node/src/integrations/tracing/langchain/instrumentation.ts +++ b/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; diff --git a/packages/node/test/integrations/tracing/langchain.test.ts b/packages/node/test/integrations/tracing/langchain.test.ts index cca557328ffd..4d22b4c1afdb 100644 --- a/packages/node/test/integrations/tracing/langchain.test.ts +++ b/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; + }; + 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); From 9e2939dec6e95c87a2af30761061a69941c5faf3 Mon Sep 17 00:00:00 2001 From: mdnanocom Date: Wed, 13 May 2026 00:07:23 +0200 Subject: [PATCH 4/5] refactor(core): Consolidate LangChain callback merging into mergeSentryCallback The langchain instrumentation's `augmentCallbackHandlers` and the langgraph instrumentation's `mergeSentryCallback` solved the same problem two different ways. The langgraph helper had three latent bugs that this PR's fix already covered for langchain: - mutated the caller's CallbackManager (handlers accumulate across runs) - called `addHandler(handler)` without `inherit=true`, so the handler never propagated to child managers created by `getChild` - deduped only against `manager.handlers`, not `inheritableHandlers` Lift the fixed implementation up to `@sentry/core` so both instrumentations share it. The langgraph integration silently picks up the streaming fix as a bonus. Drops the duplicate `augmentCallback` helper and its test; behavior is covered by the expanded `mergeSentryCallback` suite (14 cases). --- packages/core/src/shared-exports.ts | 1 + packages/core/src/tracing/langgraph/utils.ts | 44 +++++++- .../test/lib/utils/langgraph-utils.test.ts | 101 +++++++++++++++-- .../tracing/langchain/instrumentation.ts | 91 +-------------- .../integrations/tracing/langchain.test.ts | 106 ------------------ 5 files changed, 131 insertions(+), 212 deletions(-) delete mode 100644 packages/node/test/integrations/tracing/langchain.test.ts diff --git a/packages/core/src/shared-exports.ts b/packages/core/src/shared-exports.ts index dcc8c268f509..6edcb3224d81 100644 --- a/packages/core/src/shared-exports.ts +++ b/packages/core/src/shared-exports.ts @@ -173,6 +173,7 @@ export { createLangChainCallbackHandler, instrumentLangChainEmbeddings } from '. export { LANGCHAIN_INTEGRATION_NAME } from './tracing/langchain/constants'; export type { LangChainOptions, LangChainIntegration } from './tracing/langchain/types'; export { instrumentStateGraphCompile, instrumentCreateReactAgent, instrumentLangGraph } from './tracing/langgraph'; +export { mergeSentryCallback } from './tracing/langgraph/utils'; export { LANGGRAPH_INTEGRATION_NAME } from './tracing/langgraph/constants'; export type { LangGraphOptions, LangGraphIntegration, CompiledGraph } from './tracing/langgraph/types'; export type { OpenAiClient, OpenAiOptions, InstrumentedMethod } from './tracing/openai/types'; diff --git a/packages/core/src/tracing/langgraph/utils.ts b/packages/core/src/tracing/langgraph/utils.ts index 8770cbbd629b..8610a82e09e7 100644 --- a/packages/core/src/tracing/langgraph/utils.ts +++ b/packages/core/src/tracing/langgraph/utils.ts @@ -335,7 +335,28 @@ export function setResponseAttributes(span: Span, inputMessages: LangChainMessag } } -/** Merge `sentryHandler` into a langchain `callbacks` value (`BaseCallbackHandler[]` or `BaseCallbackManager`). */ +/** Duck-types a LangChain `CallbackManager` — `instanceof` is unreliable when `@langchain/core` is bundled or deduped. */ +function isCallbackManager(value: unknown): value is { + addHandler: (handler: unknown, inherit?: boolean) => void; + copy: () => unknown; + handlers?: unknown[]; + inheritableHandlers?: unknown[]; +} { + if (!value || typeof value !== 'object') { + return false; + } + const candidate = value as { addHandler?: unknown; copy?: unknown }; + return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function'; +} + +/** + * Merge `sentryHandler` into a langchain `callbacks` value (undefined, `BaseCallbackHandler[]`, or `BaseCallbackManager`). + * + * Wrapping a `CallbackManager` into `[manager, sentryHandler]` would make LangChain treat the whole manager + * as one opaque handler and drop its inheritable children — notably LangGraph's `StreamMessagesHandler`, + * which silently breaks per-token streaming. We register on a `.copy()` (so caller state stays clean across + * runs) and add ourselves as inheritable so `getChild()` propagates us into nested calls. + */ export function mergeSentryCallback(existing: unknown, sentryHandler: unknown): unknown { if (!existing) { return [sentryHandler]; @@ -348,12 +369,23 @@ export function mergeSentryCallback(existing: unknown, sentryHandler: unknown): return [...existing, sentryHandler]; } - const manager = existing as { addHandler?: (h: unknown) => void; handlers?: unknown[] }; - if (typeof manager.addHandler === 'function') { - const alreadyAdded = Array.isArray(manager.handlers) && manager.handlers.includes(sentryHandler); - if (!alreadyAdded) { - manager.addHandler(sentryHandler); + if (isCallbackManager(existing)) { + const copied = existing.copy() as { + addHandler: (handler: unknown, inherit?: boolean) => void; + handlers?: unknown[]; + inheritableHandlers?: unknown[]; + }; + // CallbackManager keeps `inheritableHandlers ⊆ handlers` (both + // `addHandler` and `setHandlers` maintain the invariant), so checking + // `handlers` alone normally suffices — 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; } return existing; diff --git a/packages/core/test/lib/utils/langgraph-utils.test.ts b/packages/core/test/lib/utils/langgraph-utils.test.ts index 829317518622..0f70d1e21051 100644 --- a/packages/core/test/lib/utils/langgraph-utils.test.ts +++ b/packages/core/test/lib/utils/langgraph-utils.test.ts @@ -48,6 +48,30 @@ describe('extractAgentNameFromParams', () => { describe('mergeSentryCallback', () => { const sentryHandler = { _sentry: true }; + /** + * Minimal `CallbackManager` stand-in. Mirrors `@langchain/core`'s real + * semantics: `addHandler(_, inherit)` pushes to both `handlers` and + * `inheritableHandlers` when `inherit !== false`, and `copy()` returns + * a fresh manager carrying the same handlers — so we don't accidentally + * test against a degenerate shape that bypasses `addHandler`. + */ + function makeFakeCallbackManager(existingHandlers: unknown[] = [], existingInheritableHandlers?: unknown[]) { + const manager = { + handlers: [...existingHandlers], + 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, this.inheritableHandlers); + }), + }; + return manager; + } + it('returns a fresh array when no existing callbacks are present', () => { expect(mergeSentryCallback(undefined, sentryHandler)).toStrictEqual([sentryHandler]); expect(mergeSentryCallback(null, sentryHandler)).toStrictEqual([sentryHandler]); @@ -65,19 +89,74 @@ describe('mergeSentryCallback', () => { expect(mergeSentryCallback(existing, sentryHandler)).toBe(existing); }); - it('calls addHandler on a CallbackManager-like object', () => { - const addHandler = vi.fn(); - const manager = { addHandler, handlers: [] as unknown[] }; - const result = mergeSentryCallback(manager, sentryHandler); - expect(result).toBe(manager); - expect(addHandler).toHaveBeenCalledWith(sentryHandler); - expect(addHandler).toHaveBeenCalledTimes(1); + it('preserves inheritable handlers when callbacks is a CallbackManager', () => { + // Reproduces the LangGraph `streamMode: ['messages']` setup: a + // CallbackManager carrying a StreamMessagesHandler is passed via + // options.callbacks. Wrapping it as `[manager, sentryHandler]` would + // drop the manager's inheritable children — instead we register + // Sentry on a copy and keep the existing handler chain intact. + const streamMessagesHandler = { + name: 'StreamMessagesHandler', + lc_prefer_streaming: true, + }; + const manager = makeFakeCallbackManager([streamMessagesHandler]); + const result = mergeSentryCallback(manager, sentryHandler) as { + handlers: unknown[]; + }; + expect(Array.isArray(result)).toBe(false); + expect(result.handlers).toEqual([streamMessagesHandler, sentryHandler]); }); - it('does not re-add when the manager already has the sentry handler', () => { - const addHandler = vi.fn(); - const manager = { addHandler, handlers: [sentryHandler] }; + it('copies the manager rather than mutating the caller-supplied one', () => { + // If we mutated the original, repeated invocations would accumulate + // Sentry handlers (and tracers from prior runs would leak across runs). + const manager = makeFakeCallbackManager([]); mergeSentryCallback(manager, sentryHandler); - expect(addHandler).not.toHaveBeenCalled(); + expect(manager.copy).toHaveBeenCalledTimes(1); + expect(manager.handlers).toEqual([]); + }); + + it('registers the sentry handler as inheritable so child managers see it', () => { + // LangChain's CallbackManager.getChild creates child managers via + // `setHandlers(this.inheritableHandlers)`. If we add ourselves without + // `inherit=true`, nested LLM calls inside an agent never receive the + // Sentry handler. + const manager = makeFakeCallbackManager([]); + const result = mergeSentryCallback(manager, sentryHandler) as { + addHandler: ReturnType; + handlers: unknown[]; + inheritableHandlers: unknown[]; + }; + expect(result.addHandler).toHaveBeenCalledWith(sentryHandler, true); + expect(result.inheritableHandlers).toEqual([sentryHandler]); + }); + + it('does not double-register when the copied manager already contains the handler', () => { + const manager = makeFakeCallbackManager([sentryHandler]); + const result = mergeSentryCallback(manager, sentryHandler) as { + handlers: unknown[]; + addHandler: ReturnType; + }; + expect(result.handlers).toEqual([sentryHandler]); + expect(result.addHandler).not.toHaveBeenCalled(); + }); + + it('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 = mergeSentryCallback(manager, sentryHandler) as { + addHandler: ReturnType; + inheritableHandlers: unknown[]; + }; + expect(result.addHandler).not.toHaveBeenCalled(); + expect(result.inheritableHandlers).toEqual([sentryHandler]); + }); + + it('returns the value unchanged when it is neither an array nor a CallbackManager', () => { + const opaque = { name: 'NotAManager' }; + expect(mergeSentryCallback(opaque, sentryHandler)).toBe(opaque); }); }); diff --git a/packages/node/src/integrations/tracing/langchain/instrumentation.ts b/packages/node/src/integrations/tracing/langchain/instrumentation.ts index 34bfdf008534..8c4c594a1375 100644 --- a/packages/node/src/integrations/tracing/langchain/instrumentation.ts +++ b/packages/node/src/integrations/tracing/langchain/instrumentation.ts @@ -12,6 +12,7 @@ import { createLangChainCallbackHandler, GOOGLE_GENAI_INTEGRATION_NAME, instrumentLangChainEmbeddings, + mergeSentryCallback, OPENAI_INTEGRATION_NAME, SDK_VERSION, } from '@sentry/core'; @@ -27,92 +28,6 @@ interface PatchedLangChainExports { [key: string]: unknown; } -/** - * Duck-types a LangChain `CallbackManager` instance. We can't `instanceof` - * check because `@langchain/core` may be bundled, deduped, or absent from - * our module graph; checking the shape avoids that coupling. - */ -function isCallbackManager(value: unknown): value is { - addHandler: (handler: unknown, inherit?: boolean) => void; - copy: () => unknown; - handlers?: unknown[]; -} { - if (!value || typeof value !== 'object') { - return false; - } - const candidate = value as { addHandler?: unknown; copy?: unknown }; - return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function'; -} - -/** - * Exported for testing. - * @internal - */ -export const _INTERNAL_augmentCallbackHandlers = augmentCallbackHandlers; - -/** - * Augments a callback handler list with Sentry's handler if not already present. - * - * `options.callbacks` may be one of three shapes (per LangChain's RunnableConfig): - * - `undefined` → no callbacks configured - * - `BaseCallbackHandler[]` → list of handler instances - * - `CallbackManager` → a manager that already holds (potentially - * inheritable) child handlers - * - * The `CallbackManager` case is the load-bearing one: when LangGraph sets up - * a run with `streamMode: ['messages']`, it puts a `StreamMessagesHandler` - * onto a `CallbackManager` and passes that manager through `options.callbacks`. - * If we naively wrap the manager into `[manager, sentryHandler]`, LangChain - * downstream treats the whole manager as a single opaque handler — its - * inheritable children (`StreamMessagesHandler`, the tracer, etc.) are never - * unpacked, and per-token streaming events silently disappear. - * - * Instead, when we receive a `CallbackManager`, we copy it (so we don't - * mutate the caller's manager across invocations) and register Sentry's - * handler as an inheritable child via `.addHandler()`. - */ -function augmentCallbackHandlers(handlers: unknown, sentryHandler: unknown): unknown { - // Handle null/undefined - return array with just our handler - if (!handlers) { - return [sentryHandler]; - } - - // If handlers is already an array - if (Array.isArray(handlers)) { - // Check if our handler is already in the list - if (handlers.includes(sentryHandler)) { - return handlers; - } - // Add our handler to the list - return [...handlers, sentryHandler]; - } - - // CallbackManager: register our handler as an inheritable child on a copy - // so we preserve any handlers already attached (notably LangGraph's - // StreamMessagesHandler used by `streamMode: ['messages']`). - if (isCallbackManager(handlers)) { - const copied = handlers.copy() as { - addHandler: (handler: unknown, inherit?: boolean) => void; - handlers?: unknown[]; - inheritableHandlers?: unknown[]; - }; - // 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; - } - - // Unknown type - return original - return handlers; -} - /** * Wraps Runnable methods (invoke, stream, batch) to inject Sentry callbacks at request time * Uses a Proxy to intercept method calls and augment the options.callbacks @@ -140,9 +55,7 @@ function wrapRunnableMethod( } // Inject our callback handler into options.callbacks (request time callbacks) - const existingCallbacks = options.callbacks; - const augmentedCallbacks = augmentCallbackHandlers(existingCallbacks, sentryHandler); - options.callbacks = augmentedCallbacks; + options.callbacks = mergeSentryCallback(options.callbacks, sentryHandler); // Call original method with augmented options return Reflect.apply(target, thisArg, args); diff --git a/packages/node/test/integrations/tracing/langchain.test.ts b/packages/node/test/integrations/tracing/langchain.test.ts deleted file mode 100644 index 4d22b4c1afdb..000000000000 --- a/packages/node/test/integrations/tracing/langchain.test.ts +++ /dev/null @@ -1,106 +0,0 @@ -import { describe, expect, test, vi } from 'vitest'; -import { _INTERNAL_augmentCallbackHandlers } from '../../../src/integrations/tracing/langchain/instrumentation'; - -const sentryHandler = { name: 'SentryCallbackHandler' }; - -/** - * Minimal `CallbackManager` stand-in. We only need the duck-typed shape - * (`addHandler` + `copy`) for the production code to recognize this as a - * `CallbackManager` rather than fall through to the "unknown" branch. - */ -function makeFakeCallbackManager(existingHandlers: unknown[] = [], existingInheritableHandlers?: unknown[]) { - const manager = { - handlers: [...existingHandlers], - 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, this.inheritableHandlers); - }), - }; - return manager; -} - -describe('augmentCallbackHandlers', () => { - test('wraps Sentry handler in an array when no callbacks are configured', () => { - const result = _INTERNAL_augmentCallbackHandlers(undefined, sentryHandler); - expect(result).toEqual([sentryHandler]); - }); - - test('appends Sentry handler when callbacks is already an array', () => { - const other = { name: 'OtherHandler' }; - const result = _INTERNAL_augmentCallbackHandlers([other], sentryHandler); - expect(result).toEqual([other, sentryHandler]); - }); - - test('is idempotent when Sentry handler is already in the array', () => { - const result = _INTERNAL_augmentCallbackHandlers([sentryHandler], sentryHandler); - expect(result).toEqual([sentryHandler]); - }); - - test('preserves inheritable handlers when callbacks is a CallbackManager', () => { - // Reproduces the LangGraph `streamMode: ['messages']` setup: a - // CallbackManager carrying a StreamMessagesHandler is passed via - // options.callbacks. Without this fix, the manager would be wrapped as - // `[manager, sentryHandler]`, dropping all its inheritable children. - const streamMessagesHandler = { - name: 'StreamMessagesHandler', - lc_prefer_streaming: true, - }; - const manager = makeFakeCallbackManager([streamMessagesHandler]); - - const result = _INTERNAL_augmentCallbackHandlers(manager, sentryHandler) as { - handlers: unknown[]; - }; - - // The result is a manager (object), not a wrapping array. - expect(Array.isArray(result)).toBe(false); - // The original child handler is still there alongside Sentry's. - expect(result.handlers).toEqual([streamMessagesHandler, sentryHandler]); - }); - - test('copies the manager rather than mutating the caller-supplied one', () => { - // If we mutated the original manager, repeated invocations would - // accumulate Sentry handlers (and tracers from prior runs would leak - // into subsequent unrelated runs). - const manager = makeFakeCallbackManager([]); - _INTERNAL_augmentCallbackHandlers(manager, sentryHandler); - expect(manager.copy).toHaveBeenCalledTimes(1); - expect(manager.handlers).toEqual([]); - }); - - test('does not double-register Sentry handler when copy already contains it', () => { - const manager = makeFakeCallbackManager([sentryHandler]); - const result = _INTERNAL_augmentCallbackHandlers(manager, sentryHandler) as { - handlers: unknown[]; - addHandler: ReturnType; - }; - expect(result.handlers).toEqual([sentryHandler]); - 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; - }; - 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); - expect(result).toBe(opaque); - }); -}); From 7dcdd93dda20522086c5d45053b63b49cfcbc9ef Mon Sep 17 00:00:00 2001 From: mdnanocom Date: Wed, 13 May 2026 13:51:48 +0200 Subject: [PATCH 5/5] review: Tighten CallbackManager detection + drop changelog entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on PR #20849: - `isCallbackManager`: in addition to duck-typing `addHandler`/`copy`, walk the prototype chain and require `constructor.name === 'CallbackManager'` (mirroring `packages/cloudflare/src/utils/isCloudflareClass.ts`). Filters out unrelated objects that coincidentally expose the same shape; the prototype walk keeps subclasses working. - Drop the changelog entry — release process generates the changelog manually. Two new test cases lock the behavior in: rejects lookalike duck-typed objects, and recognizes subclasses via the prototype walk (16/16). --- CHANGELOG.md | 1 - packages/core/src/tracing/langgraph/utils.ts | 31 ++++--- .../test/lib/utils/langgraph-utils.test.ts | 89 +++++++++++-------- 3 files changed, 68 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 884eb0bed1eb..a0b161d35104 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,6 @@ ## Unreleased -- fix(node): Preserve `CallbackManager` children when augmenting LangChain callbacks - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott ## 10.53.1 diff --git a/packages/core/src/tracing/langgraph/utils.ts b/packages/core/src/tracing/langgraph/utils.ts index 8610a82e09e7..0fadded3c82d 100644 --- a/packages/core/src/tracing/langgraph/utils.ts +++ b/packages/core/src/tracing/langgraph/utils.ts @@ -335,18 +335,31 @@ export function setResponseAttributes(span: Span, inputMessages: LangChainMessag } } -/** Duck-types a LangChain `CallbackManager` — `instanceof` is unreliable when `@langchain/core` is bundled or deduped. */ +/** + * Detects a LangChain `CallbackManager` (or subclass) without depending on `instanceof`. + * `@langchain/core` is frequently bundled or deduped, so the imported constructor doesn't + * necessarily match the one at the user's call site. We walk the prototype chain looking + * for the class name, then confirm the shape — the constructor-name check rules out + * unrelated objects that happen to expose `addHandler`/`copy`. + */ function isCallbackManager(value: unknown): value is { addHandler: (handler: unknown, inherit?: boolean) => void; copy: () => unknown; handlers?: unknown[]; - inheritableHandlers?: unknown[]; } { if (!value || typeof value !== 'object') { return false; } - const candidate = value as { addHandler?: unknown; copy?: unknown }; - return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function'; + + let proto: object | null = Object.getPrototypeOf(value); + while (proto) { + if ((proto as { constructor?: { name?: string } }).constructor?.name === 'CallbackManager') { + const candidate = value as { addHandler?: unknown; copy?: unknown }; + return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function'; + } + proto = Object.getPrototypeOf(proto); + } + return false; } /** @@ -373,16 +386,8 @@ export function mergeSentryCallback(existing: unknown, sentryHandler: unknown): const copied = existing.copy() as { addHandler: (handler: unknown, inherit?: boolean) => void; handlers?: unknown[]; - inheritableHandlers?: unknown[]; }; - // CallbackManager keeps `inheritableHandlers ⊆ handlers` (both - // `addHandler` and `setHandlers` maintain the invariant), so checking - // `handlers` alone normally suffices — 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) { + if (!copied.handlers?.includes(sentryHandler)) { copied.addHandler(sentryHandler, true); } return copied; diff --git a/packages/core/test/lib/utils/langgraph-utils.test.ts b/packages/core/test/lib/utils/langgraph-utils.test.ts index 0f70d1e21051..f666a4d6693c 100644 --- a/packages/core/test/lib/utils/langgraph-utils.test.ts +++ b/packages/core/test/lib/utils/langgraph-utils.test.ts @@ -56,20 +56,24 @@ describe('mergeSentryCallback', () => { * test against a degenerate shape that bypasses `addHandler`. */ function makeFakeCallbackManager(existingHandlers: unknown[] = [], existingInheritableHandlers?: unknown[]) { - const manager = { - handlers: [...existingHandlers], - inheritableHandlers: [...(existingInheritableHandlers ?? existingHandlers)], - addHandler: vi.fn(function (this: any, handler: unknown, inherit?: boolean) { + // Use a class so `Object.getPrototypeOf(instance).constructor.name === 'CallbackManager'`, + // which is how the production detector identifies a real LangChain CallbackManager. + class CallbackManager { + public handlers: unknown[]; + public inheritableHandlers: unknown[]; + public addHandler = vi.fn((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, this.inheritableHandlers); - }), - }; - return manager; + }); + public copy = vi.fn(() => makeFakeCallbackManager(this.handlers, this.inheritableHandlers)); + constructor(initialHandlers: unknown[], initialInheritableHandlers: unknown[]) { + this.handlers = [...initialHandlers]; + this.inheritableHandlers = [...initialInheritableHandlers]; + } + } + return new CallbackManager(existingHandlers, existingInheritableHandlers ?? existingHandlers); } it('returns a fresh array when no existing callbacks are present', () => { @@ -107,26 +111,17 @@ describe('mergeSentryCallback', () => { expect(result.handlers).toEqual([streamMessagesHandler, sentryHandler]); }); - it('copies the manager rather than mutating the caller-supplied one', () => { - // If we mutated the original, repeated invocations would accumulate - // Sentry handlers (and tracers from prior runs would leak across runs). - const manager = makeFakeCallbackManager([]); - mergeSentryCallback(manager, sentryHandler); - expect(manager.copy).toHaveBeenCalledTimes(1); - expect(manager.handlers).toEqual([]); - }); - - it('registers the sentry handler as inheritable so child managers see it', () => { - // LangChain's CallbackManager.getChild creates child managers via - // `setHandlers(this.inheritableHandlers)`. If we add ourselves without - // `inherit=true`, nested LLM calls inside an agent never receive the - // Sentry handler. + it('copies the manager and registers Sentry as an inheritable handler', () => { + // Two adjacent contracts: we operate on a copy (so repeat invocations + // don't accumulate handlers on the caller), and we pass `inherit=true` + // so LangChain's `getChild()` propagates Sentry into nested calls. const manager = makeFakeCallbackManager([]); const result = mergeSentryCallback(manager, sentryHandler) as { addHandler: ReturnType; - handlers: unknown[]; inheritableHandlers: unknown[]; }; + expect(manager.copy).toHaveBeenCalledTimes(1); + expect(manager.handlers).toEqual([]); expect(result.addHandler).toHaveBeenCalledWith(sentryHandler, true); expect(result.inheritableHandlers).toEqual([sentryHandler]); }); @@ -141,22 +136,38 @@ describe('mergeSentryCallback', () => { expect(result.addHandler).not.toHaveBeenCalled(); }); - it('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 = mergeSentryCallback(manager, sentryHandler) as { - addHandler: ReturnType; - inheritableHandlers: unknown[]; - }; - expect(result.addHandler).not.toHaveBeenCalled(); - expect(result.inheritableHandlers).toEqual([sentryHandler]); - }); - it('returns the value unchanged when it is neither an array nor a CallbackManager', () => { const opaque = { name: 'NotAManager' }; expect(mergeSentryCallback(opaque, sentryHandler)).toBe(opaque); }); + + it('does not treat a coincidentally duck-typed object as a CallbackManager', () => { + // A plain object that happens to expose `addHandler`/`copy` shouldn't be + // mistaken for a real LangChain CallbackManager — the constructor-name + // check guards against false positives. + const lookalike = { addHandler: vi.fn(), copy: vi.fn(), handlers: [] }; + expect(mergeSentryCallback(lookalike, sentryHandler)).toBe(lookalike); + expect(lookalike.addHandler).not.toHaveBeenCalled(); + expect(lookalike.copy).not.toHaveBeenCalled(); + }); + + it('recognizes subclasses of CallbackManager via the prototype walk', () => { + class CallbackManager { + public handlers: unknown[] = []; + public inheritableHandlers: unknown[] = []; + public addHandler = vi.fn((handler: unknown, inherit?: boolean) => { + this.handlers.push(handler); + if (inherit !== false) { + this.inheritableHandlers.push(handler); + } + }); + public copy = vi.fn(() => new CallbackManager()); + } + class CustomCallbackManager extends CallbackManager {} + const subclass = new CustomCallbackManager(); + const result = mergeSentryCallback(subclass, sentryHandler) as { + addHandler: ReturnType; + }; + expect(result.addHandler).toHaveBeenCalledWith(sentryHandler, true); + }); });