Skip to content

Commit acadd48

Browse files
committed
refactor(core): replace class #private with WeakMap to avoid brand-check crashes
- Replace JS private fields with module-scoped WeakMaps on hot paths: - core/api-promise.ts (APIPromise ↔ client) - core/pagination.ts (AbstractPage ↔ client) - core/streaming.ts (Stream ↔ client; tee preserves client) - client.ts (replace #encoder with private field; private baseURLOverridden) - Add minimal inline rationale comments; no issue links in code. - Remove temporary repros under tests/temp/. - No public API changes.
1 parent af7366f commit acadd48

File tree

4 files changed

+37
-18
lines changed

4 files changed

+37
-18
lines changed

src/client.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,9 @@ export class OpenAI {
329329
fetchOptions: MergedRequestInit | undefined;
330330

331331
private fetch: Fetch;
332-
#encoder: Opts.RequestEncoder;
332+
// Use a normal private field instead of JS #private to avoid
333+
// brand-check crashes when methods are invoked across copies.
334+
private encoder: Opts.RequestEncoder;
333335
protected idempotencyHeader?: string;
334336
protected _options: ClientOptions;
335337

@@ -391,7 +393,7 @@ export class OpenAI {
391393
this.fetchOptions = options.fetchOptions;
392394
this.maxRetries = options.maxRetries ?? 2;
393395
this.fetch = options.fetch ?? Shims.getDefaultFetch();
394-
this.#encoder = Opts.FallbackEncoder;
396+
this.encoder = Opts.FallbackEncoder;
395397

396398
this._options = options;
397399

@@ -426,7 +428,7 @@ export class OpenAI {
426428
/**
427429
* Check whether the base URL is set to its default.
428430
*/
429-
#baseURLOverridden(): boolean {
431+
private baseURLOverridden(): boolean {
430432
return this.baseURL !== 'https://api.openai.com/v1';
431433
}
432434

@@ -493,7 +495,7 @@ export class OpenAI {
493495
query: Record<string, unknown> | null | undefined,
494496
defaultBaseURL?: string | undefined,
495497
): string {
496-
const baseURL = (!this.#baseURLOverridden() && defaultBaseURL) || this.baseURL;
498+
const baseURL = (!this.baseURLOverridden() && defaultBaseURL) || this.baseURL;
497499
const url =
498500
isAbsoluteURL(path) ?
499501
new URL(path)
@@ -959,7 +961,7 @@ export class OpenAI {
959961
) {
960962
return { bodyHeaders: undefined, body: Shims.ReadableStreamFrom(body as AsyncIterable<Uint8Array>) };
961963
} else {
962-
return this.#encoder({ body, headers });
964+
return this.encoder({ body, headers });
963965
}
964966
}
965967

src/core/api-promise.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// File generated from our OpenAPI spec by Stainless. See CONTRIBUTING.md for details.
22

33
import { type OpenAI } from '../client';
4+
import { OpenAIError } from './error';
45

56
import { type PromiseOrValue } from '../internal/types';
67
import {
@@ -14,9 +15,12 @@ import {
1415
* A subclass of `Promise` providing additional helper methods
1516
* for interacting with the SDK.
1617
*/
18+
// Associate instance with client via a module WeakMap to avoid
19+
// JS private-field brand checks across bundles/copies.
20+
const apiPromiseClient = /* @__PURE__ */ new WeakMap<object, OpenAI>();
21+
1722
export class APIPromise<T> extends Promise<WithRequestID<T>> {
1823
private parsedPromise: Promise<WithRequestID<T>> | undefined;
19-
#client: OpenAI;
2024

2125
constructor(
2226
client: OpenAI,
@@ -32,11 +36,13 @@ export class APIPromise<T> extends Promise<WithRequestID<T>> {
3236
// to parse the response
3337
resolve(null as any);
3438
});
35-
this.#client = client;
39+
apiPromiseClient.set(this, client);
3640
}
3741

3842
_thenUnwrap<U>(transform: (data: T, props: APIResponseProps) => U): APIPromise<U> {
39-
return new APIPromise(this.#client, this.responsePromise, async (client, props) =>
43+
const client = apiPromiseClient.get(this);
44+
if (!client) throw new OpenAIError('Illegal invocation of APIPromise method');
45+
return new APIPromise(client, this.responsePromise, async (client, props) =>
4046
addRequestID(transform(await this.parseResponse(client, props), props), props.response),
4147
);
4248
}
@@ -75,8 +81,10 @@ export class APIPromise<T> extends Promise<WithRequestID<T>> {
7581

7682
private parse(): Promise<WithRequestID<T>> {
7783
if (!this.parsedPromise) {
84+
const client = apiPromiseClient.get(this);
85+
if (!client) throw new OpenAIError('Illegal invocation of APIPromise method');
7886
this.parsedPromise = this.responsePromise.then((data) =>
79-
this.parseResponse(this.#client, data),
87+
this.parseResponse(client, data),
8088
) as any as Promise<WithRequestID<T>>;
8189
}
8290
return this.parsedPromise;

src/core/pagination.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,18 @@ import { maybeObj } from '../internal/utils/values';
1010

1111
export type PageRequestOptions = Pick<FinalRequestOptions, 'query' | 'headers' | 'body' | 'path' | 'method'>;
1212

13+
// Associate pages with their client via a module WeakMap to avoid
14+
// JS private-field brand checks across bundles/copies.
15+
const pageClient = /* @__PURE__ */ new WeakMap<object, OpenAI>();
16+
1317
export abstract class AbstractPage<Item> implements AsyncIterable<Item> {
14-
#client: OpenAI;
1518
protected options: FinalRequestOptions;
1619

1720
protected response: Response;
1821
protected body: unknown;
1922

2023
constructor(client: OpenAI, response: Response, body: unknown, options: FinalRequestOptions) {
21-
this.#client = client;
24+
pageClient.set(this, client);
2225
this.options = options;
2326
this.response = response;
2427
this.body = body;
@@ -42,7 +45,9 @@ export abstract class AbstractPage<Item> implements AsyncIterable<Item> {
4245
);
4346
}
4447

45-
return await this.#client.requestAPIList(this.constructor as any, nextOptions);
48+
const client = pageClient.get(this);
49+
if (!client) throw new OpenAIError('Illegal invocation of Page method');
50+
return await client.requestAPIList(this.constructor as any, nextOptions);
4651
}
4752

4853
async *iterPages(): AsyncGenerator<this> {

src/core/streaming.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,20 @@ export type ServerSentEvent = {
1818
raw: string[];
1919
};
2020

21+
// Associate Stream instances with their client via a module WeakMap to avoid
22+
// JS private-field brand checks across bundles/copies.
23+
const streamClient = /* @__PURE__ */ new WeakMap<object, OpenAI | undefined>();
24+
2125
export class Stream<Item> implements AsyncIterable<Item> {
2226
controller: AbortController;
23-
#client: OpenAI | undefined;
2427

2528
constructor(
2629
private iterator: () => AsyncIterator<Item>,
2730
controller: AbortController,
2831
client?: OpenAI,
2932
) {
3033
this.controller = controller;
31-
this.#client = client;
34+
streamClient.set(this, client);
3235
}
3336

3437
static fromSSEResponse<Item>(
@@ -75,8 +78,8 @@ export class Stream<Item> implements AsyncIterable<Item> {
7578
try {
7679
data = JSON.parse(sse.data);
7780
} catch (e) {
78-
console.error(`Could not parse message into JSON:`, sse.data);
79-
console.error(`From chunk:`, sse.raw);
81+
logger.error(`Could not parse message into JSON:`, sse.data);
82+
logger.error(`From chunk:`, sse.raw);
8083
throw e;
8184
}
8285
// TODO: Is this where the error should be thrown?
@@ -177,9 +180,10 @@ export class Stream<Item> implements AsyncIterable<Item> {
177180
};
178181
};
179182

183+
const client = streamClient.get(this);
180184
return [
181-
new Stream(() => teeIterator(left), this.controller, this.#client),
182-
new Stream(() => teeIterator(right), this.controller, this.#client),
185+
new Stream(() => teeIterator(left), this.controller, client),
186+
new Stream(() => teeIterator(right), this.controller, client),
183187
];
184188
}
185189

0 commit comments

Comments
 (0)