Skip to content

Commit 9f0dbc5

Browse files
committed
review: Tighten CallbackManager detection + drop changelog entry
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).
1 parent 9e2939d commit 9f0dbc5

3 files changed

Lines changed: 61 additions & 14 deletions

File tree

CHANGELOG.md

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

33
## Unreleased
44

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

87
## 10.53.1

packages/core/src/tracing/langgraph/utils.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,13 @@ export function setResponseAttributes(span: Span, inputMessages: LangChainMessag
335335
}
336336
}
337337

338-
/** Duck-types a LangChain `CallbackManager` — `instanceof` is unreliable when `@langchain/core` is bundled or deduped. */
338+
/**
339+
* Detects a LangChain `CallbackManager` (or subclass) without depending on `instanceof`.
340+
* `@langchain/core` is frequently bundled or deduped, so the imported constructor doesn't
341+
* necessarily match the one at the user's call site. We walk the prototype chain looking
342+
* for the class name, then confirm the shape — the constructor-name check rules out
343+
* unrelated objects that happen to expose `addHandler`/`copy`.
344+
*/
339345
function isCallbackManager(value: unknown): value is {
340346
addHandler: (handler: unknown, inherit?: boolean) => void;
341347
copy: () => unknown;
@@ -345,8 +351,16 @@ function isCallbackManager(value: unknown): value is {
345351
if (!value || typeof value !== 'object') {
346352
return false;
347353
}
348-
const candidate = value as { addHandler?: unknown; copy?: unknown };
349-
return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function';
354+
355+
let proto: object | null = Object.getPrototypeOf(value);
356+
while (proto) {
357+
if ((proto as { constructor?: { name?: string } }).constructor?.name === 'CallbackManager') {
358+
const candidate = value as { addHandler?: unknown; copy?: unknown };
359+
return typeof candidate.addHandler === 'function' && typeof candidate.copy === 'function';
360+
}
361+
proto = Object.getPrototypeOf(proto);
362+
}
363+
return false;
350364
}
351365

352366
/**

packages/core/test/lib/utils/langgraph-utils.test.ts

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,20 +56,24 @@ describe('mergeSentryCallback', () => {
5656
* test against a degenerate shape that bypasses `addHandler`.
5757
*/
5858
function makeFakeCallbackManager(existingHandlers: unknown[] = [], existingInheritableHandlers?: unknown[]) {
59-
const manager = {
60-
handlers: [...existingHandlers],
61-
inheritableHandlers: [...(existingInheritableHandlers ?? existingHandlers)],
62-
addHandler: vi.fn(function (this: any, handler: unknown, inherit?: boolean) {
59+
// Use a class so `Object.getPrototypeOf(instance).constructor.name === 'CallbackManager'`,
60+
// which is how the production detector identifies a real LangChain CallbackManager.
61+
class CallbackManager {
62+
public handlers: unknown[];
63+
public inheritableHandlers: unknown[];
64+
public addHandler = vi.fn((handler: unknown, inherit?: boolean) => {
6365
this.handlers.push(handler);
6466
if (inherit !== false) {
6567
this.inheritableHandlers.push(handler);
6668
}
67-
}),
68-
copy: vi.fn(function (this: any) {
69-
return makeFakeCallbackManager(this.handlers, this.inheritableHandlers);
70-
}),
71-
};
72-
return manager;
69+
});
70+
public copy = vi.fn(() => makeFakeCallbackManager(this.handlers, this.inheritableHandlers));
71+
constructor(initialHandlers: unknown[], initialInheritableHandlers: unknown[]) {
72+
this.handlers = [...initialHandlers];
73+
this.inheritableHandlers = [...initialInheritableHandlers];
74+
}
75+
}
76+
return new CallbackManager(existingHandlers, existingInheritableHandlers ?? existingHandlers);
7377
}
7478

7579
it('returns a fresh array when no existing callbacks are present', () => {
@@ -159,4 +163,34 @@ describe('mergeSentryCallback', () => {
159163
const opaque = { name: 'NotAManager' };
160164
expect(mergeSentryCallback(opaque, sentryHandler)).toBe(opaque);
161165
});
166+
167+
it('does not treat a coincidentally duck-typed object as a CallbackManager', () => {
168+
// A plain object that happens to expose `addHandler`/`copy` shouldn't be
169+
// mistaken for a real LangChain CallbackManager — the constructor-name
170+
// check guards against false positives.
171+
const lookalike = { addHandler: vi.fn(), copy: vi.fn(), handlers: [] };
172+
expect(mergeSentryCallback(lookalike, sentryHandler)).toBe(lookalike);
173+
expect(lookalike.addHandler).not.toHaveBeenCalled();
174+
expect(lookalike.copy).not.toHaveBeenCalled();
175+
});
176+
177+
it('recognizes subclasses of CallbackManager via the prototype walk', () => {
178+
class CallbackManager {
179+
public handlers: unknown[] = [];
180+
public inheritableHandlers: unknown[] = [];
181+
public addHandler = vi.fn((handler: unknown, inherit?: boolean) => {
182+
this.handlers.push(handler);
183+
if (inherit !== false) {
184+
this.inheritableHandlers.push(handler);
185+
}
186+
});
187+
public copy = vi.fn(() => new CallbackManager());
188+
}
189+
class CustomCallbackManager extends CallbackManager {}
190+
const subclass = new CustomCallbackManager();
191+
const result = mergeSentryCallback(subclass, sentryHandler) as {
192+
addHandler: ReturnType<typeof vi.fn>;
193+
};
194+
expect(result.addHandler).toHaveBeenCalledWith(sentryHandler, true);
195+
});
162196
});

0 commit comments

Comments
 (0)