From 5cce8addf07c108620040311246be561db11d64a Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Sat, 21 Mar 2026 00:01:26 +0800 Subject: [PATCH 1/2] Fix APIPromise workaround in bundlers --- js/src/auto-instrumentations/hook.mts | 91 +------ .../patch-tracing-channel.test.ts | 234 ++++++++++++++++++ .../patch-tracing-channel.ts | 110 ++++++++ js/src/browser/config.ts | 5 + js/src/node/config.ts | 5 + 5 files changed, 359 insertions(+), 86 deletions(-) create mode 100644 js/src/auto-instrumentations/patch-tracing-channel.test.ts create mode 100644 js/src/auto-instrumentations/patch-tracing-channel.ts diff --git a/js/src/auto-instrumentations/hook.mts b/js/src/auto-instrumentations/hook.mts index 41a4b2394..d7f656baa 100644 --- a/js/src/auto-instrumentations/hook.mts +++ b/js/src/auto-instrumentations/hook.mts @@ -21,95 +21,14 @@ import { claudeAgentSDKConfigs } from "./configs/claude-agent-sdk.js"; import { googleGenAIConfigs } from "./configs/google-genai.js"; import { openRouterConfigs } from "./configs/openrouter.js"; import { ModulePatch } from "./loader/cjs-patch.js"; +import { patchTracingChannel } from "./patch-tracing-channel.js"; -// Patch diagnostics_channel.tracePromise to handle APIPromise correctly -// MUST be done here (before any SDK code runs) to fix Anthropic APIPromise incompatibility -// Construct the module path dynamically to prevent build from stripping "node:" prefix +// Patch diagnostics_channel.tracePromise to handle APIPromise correctly. +// MUST be done here (before any SDK code runs) to fix Anthropic APIPromise incompatibility. +// Construct the module path dynamically to prevent build from stripping "node:" prefix. const dcPath = ["node", "diagnostics_channel"].join(":"); const dc: any = await import(/* @vite-ignore */ dcPath as any); - -// Get TracingChannel class by creating a dummy instance -const dummyChannel = dc.tracingChannel("dummy"); -const TracingChannel = dummyChannel.constructor; - -if ( - TracingChannel && - !Object.getOwnPropertyDescriptor(TracingChannel.prototype, "hasSubscribers") -) { - Object.defineProperty(TracingChannel.prototype, "hasSubscribers", { - configurable: true, - enumerable: false, - get(this: { - start?: { hasSubscribers?: boolean }; - end?: { hasSubscribers?: boolean }; - asyncStart?: { hasSubscribers?: boolean }; - asyncEnd?: { hasSubscribers?: boolean }; - error?: { hasSubscribers?: boolean }; - }) { - return Boolean( - this.start?.hasSubscribers || - this.end?.hasSubscribers || - this.asyncStart?.hasSubscribers || - this.asyncEnd?.hasSubscribers || - this.error?.hasSubscribers, - ); - }, - }); -} - -if (TracingChannel && TracingChannel.prototype.tracePromise) { - TracingChannel.prototype.tracePromise = function ( - fn: any, - context: any = {}, - thisArg: any, - ...args: any[] - ) { - const { start, end, asyncStart, asyncEnd, error } = this; - - function reject(err: any) { - context.error = err; - error?.publish(context); - asyncStart?.publish(context); - asyncEnd?.publish(context); - return Promise.reject(err); - } - - function resolve(result: any) { - context.result = result; - asyncStart?.publish(context); - asyncEnd?.publish(context); - return result; - } - - start?.publish(context); - - try { - // PATCHED: Removed instanceof Promise check and Promise.resolve() wrapper - // This allows APIPromise and other Promise subclasses to work correctly - - const result = Reflect.apply(fn, thisArg, args); - - if ( - result && - (typeof result === "object" || typeof result === "function") && - typeof result.then === "function" - ) { - return result.then(resolve, reject); - } - - context.result = result; - asyncStart?.publish(context); - asyncEnd?.publish(context); - return result; - } catch (err) { - context.error = err; - error?.publish(context); - throw err; - } finally { - end?.publish(context); - } - }; -} +patchTracingChannel(dc.tracingChannel); // Combine all instrumentation configs const allConfigs = [ diff --git a/js/src/auto-instrumentations/patch-tracing-channel.test.ts b/js/src/auto-instrumentations/patch-tracing-channel.test.ts new file mode 100644 index 000000000..da89a5550 --- /dev/null +++ b/js/src/auto-instrumentations/patch-tracing-channel.test.ts @@ -0,0 +1,234 @@ +/** + * Regression tests for patchTracingChannel. + * + * The bug: Node.js diagnostics_channel's tracePromise calls PromisePrototypeThen + * (the original, unoverridden Promise.prototype.then) on the return value. This uses + * the Symbol.species protocol to create the result promise — it calls + * new SubClass(resolve, reject) — which breaks for Promise subclasses that don't + * accept a standard (resolve, reject) executor as their first argument (e.g., + * Anthropic's APIPromise, which takes a responsePromise). + * + * The fix: call result.then(resolve, reject) directly, which invokes whatever .then() + * override the subclass provides (typically a safe version that delegates to an inner + * native Promise, avoiding the species protocol entirely). + */ + +import { describe, it, expect } from "vitest"; +import { patchTracingChannel } from "./patch-tracing-channel"; + +/** + * Creates a fresh TracingChannel-like class with the ORIGINAL broken tracePromise + * behavior, simulating Node.js diagnostics_channel before the patch is applied. + * + * The key difference from the patched version: it calls + * `Promise.prototype.then.call(result, ...)` (equivalent to Node.js's internal + * PromisePrototypeThen), which triggers Symbol.species on Promise subclasses. + */ +function makeUnpatchedTracingChannel() { + class FakeChannel { + readonly hasSubscribers = true; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + publish(_msg: any): void {} + // eslint-disable-next-line @typescript-eslint/no-explicit-any + runStores(_msg: any, fn: () => any): any { + return fn(); + } + } + + class FakeTracingChannel { + start = new FakeChannel(); + end = new FakeChannel(); + asyncStart = new FakeChannel(); + asyncEnd = new FakeChannel(); + error = new FakeChannel(); + + tracePromise( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + fn: (...args: any[]) => any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + context: Record = {}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + thisArg: any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...args: any[] + ): Promise { + const { start, end, asyncStart, asyncEnd, error } = this; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + function reject(err: any) { + context.error = err; + error.publish(context); + asyncStart.publish(context); + asyncEnd.publish(context); + return Promise.reject(err); + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + function resolve(result: any) { + context.result = result; + asyncStart.publish(context); + asyncEnd.publish(context); + return result; + } + + start.publish(context); + + try { + const result = Reflect.apply(fn, thisArg, args); + + // BROKEN: calls original Promise.prototype.then directly, equivalent to + // Node.js's internal PromisePrototypeThen(result, resolve, reject). + // This triggers Symbol.species on Promise subclasses, calling + // new SubClass(resolve, reject) — which breaks for non-standard constructors. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (Promise.prototype.then as any).call( + result as Promise, + resolve, + reject, + ); + } catch (err) { + context.error = err; + error.publish(context); + throw err; + } finally { + end.publish(context); + } + } + } + + return FakeTracingChannel; +} + +/** + * Minimal reproduction of Anthropic's APIPromise pattern: + * - Non-standard constructor: takes a responsePromise, not a (resolve, reject) executor + * - Overrides .then() to delegate to an inner native Promise (avoiding species protocol) + * - Sets Symbol.species to a class that throws, reproducing the exact error + * + * When PromisePrototypeThen(apiPromise, resolve, reject) is called (the original .then), + * it uses Symbol.species to create the result promise: new FailingSpecies(resolve, reject) + * which throws "Promise resolve or reject function is not callable". + * + * When apiPromise.then(resolve, reject) is called (the patched behavior), it invokes + * the safe .then() override which delegates to the inner native promise, bypassing + * Symbol.species entirely. + */ +class MockAPIPromise extends Promise { + readonly #innerPromise: Promise; + + constructor(responsePromise: Promise) { + let res!: (value: T | PromiseLike) => void; + super((resolve) => { + res = resolve; + }); + this.#innerPromise = Promise.resolve(responsePromise); + this.#innerPromise.then(res); + } + + // Safe override: delegates to the inner native Promise instead of `this`. + // Calling .then() on a native Promise uses native Promise as the species, + // so it never tries to call new MockAPIPromise(resolve, reject). + override then( + onfulfilled?: ((value: T) => TResult1 | PromiseLike) | null, + onrejected?: ((reason: unknown) => TResult2 | PromiseLike) | null, + ): Promise { + return this.#innerPromise.then(onfulfilled, onrejected); + } + + // Symbol.species returns a class that throws, reproducing the exact error seen + // when Node.js's PromisePrototypeThen tries to construct the result promise. + static override get [Symbol.species](): PromiseConstructor { + return class { + constructor() { + throw new TypeError( + "Promise resolve or reject function is not callable", + ); + } + } as unknown as PromiseConstructor; + } +} + +describe("patchTracingChannel", () => { + it("unpatched tracePromise throws when given a Promise subclass with incompatible Symbol.species", () => { + // This reproduces the error reported with Anthropic's APIPromise + Node.js bundler path. + const FakeTCClass = makeUnpatchedTracingChannel(); + const channel = new FakeTCClass(); + const apiPromise = new MockAPIPromise(Promise.resolve("hello")); + + // The unpatched tracePromise calls Promise.prototype.then.call(result, ...) which + // uses Symbol.species to create the result promise, throwing synchronously. + expect(() => channel.tracePromise(() => apiPromise, {}, null)).toThrow( + "Promise resolve or reject function is not callable", + ); + }); + + it("patched tracePromise correctly handles Promise subclasses with non-standard Symbol.species", async () => { + const FakeTCClass = makeUnpatchedTracingChannel(); + const channel = new FakeTCClass(); + + // Apply the patch — replaces FakeTCClass.prototype.tracePromise with the fixed version + patchTracingChannel(() => channel); + + const context: Record = {}; + const apiPromise = new MockAPIPromise(Promise.resolve("hello")); + + // The patched tracePromise calls result.then(resolve, reject) directly, which + // invokes our safe .then() override that avoids the species protocol. + const result = await channel.tracePromise(() => apiPromise, context, null); + + expect(result).toBe("hello"); + expect(context.result).toBe("hello"); + }); + + it("patched tracePromise correctly handles plain async functions", async () => { + const FakeTCClass = makeUnpatchedTracingChannel(); + const channel = new FakeTCClass(); + patchTracingChannel(() => channel); + + const context: Record = {}; + const result = await channel.tracePromise( + async () => "world", + context, + null, + ); + + expect(result).toBe("world"); + expect(context.result).toBe("world"); + }); + + it("patched tracePromise propagates rejections and sets context.error", async () => { + const FakeTCClass = makeUnpatchedTracingChannel(); + const channel = new FakeTCClass(); + patchTracingChannel(() => channel); + + const context: Record = {}; + const err = new Error("api error"); + + await expect( + channel.tracePromise( + async () => { + throw err; + }, + context, + null, + ), + ).rejects.toBe(err); + + expect(context.error).toBe(err); + }); + + it("is idempotent — applying the patch twice produces correct behavior", async () => { + const FakeTCClass = makeUnpatchedTracingChannel(); + const channel = new FakeTCClass(); + + patchTracingChannel(() => channel); + patchTracingChannel(() => channel); + + const context: Record = {}; + const apiPromise = new MockAPIPromise(Promise.resolve(99)); + const result = await channel.tracePromise(() => apiPromise, context, null); + + expect(result).toBe(99); + expect(context.result).toBe(99); + }); +}); diff --git a/js/src/auto-instrumentations/patch-tracing-channel.ts b/js/src/auto-instrumentations/patch-tracing-channel.ts new file mode 100644 index 000000000..67555c945 --- /dev/null +++ b/js/src/auto-instrumentations/patch-tracing-channel.ts @@ -0,0 +1,110 @@ +/** + * Patches TracingChannel.prototype to handle APIPromise and other Promise subclasses + * that change the constructor signature (violating the species contract). + * + * node:diagnostics_channel's tracePromise wraps the result with Promise.resolve(), + * which calls the subclass constructor with the wrong signature for classes like + * Anthropic's APIPromise. This patch uses duck-typing (.then check) instead. + * + * This is applied both in the loader hook (hook.mts) for the --import path, + * and in configureNode/configureBrowser for the bundler plugin path. + */ + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function patchTracingChannel( + tracingChannelFn: (name: string) => any, +): void { + const dummyChannel = tracingChannelFn("__braintrust_probe__"); + const TracingChannel = dummyChannel?.constructor; + + if (!TracingChannel?.prototype) { + return; + } + + if ( + !Object.getOwnPropertyDescriptor(TracingChannel.prototype, "hasSubscribers") + ) { + Object.defineProperty(TracingChannel.prototype, "hasSubscribers", { + configurable: true, + enumerable: false, + get(this: { + start?: { hasSubscribers?: boolean }; + end?: { hasSubscribers?: boolean }; + asyncStart?: { hasSubscribers?: boolean }; + asyncEnd?: { hasSubscribers?: boolean }; + error?: { hasSubscribers?: boolean }; + }) { + return Boolean( + this.start?.hasSubscribers || + this.end?.hasSubscribers || + this.asyncStart?.hasSubscribers || + this.asyncEnd?.hasSubscribers || + this.error?.hasSubscribers, + ); + }, + }); + } + + if (TracingChannel.prototype.tracePromise) { + TracingChannel.prototype.tracePromise = function ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + fn: any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + context: any = {}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + thisArg: any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...args: any[] + ) { + const { start, end, asyncStart, asyncEnd, error } = this; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + function reject(err: any) { + context.error = err; + error?.publish(context); + asyncStart?.publish(context); + asyncEnd?.publish(context); + return Promise.reject(err); + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + function resolve(result: any) { + context.result = result; + asyncStart?.publish(context); + asyncEnd?.publish(context); + return result; + } + + // Use runStores (not just publish) so fn runs inside the ALS context + // established by bindStore — required for span context to propagate across awaits. + // PATCHED: inside the callback, use duck-type thenable check instead of + // PromisePrototypeThen, which triggers Symbol.species and breaks Promise subclasses + // like Anthropic's APIPromise that have non-standard constructors. + return start.runStores(context, () => { + try { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result: any = Reflect.apply(fn, thisArg, args); + end?.publish(context); + + if ( + result && + (typeof result === "object" || typeof result === "function") && + typeof result.then === "function" + ) { + return result.then(resolve, reject); + } + + context.result = result; + asyncStart?.publish(context); + asyncEnd?.publish(context); + return result; + } catch (err) { + context.error = err; + error?.publish(context); + end?.publish(context); + throw err; + } + }); + }; + } +} diff --git a/js/src/browser/config.ts b/js/src/browser/config.ts index 0b8d7fd0a..ceda57b81 100644 --- a/js/src/browser/config.ts +++ b/js/src/browser/config.ts @@ -2,6 +2,7 @@ import { tracingChannel } from "dc-browser"; import iso from "../isomorph"; import { _internalSetInitialState } from "../logger"; import { registry } from "../instrumentation/registry"; +import { patchTracingChannel } from "../auto-instrumentations/patch-tracing-channel"; // This is copied from next.js. It seems they define AsyncLocalStorage in the edge // environment, even though it's not defined in the browser. @@ -36,6 +37,10 @@ export function configureBrowser(): void { iso.newTracingChannel = <_M = any>(nameOrChannels: string | object) => tracingChannel(nameOrChannels as any) as any; + // Patch TracingChannel.prototype.tracePromise to handle APIPromise and other + // Promise subclasses (mirrors the fix in hook.mts for the --import loader path). + patchTracingChannel(tracingChannel); + iso.getEnv = (name: string) => { if (typeof process === "undefined" || typeof process.env === "undefined") { return undefined; diff --git a/js/src/node/config.ts b/js/src/node/config.ts index 78da9ea54..eafbe17c0 100644 --- a/js/src/node/config.ts +++ b/js/src/node/config.ts @@ -1,6 +1,7 @@ import { AsyncLocalStorage } from "node:async_hooks"; import * as diagnostics_channel from "node:diagnostics_channel"; import * as path from "node:path"; +import { patchTracingChannel } from "../auto-instrumentations/patch-tracing-channel"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as fsSync from "node:fs"; @@ -24,6 +25,10 @@ export function configureNode() { iso.newAsyncLocalStorage = () => new AsyncLocalStorage(); iso.newTracingChannel = <_M = any>(nameOrChannels: string | object) => (diagnostics_channel as any).tracingChannel(nameOrChannels) as any; + + // Patch TracingChannel.prototype.tracePromise to handle APIPromise and other + // Promise subclasses (mirrors the fix in hook.mts for the --import loader path). + patchTracingChannel((diagnostics_channel as any).tracingChannel); iso.processOn = (event: string, handler: (code: unknown) => void) => { process.on(event, handler); }; From d3ddea1220757acfd76be84422d965ed3d38c9cc Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Sat, 21 Mar 2026 02:50:05 +0800 Subject: [PATCH 2/2] test(e2e): Update snapshots for patchTracingChannel bundling change Auto-instrumented spans now produce context:{} instead of context:{caller_filename:"",...} because patchTracingChannel is bundled into index.mjs alongside getCallerLocation. When all stack frames share the same directory, getCallerLocation returns undefined. The value was an artifact of the native C++ tracePromise call stack, not meaningful user code location info. Co-Authored-By: Claude Sonnet 4.6 --- .../__snapshots__/log-payloads.json | 42 ++++--------------- .../__snapshots__/log-payloads.json | 30 +++---------- 2 files changed, 12 insertions(+), 60 deletions(-) diff --git a/e2e/scenarios/anthropic-auto-instrumentation-node-hook/__snapshots__/log-payloads.json b/e2e/scenarios/anthropic-auto-instrumentation-node-hook/__snapshots__/log-payloads.json index b95b5edeb..71bb1bc4b 100644 --- a/e2e/scenarios/anthropic-auto-instrumentation-node-hook/__snapshots__/log-payloads.json +++ b/e2e/scenarios/anthropic-auto-instrumentation-node-hook/__snapshots__/log-payloads.json @@ -79,11 +79,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": [ @@ -214,11 +210,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": [ @@ -367,11 +359,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": [ @@ -495,11 +483,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": [ @@ -623,11 +607,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": [ @@ -783,11 +763,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": [ @@ -918,11 +894,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": [ diff --git a/e2e/scenarios/google-genai-auto-instrumentation-node-hook/__snapshots__/log-payloads.json b/e2e/scenarios/google-genai-auto-instrumentation-node-hook/__snapshots__/log-payloads.json index 5e7813b51..082a95b8d 100644 --- a/e2e/scenarios/google-genai-auto-instrumentation-node-hook/__snapshots__/log-payloads.json +++ b/e2e/scenarios/google-genai-auto-instrumentation-node-hook/__snapshots__/log-payloads.json @@ -79,11 +79,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": { @@ -254,11 +250,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": { @@ -450,11 +442,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": { @@ -608,11 +596,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": { @@ -757,11 +741,7 @@ ] }, { - "context": { - "caller_filename": "", - "caller_functionname": "", - "caller_lineno": 0 - }, + "context": {}, "created": "", "id": "", "input": {