fix(node): Preserve CallbackManager handlers in LangChain instrumentation by mdnanocom · Pull Request #20849 · getsentry/sentry-javascript

mdnanocom · 2026-05-12T19:00:53Z

Summary

augmentCallbackHandlers in packages/node/src/integrations/tracing/langchain/instrumentation.ts previously fell into its typeof handlers === 'object' branch whenever options.callbacks was a single object and wrapped it into [handlers, sentryHandler]. When that object was a LangChain CallbackManager, downstream code treats the whole manager as one opaque handler — its inheritable children (notably LangGraph's StreamMessagesHandler installed by streamMode: ['messages'], and the LangSmith tracer) are never unpacked, so per-token streaming events and nested tracing silently disappear.

This PR detects CallbackManager via duck-typing (avoids coupling to a specific @langchain/core resolution) and registers Sentry's handler as an inheritable child on a copy via .addHandler(handler, true). The manager's existing children continue to receive handleLLMNewToken and friends.

Repro

A LangGraph compiled graph + ChatOpenAI (or any LangChain provider with bindTools(...)), driven through @ai-sdk/langchain's toUIMessageStream:

const stream = await graph.stream(
  { messages },
  { streamMode: ['values', 'messages'] },
);
return createUIMessageStreamResponse({ stream: toUIMessageStream(stream) });

Without this fix: the SSE output collapses to a single aggregated text-delta at end of run. Probing inside _streamResponseChunks shows the per-token chunks fire runManager.handleLLMNewToken correctly, but the LLM-level run-manager's handlers list contains only [CallbackManager, SentryCallbackHandler, langchain_tracer] — StreamMessagesHandler is missing.
With this fix: every token is delivered as the model produces it (verified end-to-end against gpt-5.5 + useResponsesApi: true + bindTools and against gpt-4o-mini with no tools).

Tests

Added packages/node/test/integrations/tracing/langchain.test.ts with 7 unit tests covering:

undefined callbacks → single-handler array
pre-existing array → append (idempotent)
CallbackManager → preserves children, copies rather than mutates, deduplicates
opaque non-manager object → returned unchanged

Verify

Added unit tests for the fixed behaviour
yarn build succeeds
yarn test:unit passes (381/381 in packages/node)
No related issue currently open — happy to open one if preferred.

…tion `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.

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.

mdnanocom · 2026-05-12T19:51:01Z

Thanks for the review — wanted to share what I found before pushing the fix.

Tracing through @langchain/core@1.1.39's CallbackManager:

addHandler(handler, true) pushes to both handlers and inheritableHandlers.
setHandlers(handlers, inherit=true) (used by getChild) clears both arrays, then calls addHandler(_, true) for each — so it also keeps both in sync.

That means in normal operation inheritableHandlers ⊆ handlers, and copied.handlers.includes(sentryHandler) already catches the nested-call case the bot describes. I don't think I can reproduce the duplicate-registration in practice with the stock CallbackManager.

That said, the suggested fix costs nothing and pre-empts the failure mode for any externally-constructed subclass that bypasses addHandler. Pushed it as commit 4c74bfa with a regression test (does not double-register when the handler lives only on inheritableHandlers) — and an explanatory comment on the dedupe so the reasoning doesn't decay.

mdnanocom · 2026-05-12T22:07:33Z

Done in 7d61cf5b (pushed). Lifted the fixed implementation to @sentry/core as mergeSentryCallback and routed both @sentry/instrumentation-langchain and @sentry/instrumentation-langgraph through it. The langgraph integration silently picks up the streaming fix as a bonus — the existing helper had the same three bugs (mutates caller manager, no inherit=true, dedupes only against handlers).

Net: 5 files, +163 / -215. Coverage is now in a single mergeSentryCallback suite (14 cases). 13/13 node tracing files (270 tests) and 83/83 core utils files (1439 tests) pass.

…ryCallback 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).

Address review feedback on PR getsentry#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).

…backmanager-preservation

github-actions · 2026-05-18T11:27:07Z

👋 @nicohrubec, @andreiborza — Please review this PR when you get a chance!

andreiborza

Thank you for your contribution! I left a couple of comments, overall looks good!

- Revert constructor-name guard in CallbackManager detection; minification mangles class names which would silently disable instrumentation. Back to pure duck-typing (`addHandler` + `copy`) — the surface that flows through `options.callbacks` is already constrained. - Rename `mergeSentryCallback` -> `_INTERNAL_mergeLangChainCallbackHandler` and move it (and `isCallbackManager`) from `tracing/langgraph/utils.ts` to `tracing/langchain/utils.ts`; the helpers are LangChain concepts. - Mark `@internal`, simplify the doc comment. - Drop the constructor-name guard tests + the test stand-in's class wrapper (no longer needed). Drop the two test inline-comment blocks flagged redundant by review. - Move the suite from `test/lib/utils/langgraph-utils.test.ts` to `test/lib/tracing/langchain-utils.test.ts` to match the source layout.

mdnanocom · 2026-05-20T12:33:07Z

thanks for the review @andreiborza i made the requested changes :)

andreiborza

Thanks for addressing the reviews, looks good!

Just one more thing I didn't notice earlier, then we're good to go!

andreiborza · 2026-05-21T05:37:59Z

+    const copied = existing.copy() as {
+      addHandler: (handler: unknown, inherit?: boolean) => void;
+      handlers?: unknown[];
+    };
+    if (!copied.handlers?.includes(sentryHandler)) {
+      copied.addHandler(sentryHandler, true);
+    }
+    return copied;


m: We can make a small improvement here by only copying if the existing.handlers does not include the sentry handler already. That way we don't make a copy unless we really need to.

done @andreiborza

Per andreiborza's review: short-circuit before calling existing.copy() if the manager's handlers already include the sentry handler. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 6f36bd1. Configure here.}

Call site in node langchain instrumentation still referenced the old `mergeSentryCallback` name from before the rename, causing a ReferenceError on every invoke/stream/batch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

github-actions · 2026-05-21T11:11:16Z

👋 @nicohrubec — Please review this PR when you get a chance!

mdnanocom requested a review from a team as a code owner May 12, 2026 19:00

test(node): Add unit tests + changelog for LangChain callback fix

4c74bfa

mdnanocom force-pushed the fix/langchain-callbackmanager-preservation branch from 92409bd to 4c74bfa Compare May 12, 2026 19:03

sentry Bot reviewed May 12, 2026

View reviewed changes

Comment thread packages/node/src/integrations/tracing/langchain/instrumentation.ts Outdated

cursor Bot reviewed May 12, 2026

View reviewed changes

Comment thread packages/node/src/integrations/tracing/langchain/instrumentation.ts Outdated

mdnanocom force-pushed the fix/langchain-callbackmanager-preservation branch 2 times, most recently from b2b6b6c to 0c2c666 Compare May 13, 2026 08:24

mdnanocom force-pushed the fix/langchain-callbackmanager-preservation branch from 0c2c666 to 9e2939d Compare May 13, 2026 08:29

JPeer264 reviewed May 13, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Comment thread packages/core/src/tracing/langgraph/utils.ts Outdated

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

JPeer264 requested review from andreiborza and nicohrubec May 13, 2026 09:52

mdnanocom force-pushed the fix/langchain-callbackmanager-preservation branch from 9f0dbc5 to 7dcdd93 Compare May 13, 2026 12:07

mdnanocom added 3 commits May 13, 2026 14:07

Merge remote-tracking branch 'origin/develop' into fix/langchain-call…

fb95ad5

…backmanager-preservation

Merge branch 'develop' into fix/langchain-callbackmanager-preservation

5657854

Merge branch 'develop' into fix/langchain-callbackmanager-preservation

7333d66

Merge branch 'develop' into fix/langchain-callbackmanager-preservation

d88c503

sentry Bot reviewed May 18, 2026

View reviewed changes

Comment thread packages/core/src/tracing/langgraph/utils.ts Outdated

andreiborza requested changes May 19, 2026

View reviewed changes

mdnanocom added 2 commits May 20, 2026 14:27

Merge branch 'develop' into fix/langchain-callbackmanager-preservation

cde3b58

andreiborza reviewed May 21, 2026

View reviewed changes

review: Skip copy when manager already contains the sentry handler

6f36bd1

Per andreiborza's review: short-circuit before calling existing.copy() if the manager's handlers already include the sentry handler. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

cursor Bot reviewed May 21, 2026

View reviewed changes

Comment thread packages/node/src/integrations/tracing/langchain/instrumentation.ts Outdated

mdnanocom and others added 2 commits May 21, 2026 11:00

Merge branch 'develop' into fix/langchain-callbackmanager-preservation

745b9e1

Uh oh!

Conversation

mdnanocom commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Repro

Tests

Verify

Uh oh!

Uh oh!

mdnanocom commented May 12, 2026

Uh oh!

Uh oh!

mdnanocom commented May 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 18, 2026

Uh oh!

Uh oh!

andreiborza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdnanocom commented May 20, 2026

Uh oh!

andreiborza left a comment

Choose a reason for hiding this comment

Uh oh!

andreiborza May 21, 2026

Choose a reason for hiding this comment

Uh oh!

mdnanocom May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mdnanocom commented May 12, 2026 •

edited

Loading

mdnanocom May 21, 2026 •

edited

Loading