Skip to content

throw appropriate error for non-byob stream mode #5748

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
anonrig merged 1 commit into from
Dec 24, 2025
Merged

throw appropriate error for non-byob stream mode #5748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/workerd/api/streams/readable.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ ReadableStream::Reader ReadableStream::getReader(
KJ_IF_SOME(o, options) {
KJ_IF_SOME(mode, o.mode) {
JSG_REQUIRE(
mode == "byob", RangeError, "mode must be undefined or 'byob' in call to getReader().");
mode == "byob", TypeError, "mode must be undefined or 'byob' in call to getReader().");
// No need to check that the ReadableStream implementation is a byte stream: the first
// invocation of read() will do that for us and throw if necessary. Also, we should really
// just support reading non-byte streams with BYOB readers.
Expand Down
6 changes: 0 additions & 6 deletions src/wpt/streams-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ export default {
'readable-byte-streams/templated.any.js': {
comment: 'To be investigated',
expectedFailures: [
'ReadableStream with byte source (empty): calling getReader with invalid arguments should throw appropriate errors',
'ReadableStream with byte source (empty) BYOB reader: canceling via the reader should cause the reader to act closed',
],
},
Expand Down Expand Up @@ -438,8 +437,6 @@ export default {
// TODO(conform): The spec allows error to be called with no argument at all, treating
// it as undefined, currently we require that undefined is passed explicitly.
'ReadableStreamDefaultReader closed promise should be rejected with undefined if that is the error',
// TODO(conform): The spec expects this to be a TypeError, not a RangeError
'getReader() should call ToString() on mode',
],
},
'readable-streams/floating-point-total-queue-size.any.js': {
Expand Down Expand Up @@ -484,8 +481,6 @@ export default {
"ReadableStream can't be constructed with garbage",
// TODO(conform): We currently allow the empty type value
"ReadableStream can't be constructed with an invalid type",
// TODO(conform): The spec expects a TypeError here, not a RangeError
'default ReadableStream getReader() should only accept mode:undefined',
// TODO(conform): The spec expects us to call pull an extra time here despite. [Despite what? -NP]
'ReadableStream: should pull after start, and after every read',
// TODO(conform): The standard generally anticipates that the closed
Expand Down Expand Up @@ -581,7 +576,6 @@ export default {
'ReadableStream reader (closed via cancel after getting reader): closed should fulfill with undefined',
],
expectedFailures: [
'ReadableStream (empty): calling getReader with invalid arguments should throw appropriate errors',
'ReadableStream reader (closed before getting reader): releasing the lock should cause closed to reject and change identity',
'ReadableStream reader (closed after getting reader): releasing the lock should cause closed to reject and change identity',
'ReadableStream reader (closed via cancel after getting reader): releasing the lock should cause closed to reject and change identity',
Expand Down