Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- fix(node): Preserve `CallbackManager` children when augmenting LangChain callbacks
Comment thread
mdnanocom marked this conversation as resolved.
Outdated
- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

## 10.53.1
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/shared-exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
44 changes: 38 additions & 6 deletions packages/core/src/tracing/langgraph/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Comment thread
mdnanocom marked this conversation as resolved.
Outdated
}

/**
* 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.
*/
Comment thread
mdnanocom marked this conversation as resolved.
Outdated
export function mergeSentryCallback(existing: unknown, sentryHandler: unknown): unknown {
Comment thread
mdnanocom marked this conversation as resolved.
Outdated
if (!existing) {
return [sentryHandler];
Expand All @@ -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;
Expand Down
101 changes: 90 additions & 11 deletions packages/core/test/lib/utils/langgraph-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,30 @@ describe('extractAgentNameFromParams', () => {
describe('mergeSentryCallback', () => {
const sentryHandler = { _sentry: true };

/**
Comment thread
mdnanocom marked this conversation as resolved.
Outdated
* 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]);
Expand All @@ -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.
Comment thread
mdnanocom marked this conversation as resolved.
Outdated
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<typeof vi.fn>;
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<typeof vi.fn>;
};
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<typeof vi.fn>;
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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
createLangChainCallbackHandler,
GOOGLE_GENAI_INTEGRATION_NAME,
instrumentLangChainEmbeddings,
mergeSentryCallback,
OPENAI_INTEGRATION_NAME,
SDK_VERSION,
} from '@sentry/core';
Expand All @@ -27,34 +28,6 @@ interface PatchedLangChainExports {
[key: string]: unknown;
}

/**
* Augments a callback handler list with Sentry's handler if not already present
*/
function augmentCallbackHandlers(handlers: unknown, sentryHandler: unknown): unknown {
Comment thread
mdnanocom marked this conversation as resolved.
// 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];
}

// If it's a single handler object, convert to array
if (typeof handlers === 'object') {
return [handlers, sentryHandler];
}

// 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
Expand Down Expand Up @@ -82,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);
Comment thread
mdnanocom marked this conversation as resolved.
Outdated

// Call original method with augmented options
return Reflect.apply(target, thisArg, args);
Expand Down