From 97a5b54d46bbf763e494058667e37c735a60c0a7 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 27 Mar 2026 14:36:16 +0100 Subject: [PATCH 1/4] add regression test --- .../openai-instrumentation/assertions.ts | 21 ++++++++ .../openai-instrumentation/scenario.impl.mjs | 51 +++++++++++++++++++ .../openai-instrumentation/scenario.test.ts | 4 ++ 3 files changed, 76 insertions(+) diff --git a/e2e/scenarios/openai-instrumentation/assertions.ts b/e2e/scenarios/openai-instrumentation/assertions.ts index ba7fcca8e..d1826696f 100644 --- a/e2e/scenarios/openai-instrumentation/assertions.ts +++ b/e2e/scenarios/openai-instrumentation/assertions.ts @@ -427,6 +427,7 @@ function buildPayloadSummary(events: CapturedLogEvent[]): Json { } export function defineOpenAIInstrumentationAssertions(options: { + assertPrivateFieldMethodsOperation?: boolean; name: string; runScenario: RunOpenAIScenario; snapshotName: string; @@ -466,6 +467,26 @@ export function defineOpenAIInstrumentationAssertions(options: { }); }); + if (options.assertPrivateFieldMethodsOperation) { + test( + "keeps wrapped v6 client private-field methods callable", + testConfig, + () => { + const root = findLatestSpan(events, ROOT_NAME); + const operation = findLatestSpan( + events, + "openai-client-private-fields-operation", + ); + + expect(operation).toBeDefined(); + expect(operation?.row.metadata).toMatchObject({ + operation: "client-private-fields", + }); + expect(operation?.span.parentIds).toEqual([root?.span.id ?? ""]); + }, + ); + } + for (const spec of OPERATION_SPECS) { test(spec.testName, testConfig, () => { const root = findLatestSpan(events, ROOT_NAME); diff --git a/e2e/scenarios/openai-instrumentation/scenario.impl.mjs b/e2e/scenarios/openai-instrumentation/scenario.impl.mjs index 5383462a0..99771e5b0 100644 --- a/e2e/scenarios/openai-instrumentation/scenario.impl.mjs +++ b/e2e/scenarios/openai-instrumentation/scenario.impl.mjs @@ -44,6 +44,15 @@ async function awaitMaybeWithResponse(request) { }; } +function parseMajorVersion(version) { + if (typeof version !== "string") { + return null; + } + + const major = Number.parseInt(version.split(".")[0], 10); + return Number.isNaN(major) ? null : major; +} + export async function runOpenAIInstrumentationScenario(options) { const baseClient = new options.OpenAI({ apiKey: process.env.OPENAI_API_KEY, @@ -52,9 +61,51 @@ export async function runOpenAIInstrumentationScenario(options) { const client = options.decorateClient ? options.decorateClient(baseClient) : baseClient; + const openAIMajorVersion = parseMajorVersion(options.openaiSdkVersion); + const shouldCheckPrivateFieldMethods = + typeof options.decorateClient === "function" && + openAIMajorVersion !== null && + openAIMajorVersion >= 6; await runTracedScenario({ callback: async () => { + if (shouldCheckPrivateFieldMethods) { + await runOperation( + "openai-client-private-fields-operation", + "client-private-fields", + async () => { + if ( + typeof client.buildURL !== "function" || + typeof client.buildRequest !== "function" + ) { + throw new Error( + "Expected wrapped OpenAI v6 client to expose buildURL and buildRequest", + ); + } + + const builtUrl = client.buildURL("/files", null); + if (typeof builtUrl !== "string" || !builtUrl.includes("/files")) { + throw new Error( + `Unexpected buildURL result: ${String(builtUrl)}`, + ); + } + + const builtRequest = await client.buildRequest( + { method: "post", path: "/files" }, + { retryCount: 0 }, + ); + if ( + typeof builtRequest?.url !== "string" || + !builtRequest.url.includes("/files") + ) { + throw new Error( + `Unexpected buildRequest result: ${String(builtRequest?.url)}`, + ); + } + }, + ); + } + await runOperation("openai-chat-operation", "chat", async () => { await client.chat.completions.create({ model: OPENAI_MODEL, diff --git a/e2e/scenarios/openai-instrumentation/scenario.test.ts b/e2e/scenarios/openai-instrumentation/scenario.test.ts index 65b4c81e8..f9bc438ae 100644 --- a/e2e/scenarios/openai-instrumentation/scenario.test.ts +++ b/e2e/scenarios/openai-instrumentation/scenario.test.ts @@ -40,8 +40,12 @@ const openaiScenarios = await Promise.all( ); for (const scenario of openaiScenarios) { + const assertPrivateFieldMethodsOperation = + scenario.snapshotName === "openai-v6"; + describe(`openai sdk ${scenario.version}`, () => { defineOpenAIInstrumentationAssertions({ + assertPrivateFieldMethodsOperation, name: "wrapped instrumentation", runScenario: async ({ runScenarioDir }) => { await runScenarioDir({ From 88fbd0869ea902d242b8db630369f7d423cd59e6 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 30 Mar 2026 13:27:36 +0200 Subject: [PATCH 2/4] hacky fix iteration one --- js/src/wrappers/oai.test.ts | 14 ++++++++++++++ js/src/wrappers/oai.ts | 38 +++++++++++++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/js/src/wrappers/oai.test.ts b/js/src/wrappers/oai.test.ts index 4ccb6f3c3..cde312a47 100644 --- a/js/src/wrappers/oai.test.ts +++ b/js/src/wrappers/oai.test.ts @@ -35,6 +35,20 @@ test("openai is installed", () => { assert.ok(OpenAI); }); +test("wrapOpenAI keeps private-field helper methods callable", async () => { + const wrapped = wrapOpenAI(new OpenAI({ apiKey: "test-key" })); + + expect(wrapped.buildURL).toBe(wrapped.buildURL); + expect(wrapped.buildRequest).toBe(wrapped.buildRequest); + expect(() => wrapped.buildURL("/files", null)).not.toThrow(); + + const request = await wrapped.buildRequest( + { method: "post", path: "/files" }, + { retryCount: 0 }, + ); + expect(request.url).toContain("/files"); +}); + describe("openai client unit tests", TEST_SUITE_OPTIONS, () => { let oai: OpenAI; let client: OpenAI; diff --git a/js/src/wrappers/oai.ts b/js/src/wrappers/oai.ts index 6411020b8..1a2b65af5 100644 --- a/js/src/wrappers/oai.ts +++ b/js/src/wrappers/oai.ts @@ -74,6 +74,12 @@ type OpenAILike = OpenAIV4Client; export function wrapOpenAIv4(openai: T): T { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions const typedOpenai = openai as OpenAIV4Client; + // Recover `this` for fallback methods so private fields and internal slots + // keep seeing the original OpenAI instance instead of the proxy. + const fallbackMethodCache = new WeakMap< + (...args: unknown[]) => unknown, + (...args: unknown[]) => unknown + >(); const completionProxy = new Proxy(typedOpenai.chat.completions, { get(target, name, receiver) { @@ -142,8 +148,8 @@ export function wrapOpenAIv4(openai: T): T { } // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - return new Proxy(typedOpenai, { - get(target, name, receiver) { + const topLevelProxy = new Proxy(typedOpenai, { + get(target, name) { switch (name) { case "chat": return chatProxy; @@ -158,9 +164,33 @@ export function wrapOpenAIv4(openai: T): T { if (name === "beta" && betaProxy) { return betaProxy; } - return Reflect.get(target, name, receiver); + + const baseVal = Reflect.get(target, name, target); + if (typeof baseVal !== "function") { + return baseVal; + } + + const typedBaseVal = baseVal as (...args: unknown[]) => unknown; + const cached = fallbackMethodCache.get(typedBaseVal); + if (cached) { + return cached; + } + + const recoveredThisMethod = function ( + this: unknown, + ...args: unknown[] + ): unknown { + const thisArg = this === topLevelProxy ? target : this; + const output = Reflect.apply(typedBaseVal, thisArg, args); + return output === target ? topLevelProxy : output; + }; + + fallbackMethodCache.set(typedBaseVal, recoveredThisMethod); + return recoveredThisMethod; }, - }) as T; + }); + + return topLevelProxy as T; } type SpanInfo = { From bfc1ecb377ae82babd04a5fcbe1d433b99fc5e7b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 31 Mar 2026 11:32:12 +0200 Subject: [PATCH 3/4] Add comments --- js/src/wrappers/oai.ts | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/js/src/wrappers/oai.ts b/js/src/wrappers/oai.ts index 1a2b65af5..dc8d89c1a 100644 --- a/js/src/wrappers/oai.ts +++ b/js/src/wrappers/oai.ts @@ -76,7 +76,7 @@ export function wrapOpenAIv4(openai: T): T { const typedOpenai = openai as OpenAIV4Client; // Recover `this` for fallback methods so private fields and internal slots // keep seeing the original OpenAI instance instead of the proxy. - const fallbackMethodCache = new WeakMap< + const privateMethodWorkaroundCache = new WeakMap< (...args: unknown[]) => unknown, (...args: unknown[]) => unknown >(); @@ -165,28 +165,39 @@ export function wrapOpenAIv4(openai: T): T { return betaProxy; } - const baseVal = Reflect.get(target, name, target); - if (typeof baseVal !== "function") { - return baseVal; + // The following rather convoluted code is a workaround for https://github.com/braintrustdata/braintrust-sdk-javascript/issues/1693 + // The problem is that Proxies are inherently difficult to work with native private class fields because when a + // class function accesses a private field, JS checks whether `this` is equal to the actual instance with the + // private field and if that's not the case, it throws a `TypeError`. + // We could have also done `if (typeof value === "function") return value.bind(target);`, but it would have + // created a new function on each function access, so we are caching, and also it would have always stomped on + // someone passing another `this` which may clash with different instrumentations. + + // Use the real client as receiver when reading fallback members. + const value = Reflect.get(target, name, target); + if (typeof value !== "function") { + return value; } - const typedBaseVal = baseVal as (...args: unknown[]) => unknown; - const cached = fallbackMethodCache.get(typedBaseVal); - if (cached) { - return cached; + const cachedValue = privateMethodWorkaroundCache.get(value); + if (cachedValue) { + return cachedValue; } - const recoveredThisMethod = function ( + const thisBoundValue = function ( this: unknown, ...args: unknown[] ): unknown { + // Calling through the proxy would set `this` to the proxy and break + // native private-field methods, so recover the original target. const thisArg = this === topLevelProxy ? target : this; - const output = Reflect.apply(typedBaseVal, thisArg, args); + const output = Reflect.apply(value, thisArg, args); + // Preserve chaining on wrapped clients (method returns `this`). return output === target ? topLevelProxy : output; }; - fallbackMethodCache.set(typedBaseVal, recoveredThisMethod); - return recoveredThisMethod; + privateMethodWorkaroundCache.set(value, thisBoundValue); + return thisBoundValue; }, }); From bcca50f0a0e2761c09c82dec42d74867a8bd0050 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 31 Mar 2026 17:20:05 +0200 Subject: [PATCH 4/4] address review --- e2e/scenarios/openai-instrumentation/scenario.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/e2e/scenarios/openai-instrumentation/scenario.test.ts b/e2e/scenarios/openai-instrumentation/scenario.test.ts index f9bc438ae..770177579 100644 --- a/e2e/scenarios/openai-instrumentation/scenario.test.ts +++ b/e2e/scenarios/openai-instrumentation/scenario.test.ts @@ -14,12 +14,14 @@ const openaiScenarios = await Promise.all( [ { autoEntry: "scenario.openai-v4.mjs", + disablePrivateFieldMethodsAssertion: true, dependencyName: "openai-v4", snapshotName: "openai-v4", wrapperEntry: "scenario.openai-v4.ts", }, { autoEntry: "scenario.openai-v5.mjs", + disablePrivateFieldMethodsAssertion: true, dependencyName: "openai-v5", snapshotName: "openai-v5", wrapperEntry: "scenario.openai-v5.ts", @@ -41,7 +43,7 @@ const openaiScenarios = await Promise.all( for (const scenario of openaiScenarios) { const assertPrivateFieldMethodsOperation = - scenario.snapshotName === "openai-v6"; + !scenario.disablePrivateFieldMethodsAssertion; describe(`openai sdk ${scenario.version}`, () => { defineOpenAIInstrumentationAssertions({