From e56ff79f4db27ab5f43e50f02119f7f1d0a6207d Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 11 Sep 2026 22:23:52 +0000 Subject: [PATCH] Pin error() during a pending close in the readable stream suites After close() with a chunk still queued the stream has only requested its close and, per spec, is still readable, so a later error() errors it and discards the queue; the TypeScript implementation does that, while the C++ implementation treats the requested close as final, ignores the late error and drains the chunk to a clean close (and, for byte streams, already reports desiredSize as 0 rather than the high-water mark minus the queued bytes). Both suites pinned error() after a completed close only; pin the pending-close ordering on each side and record it in the ledgers. --- src/tests/streams/readable-byte/AGENTS.md | 3 +- src/tests/streams/readable-byte/controller.js | 42 +++++++++++++++++++ src/tests/streams/readable-byte/main.js | 1 + src/tests/streams/readable/AGENTS.md | 6 ++- src/tests/streams/readable/controller.js | 40 +++++++++++++++++- src/tests/streams/readable/main.js | 1 + 6 files changed, 88 insertions(+), 5 deletions(-) diff --git a/src/tests/streams/readable-byte/AGENTS.md b/src/tests/streams/readable-byte/AGENTS.md index 6c9f9f02d72..c22c2986fa1 100644 --- a/src/tests/streams/readable-byte/AGENTS.md +++ b/src/tests/streams/readable-byte/AGENTS.md @@ -37,6 +37,7 @@ behavior-parity (messages aside). | 20 | remainder after a partial BYOB read, delivered to a DEFAULT read | copied into a fresh auto-allocated buffer (4096, or 16384 under the ledger #5 autogate), byteOffset 0 | view into the original enqueued buffer with its offset preserved (spec) | `partialViewThenDefaultRead` | | 21 | byobRequest after a PARTIAL enqueue into a pending BYOB read | original request invalidated; no replacement exposed (null) | original request invalidated; a fresh request exposes a view shrunk to the remaining byte count (spec) | `cancelWithPartiallyFilledPull` | | 22 | cancel() after a partial enqueue into a pending BYOB read | read resolves done with an empty view | read resolves done with value undefined (spec) | `cancelWithPartiallyFilledPull` | +| 23 | error() while a close() is still pending (bytes queued) — readable #18 mirror | ignored: desiredSize already 0, the bytes drain to a clean close for default and BYOB readers | desiredSize is hwm minus the queued bytes (-2) until the error, then the stream errors: bytes discarded, default/BYOB reads and closed reject | `errorAfterCloseWithQueuedBytes` | Parity worth noting (probed, pinned): byte hwm defaults to 0 with NO automatic pull; pull-throw and error-then-throw identity; enqueue @@ -105,7 +106,7 @@ named suite test pins directly, differing only in incidental asserts. | --- | --- | | `construction.js` | ledger #1, #2, #4; byte hwm default 0 | | `pull-timing.js` | ledger #3; pull-throw seeds | -| `controller.js` | ledger #5, #7, #21, #22; enqueue-discards-request; read-after-close; detach-at-call | +| `controller.js` | ledger #5, #7, #21, #22, #23; enqueue-discards-request; read-after-close; detach-at-call | | `byob-reader.js` | ledger #20; view-type matrix + offsets + auto-allocate sizing (migrated streams-byob-edge-cases) + mismatched sizes/types, subarray, multi-pending-reads, byobreaderRegression (migrated streams-js-test) | | `respond.js` | ledger #6, #8, #15, #16; all 31 streams-respond-test tests (respond/respondWithNewView/pumps/cancel races/UAF shapes) + js-test respond family | | `release-relock.js` | ledger #9, #10; the WPT releaseLock→second-reader cluster | diff --git a/src/tests/streams/readable-byte/controller.js b/src/tests/streams/readable-byte/controller.js index d8d33114864..1c2df699cf0 100644 --- a/src/tests/streams/readable-byte/controller.js +++ b/src/tests/streams/readable-byte/controller.js @@ -180,6 +180,48 @@ export const closeWithPendingUnfilledByobRead = { }, }; +// DIVERGENCE (ledger #23, the byte mirror of readable #18): error() while +// a close() is still pending (bytes queued). Per spec the stream is still +// "readable", so the TypeScript implementation reports desiredSize as +// hwm minus the queued bytes (-2) before the error, then errors the +// stream: the queued bytes are discarded and default and BYOB reads, and +// closed, reject with the error. The C++ implementation treats the +// requested close as final: desiredSize is already 0, the late error is +// ignored and the bytes drain to a clean close. +export const errorAfterCloseWithQueuedBytes = { + async test() { + const err = new Error('late'); + for (const mode of [undefined, 'byob']) { + let controller; + const rs = new ReadableStream({ + type: 'bytes', + start(c) { + controller = c; + }, + }); + controller.enqueue(new Uint8Array([1, 2])); + controller.close(); + strictEqual(controller.desiredSize, usingTsImpl ? -2 : 0); + controller.error(err); + const reader = rs.getReader({ mode }); + const read = () => + mode === 'byob' ? reader.read(new Uint8Array(4)) : reader.read(); + if (usingTsImpl) { + strictEqual(controller.desiredSize, null); + strictEqual(await rejectionOf(read()), err); + strictEqual(await rejectionOf(reader.closed), err); + } else { + strictEqual(controller.desiredSize, 0); + const first = await read(); + strictEqual(first.done, false); + strictEqual(first.value.byteLength, 2); + strictEqual((await read()).done, true); + await reader.closed; + } + } + }, +}; + export const controllerType = { async test() { let c; diff --git a/src/tests/streams/readable-byte/main.js b/src/tests/streams/readable-byte/main.js index 174fe4c8c9a..3cd7fff0b06 100644 --- a/src/tests/streams/readable-byte/main.js +++ b/src/tests/streams/readable-byte/main.js @@ -52,6 +52,7 @@ export { readDetachesCallerBuffer, closeWithPendingUnfilledByobRead, controllerType, + errorAfterCloseWithQueuedBytes, cancelWithPartiallyFilledPull, readViewThenCancelOrdering, } from 'controller'; diff --git a/src/tests/streams/readable/AGENTS.md b/src/tests/streams/readable/AGENTS.md index a77e7ab7929..c266c38bb82 100644 --- a/src/tests/streams/readable/AGENTS.md +++ b/src/tests/streams/readable/AGENTS.md @@ -32,10 +32,12 @@ behavioral gaps; the reentrancy family is mostly parity at finite hwm. | 15 | adopted body stream lock after consumption | released | kept locked | `bodyIdentityAndLockCoupling` | | 16 | then-getter fires during a read cycle | 1 (harness context) | 2 | `thenGetterFireCountOnRead` | | 17 | close-twice / enqueue-after-close / size-not-function / from-return validation messages | own texts | own texts | `closeTerminality`, `sizeMustBeFunction`, `fromReturnValidationMessages` | +| 18 | error() while a close() is still pending (chunk queued) | ignored: the requested close is final, the chunk drains to a clean close, desiredSize stays 0 | errors the stream (spec: close() only requested the close, the state is still "readable"): chunk discarded, reads/closed reject, desiredSize null | `errorAfterCloseWithQueuedChunk` | Parity worth noting (probed, pinned): pull serialization (never re-entered); pull/async-start rejection identity; error-undefined -preserved through closed; error() twice / after close are no-ops; +preserved through closed; error() twice / after a completed close are +no-ops (a close still pending is ledger #18); desiredSize lifecycle (1 → 0 close, null error, 0 cancel) and enqueue-skips-queue-with-pending-read; cancel-with-pending-pull; cancel reason identity + once; locked-stream cancel rejects without running the @@ -74,7 +76,7 @@ C++ implementation; `draining-reader.js` asserts both sides. | `api-surface.js` | globals, controller not constructable, getReader modes, locked lifecycle | | `construction.js` | ledger #1-#3, default hwm 1 | | `source-algorithms.js` | ledger #4-#6, pull serialization, rejection identity, cancel-with-pending-pull | -| `controller.js` | desiredSize accounting/terminal states, error idempotence, close terminality (#17), close-drains-queue | +| `controller.js` | desiredSize accounting/terminal states, error idempotence, close terminality (#17), close-drains-queue, error during a pending close (#18) | | `reader.js` | read ordering, releaseLock, closed replacement (#7), undefined error, reader.cancel, reader swap | | `cancel.js` | reason identity, locked-cancel, hook rejection identity, queue discard | | `bad-strategies.js` | ledger #8, #9, size-not-function | diff --git a/src/tests/streams/readable/controller.js b/src/tests/streams/readable/controller.js index 17c96b71b88..7ed8f490302 100644 --- a/src/tests/streams/readable/controller.js +++ b/src/tests/streams/readable/controller.js @@ -113,8 +113,10 @@ export const controllerErrorRejectsReads = { }, }; -// error() twice and error() after close are silent no-ops on both sides -// (the WPT bad-underlying-sources seeds fail on other grounds). +// error() twice and error() after a completed close (empty queue, so the +// stream has closed) are silent no-ops on both sides (the WPT +// bad-underlying-sources seeds fail on other grounds). For error() while +// a close is still pending see errorAfterCloseWithQueuedChunk. export const errorIdempotence = { test() { let c1; @@ -180,6 +182,40 @@ export const closeDrainsQueue = { }, }; +// DIVERGENCE (ledger #18): error() while a close() is still pending (a +// chunk queued). Per spec the stream is still "readable" — close() only +// requested the close — so the TypeScript implementation errors it: the +// queued chunk is discarded, reads and closed reject with the error and +// desiredSize turns null. The C++ implementation treats the requested +// close as final: the late error is ignored, the chunk drains to a clean +// close, and desiredSize stays 0. error() throws on neither side. +export const errorAfterCloseWithQueuedChunk = { + async test() { + const err = new Error('late'); + let controller; + const rs = new ReadableStream({ + start(c) { + controller = c; + }, + }); + controller.enqueue('a'); + controller.close(); + strictEqual(controller.desiredSize, 0); + controller.error(err); + const reader = rs.getReader(); + if (usingTsImpl) { + strictEqual(controller.desiredSize, null); + strictEqual(await rejectionOf(reader.read()), err); + strictEqual(await rejectionOf(reader.closed), err); + } else { + strictEqual(controller.desiredSize, 0); + strictEqual((await reader.read()).value, 'a'); + strictEqual((await reader.read()).done, true); + await reader.closed; + } + }, +}; + export const controllerType = { async test() { let c; diff --git a/src/tests/streams/readable/main.js b/src/tests/streams/readable/main.js index 55cb5518237..43e816f8bd2 100644 --- a/src/tests/streams/readable/main.js +++ b/src/tests/streams/readable/main.js @@ -52,6 +52,7 @@ export { errorIdempotence, closeTerminality, closeDrainsQueue, + errorAfterCloseWithQueuedChunk, controllerType, } from 'controller';