fix(core): prevent private-field brand-check runtime crashes#1648
Open
SkinnnyJay wants to merge 6 commits intoopenai:masterfrom
Open
fix(core): prevent private-field brand-check runtime crashes#1648SkinnnyJay wants to merge 6 commits intoopenai:masterfrom
SkinnnyJay wants to merge 6 commits intoopenai:masterfrom
Conversation
1 task
…eck 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.
- APIPromise: detached then/catch/finally do not throw private-field TypeError - Pagination: AbstractPage.getNextPage guarded under detachment - Streaming: Stream.tee remains stable under detachment
SkinnnyJay
force-pushed
the
bug-1590
branch
from
ab81606 to
370ab76
Compare
Author
|
Ready for review.. |
|
Could you take a look at this fix please...? |
|
+1 |
mariocandela
approved these changes
Nov 1, 2025
Author
|
Happy to help, @Pierre-Assemat @mariocandela. Just to confirm - are you referring to reviewing the current fix in this PR, or reporting a new issue caused by it? If it’s the latter, could you share a minimal example and the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes being requested
Replace class private fields with module-scoped
WeakMapsto prevent runtime crashes caused by JavaScript private field brand checks across duplicate SDK installations.Core changes:
#privatefields withWeakMap-basedencapsulation in core componentscore/api-promise.ts,core/pagination.ts, andcore/streaming.tsWeakMapapproachTest coverage:
cross-copymethod compatibility works correctlyAdditional context & links
Bug: #1590
This addresses
"TypeError: Cannot read private member"runtime crashes that occur when methods from different SDK copies interact, particularly in environments with duplicate SDK installations.Problem:
JavaScript private field brand checks fail when instances cross module boundaries between different SDK copies, causing runtime errors in production environments.
Solution:
WeakMaps provide the same encapsulation benefits as private fields, but don't enforce brand checks, allowing instances from different SDK copies to interoperate safely.
Impact:
✅ I understand that this repository is auto-generated, and my pull request may not be merged