Skip to content

streams: use Web IDL "a promise resolved with" semantics for callback results - #32620

Closed
alii wants to merge 8 commits into
mainfrom
ali/streams-webidl-promise-resolved-with
Closed

streams: use Web IDL "a promise resolved with" semantics for callback results#32620
alii wants to merge 8 commits into
mainfrom
ali/streams-webidl-promise-resolved-with

streams: return raw start() result from underlying-sink startAlgorithm

6d74418
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jun 23, 2026 in 39m 52s

Code review found 1 important issue

Found 4 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/js/builtins/StreamInternals.ts:48-57 shieldingPromiseResolve $enqueueJob job not tamper-proof: throws inside r.$then(...) hang the wrapper instead of rejecti

Annotations

Check failure on line 57 in src/js/builtins/StreamInternals.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

shieldingPromiseResolve $enqueueJob job not tamper-proof: throws inside r.$then(...) hang the wrapper instead of rejecting

🔴 The `$enqueueJob` body is not as tamper-proof as the comment claims: `r.$then(...)` is a prototype-chain lookup of the private `@then` (the very guard this PR removes acknowledged it can be `undefined`), and even when found, intrinsic `then` still runs `SpeciesConstructor(r)` — so a callback returning a native promise with `Object.setPrototypeOf(p, null)` or `p.constructor = { get [Symbol.species]() { throw … } }` throws inside the microtask job, the fresh `promise` is never settled, and the s