Skip to content

Commit 4c74bfa

Browse files
committed
test(node): Add unit tests + changelog for LangChain callback fix
1 parent e2b872a commit 4c74bfa

3 files changed

Lines changed: 94 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## Unreleased
44

5+
- fix(node): Preserve `CallbackManager` children when augmenting LangChain callbacks
56
- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
67

78
## 10.53.1

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ function isCallbackManager(value: unknown): value is {
4444
return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function';
4545
}
4646

47+
/**
48+
* Exported for testing.
49+
* @internal
50+
*/
51+
export const _INTERNAL_augmentCallbackHandlers = augmentCallbackHandlers;
52+
4753
/**
4854
* Augments a callback handler list with Sentry's handler if not already present.
4955
*
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { describe, expect, test, vi } from 'vitest';
2+
import { _INTERNAL_augmentCallbackHandlers } from '../../../src/integrations/tracing/langchain/instrumentation';
3+
4+
const sentryHandler = { name: 'SentryCallbackHandler' };
5+
6+
/**
7+
* Minimal `CallbackManager` stand-in. We only need the duck-typed shape
8+
* (`addHandler` + `copy`) for the production code to recognize this as a
9+
* `CallbackManager` rather than fall through to the "unknown" branch.
10+
*/
11+
function makeFakeCallbackManager(existingHandlers: unknown[] = []) {
12+
const manager = {
13+
handlers: [...existingHandlers],
14+
addHandler: vi.fn(function (this: any, handler: unknown, _inherit?: boolean) {
15+
this.handlers.push(handler);
16+
}),
17+
copy: vi.fn(function (this: any) {
18+
return makeFakeCallbackManager(this.handlers);
19+
}),
20+
};
21+
return manager;
22+
}
23+
24+
describe('augmentCallbackHandlers', () => {
25+
test('wraps Sentry handler in an array when no callbacks are configured', () => {
26+
const result = _INTERNAL_augmentCallbackHandlers(undefined, sentryHandler);
27+
expect(result).toEqual([sentryHandler]);
28+
});
29+
30+
test('appends Sentry handler when callbacks is already an array', () => {
31+
const other = { name: 'OtherHandler' };
32+
const result = _INTERNAL_augmentCallbackHandlers([other], sentryHandler);
33+
expect(result).toEqual([other, sentryHandler]);
34+
});
35+
36+
test('is idempotent when Sentry handler is already in the array', () => {
37+
const result = _INTERNAL_augmentCallbackHandlers([sentryHandler], sentryHandler);
38+
expect(result).toEqual([sentryHandler]);
39+
});
40+
41+
test('preserves inheritable handlers when callbacks is a CallbackManager', () => {
42+
// Reproduces the LangGraph `streamMode: ['messages']` setup: a
43+
// CallbackManager carrying a StreamMessagesHandler is passed via
44+
// options.callbacks. Without this fix, the manager would be wrapped as
45+
// `[manager, sentryHandler]`, dropping all its inheritable children.
46+
const streamMessagesHandler = {
47+
name: 'StreamMessagesHandler',
48+
lc_prefer_streaming: true,
49+
};
50+
const manager = makeFakeCallbackManager([streamMessagesHandler]);
51+
52+
const result = _INTERNAL_augmentCallbackHandlers(manager, sentryHandler) as {
53+
handlers: unknown[];
54+
};
55+
56+
// The result is a manager (object), not a wrapping array.
57+
expect(Array.isArray(result)).toBe(false);
58+
// The original child handler is still there alongside Sentry's.
59+
expect(result.handlers).toEqual([streamMessagesHandler, sentryHandler]);
60+
});
61+
62+
test('copies the manager rather than mutating the caller-supplied one', () => {
63+
// If we mutated the original manager, repeated invocations would
64+
// accumulate Sentry handlers (and tracers from prior runs would leak
65+
// into subsequent unrelated runs).
66+
const manager = makeFakeCallbackManager([]);
67+
_INTERNAL_augmentCallbackHandlers(manager, sentryHandler);
68+
expect(manager.copy).toHaveBeenCalledTimes(1);
69+
expect(manager.handlers).toEqual([]);
70+
});
71+
72+
test('does not double-register Sentry handler when copy already contains it', () => {
73+
const manager = makeFakeCallbackManager([sentryHandler]);
74+
const result = _INTERNAL_augmentCallbackHandlers(manager, sentryHandler) as {
75+
handlers: unknown[];
76+
addHandler: ReturnType<typeof vi.fn>;
77+
};
78+
expect(result.handlers).toEqual([sentryHandler]);
79+
expect(result.addHandler).not.toHaveBeenCalled();
80+
});
81+
82+
test('returns the value unchanged when it is neither an array nor a CallbackManager', () => {
83+
const opaque = { name: 'NotAManager' };
84+
const result = _INTERNAL_augmentCallbackHandlers(opaque, sentryHandler);
85+
expect(result).toBe(opaque);
86+
});
87+
});

0 commit comments

Comments
 (0)