From 4e5c011d953c92d1fcb0a1e2f60d950116d07bf1 Mon Sep 17 00:00:00 2001 From: Rola Abuhasna Date: Fri, 25 Jul 2025 08:47:30 +0200 Subject: [PATCH] fix(core): Fix OpenAI SDK private field access by binding non-instrumented fns (#17163) The OpenAI SDK v5 uses ES private class fields (e.g. #baseURL), which can only be accessed when methods are called with the correct this context. In our instrumentation, methods that are not explicitly instrumented (i.e. skipped by shouldInstrument) were returned unbound, which broke calls to internal OpenAI methods like .parse() by triggering: TypeError: Cannot read private member from an object whose class did not declare it. This PR fixes that by explicitly binding all non-instrumented functions to their original instance (value.bind(obj)), ensuring correct this context and avoiding runtime errors when accessing private fields. (cherry picked from commit 8b2c6851758eb1e3f8fc63af87407c188595e7ad) --- packages/core/src/utils/openai/index.ts | 6 + .../openai-integration-functions.test.ts | 210 ++++++++++++++++++ 2 files changed, 216 insertions(+) create mode 100644 packages/core/test/utils/openai-integration-functions.test.ts diff --git a/packages/core/src/utils/openai/index.ts b/packages/core/src/utils/openai/index.ts index 2b5fdbef9c11..9bab70c2ae7c 100644 --- a/packages/core/src/utils/openai/index.ts +++ b/packages/core/src/utils/openai/index.ts @@ -264,6 +264,12 @@ function createDeepProxy(target: object, currentPath = '', options?: OpenAiOptio return instrumentMethod(value as (...args: unknown[]) => Promise, methodPath, obj, options); } + if (typeof value === 'function') { + // Bind non-instrumented functions to preserve the original `this` context, + // which is required for accessing private class fields (e.g. #baseURL) in OpenAI SDK v5. + return value.bind(obj); + } + if (value && typeof value === 'object') { return createDeepProxy(value as object, methodPath, options); } diff --git a/packages/core/test/utils/openai-integration-functions.test.ts b/packages/core/test/utils/openai-integration-functions.test.ts new file mode 100644 index 000000000000..d032fb1e69ed --- /dev/null +++ b/packages/core/test/utils/openai-integration-functions.test.ts @@ -0,0 +1,210 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import type { OpenAiClient } from '../../src'; +import { instrumentOpenAiClient } from '../../src/utils/openai'; + +interface FullOpenAIClient { + chat: { + completions: { + create: (params: ChatCompletionParams) => Promise; + parse: (params: ParseCompletionParams) => Promise; + }; + }; +} +interface ChatCompletionParams { + model: string; + messages: Array<{ role: string; content: string }>; +} + +interface ChatCompletionResponse { + id: string; + model: string; + choices: Array<{ message: { content: string } }>; +} + +interface ParseCompletionParams { + model: string; + messages: Array<{ role: string; content: string }>; + response_format: { + type: string; + json_schema: { + name: string; + schema: { + type: string; + properties: Record; + }; + }; + }; +} + +interface ParseCompletionResponse { + id: string; + model: string; + choices: Array<{ + message: { + content: string; + parsed: { name: string; age: number }; + }; + }>; + parsed: { name: string; age: number }; +} + +/** + * Mock OpenAI client that simulates the private field behavior + * that causes the "Cannot read private member" error + */ +class MockOpenAIClient implements FullOpenAIClient { + // Simulate private fields using WeakMap (similar to how TypeScript private fields work) + static #privateData = new WeakMap(); + + // Simulate instrumented methods + chat = { + completions: { + create: async (params: ChatCompletionParams): Promise => { + this.#buildURL('/chat/completions'); + return { id: 'test', model: params.model, choices: [{ message: { content: 'Hello!' } }] }; + }, + + // This is NOT instrumented + parse: async (params: ParseCompletionParams): Promise => { + this.#buildURL('/chat/completions'); + return { + id: 'test', + model: params.model, + choices: [ + { + message: { + content: 'Hello!', + parsed: { name: 'John', age: 30 }, + }, + }, + ], + parsed: { name: 'John', age: 30 }, + }; + }, + }, + }; + + constructor() { + MockOpenAIClient.#privateData.set(this, { + apiKey: 'test-key', + baseURL: 'https://api.openai.com', + }); + } + + // Simulate the buildURL method that accesses private fields + #buildURL(path: string): string { + const data = MockOpenAIClient.#privateData.get(this); + if (!data) { + throw new TypeError('Cannot read private member from an object whose class did not declare it'); + } + return `${data.baseURL}${path}`; + } +} + +describe('OpenAI Integration Private Field Fix', () => { + let mockClient: MockOpenAIClient; + let instrumentedClient: FullOpenAIClient & OpenAiClient; + + beforeEach(() => { + mockClient = new MockOpenAIClient(); + instrumentedClient = instrumentOpenAiClient(mockClient as unknown as OpenAiClient) as FullOpenAIClient & + OpenAiClient; + }); + + it('should work with instrumented methods (chat.completions.create)', async () => { + // This should work because it's instrumented and we handle it properly + const result = await instrumentedClient.chat.completions.create({ + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }); + + expect(result.model).toBe('gpt-4'); + }); + + it('should work with non-instrumented methods without breaking private fields', async () => { + // The parse method should work now with our fix - previously it would throw: + // "TypeError: Cannot read private member from an object whose class did not declare it" + + await expect( + instrumentedClient.chat.completions.parse({ + model: 'gpt-4', + messages: [{ role: 'user', content: 'Extract name and age from: John is 30 years old' }], + response_format: { + type: 'json_schema', + json_schema: { + name: 'person', + schema: { + type: 'object', + properties: { + name: { type: 'string' }, + age: { type: 'number' }, + }, + }, + }, + }, + }), + ).resolves.toBeDefined(); + }); + + it('should preserve the original context for all method calls', async () => { + // Verify that 'this' context is preserved for instrumented methods + const createResult = await instrumentedClient.chat.completions.create({ + model: 'gpt-4', + messages: [{ role: 'user', content: 'test' }], + }); + + expect(createResult.model).toBe('gpt-4'); + + // Verify that 'this' context is preserved for non-instrumented methods + const parseResult = await instrumentedClient.chat.completions.parse({ + model: 'gpt-4', + messages: [{ role: 'user', content: 'Extract name and age from: John is 30 years old' }], + response_format: { + type: 'json_schema', + json_schema: { + name: 'person', + schema: { + type: 'object', + properties: { + name: { type: 'string' }, + age: { type: 'number' }, + }, + }, + }, + }, + }); + + expect(parseResult.parsed).toEqual({ name: 'John', age: 30 }); + }); + + it('should handle nested object access correctly', async () => { + expect(typeof instrumentedClient.chat.completions.create).toBe('function'); + expect(typeof instrumentedClient.chat.completions.parse).toBe('function'); + }); + + it('should work with non-instrumented methods', async () => { + const result = await instrumentedClient.chat.completions.parse({ + model: 'gpt-4', + messages: [{ role: 'user', content: 'Extract name and age from: John is 30 years old' }], + response_format: { + type: 'json_schema', + json_schema: { + name: 'person', + schema: { + type: 'object', + properties: { + name: { type: 'string' }, + age: { type: 'number' }, + }, + }, + }, + }, + }); + + expect(result.model).toBe('gpt-4'); + expect(result.parsed).toEqual({ name: 'John', age: 30 }); + + // Verify we can access the parse method without issues + expect(typeof instrumentedClient.chat.completions.parse).toBe('function'); + }); +});