Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,48 @@ 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';
}

/**
* 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 {
Comment thread
mdnanocom marked this conversation as resolved.
// Handle null/undefined - return array with just our handler
Expand All @@ -46,9 +87,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);
}
Comment thread
sentry[bot] marked this conversation as resolved.
Outdated
return copied;
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
}

// Unknown type - return original
Expand Down
87 changes: 87 additions & 0 deletions packages/node/test/integrations/tracing/langchain.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof vi.fn>;
};
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);
});
});