Repository navigation
Conversation
Collaborator
Author
Collaborator
Author
|
Status
|
Collaborator
Author
|
Survey behind the numbers in the PR body: 179 wrong calls across the web stream classes, run on Node.js v26.3.0, Bun 1.3.14, a release build of main 11c3671 and a release build of this PR.
Results: call | Node.js | Bun 1.3.14 | main | this PRScript (prints one JSON line for each call)// Battery: erroneous calls across the web streams API. One JSON line per probe:
// {label, how, name, code, message}. Compare runtimes by label.
process.on('unhandledRejection', () => {});
const rows = [];
const P = (label, fn) => rows.push([label, fn]);
const rs = (src, strat) => new ReadableStream(src, strat);
const bytes = () => { let c; const s = new ReadableStream({ type: 'bytes', start(x) { c = x; } }); return { s, c }; };
const byobReader = () => { const { s, c } = bytes(); c.enqueue(new Uint8Array([7, 8])); return s.getReader({ mode: 'byob' }); };
const defCtl = () => { let c; const s = new ReadableStream({ start(x) { c = x; } }); return { s, c }; };
const wsCtl = () => { let c; const s = new WritableStream({ start(x) { c = x; } }); return { s, c }; };
const tsCtl = () => { let c; const s = new TransformStream({ start(x) { c = x; } }); return { s, c }; };
// a byte stream with a pending BYOB request
const pendingByob = () => {
let c; const s = new ReadableStream({ type: 'bytes', start(x) { c = x; } });
const r = s.getReader({ mode: 'byob' });
const p = r.read(new Uint8Array(8)); p.catch(() => {});
return { s, c, r, req: c.byobRequest };
};
// ── argument errors ──
for (const [l, v] of [['none', '__none__'], ['undefined', undefined], ['null', null], ['5', 5], ["'x'", 'x'], ['{}', {}], ['[]', []], ['ArrayBuffer', new ArrayBuffer(1)]]) {
P(`byobReader.read(${l})`, () => v === '__none__' ? byobReader().read() : byobReader().read(v));
P(`byteController.enqueue(${l})`, () => v === '__none__' ? bytes().c.enqueue() : bytes().c.enqueue(v));
P(`byobRequest.respondWithNewView(${l})`, () => { const { req } = pendingByob(); return v === '__none__' ? req.respondWithNewView() : req.respondWithNewView(v); });
}
for (const [l, v] of [['none', '__none__'], ["'x'", 'x'], ['-1', -1], ['NaN', NaN], ['1e30', 1e30], ['{}', {}], ['9 (too many)', 9], ['0', 0]]) {
P(`byobRequest.respond(${l})`, () => { const { req } = pendingByob(); return v === '__none__' ? req.respond() : req.respond(v); });
}
P('byobRequest.respondWithNewView(larger view)', () => { const { req } = pendingByob(); return req.respondWithNewView(new Uint8Array(64)); });
P('byobRequest.respondWithNewView(view at other offset)', () => { const { req } = pendingByob(); return req.respondWithNewView(new Uint8Array(req.view.buffer, 2, 2)); });
P('byobRequest.respondWithNewView(zero-length, readable)', () => { const { req } = pendingByob(); return req.respondWithNewView(new Uint8Array(req.view.buffer, 0, 0)); });
P('byobReader.read(zero-length view)', () => byobReader().read(new Uint8Array(0)));
P('byobReader.read(view, 5)', () => byobReader().read(new Uint8Array(2), 5));
P('byobReader.read(view, {min:0})', () => byobReader().read(new Uint8Array(2), { min: 0 }));
P('byobReader.read(view, {min:9})', () => byobReader().read(new Uint8Array(2), { min: 9 }));
P('byobReader.read(view, {min:-1})', () => byobReader().read(new Uint8Array(2), { min: -1 }));
P("byobReader.read(view, {min:'x'})", () => byobReader().read(new Uint8Array(2), { min: 'x' }));
P('byteController.enqueue(zero-length view)', () => bytes().c.enqueue(new Uint8Array(0)));
P("rs.getReader('x')", () => rs().getReader('x'));
P('rs.getReader(5)', () => rs().getReader(5));
P("rs.getReader({mode:'x'})", () => rs().getReader({ mode: 'x' }));
P('rs.getReader({mode:5})', () => rs().getReader({ mode: 5 }));
P("rs.getReader({mode:'byob'}) on a default stream", () => rs().getReader({ mode: 'byob' }));
P('rs.pipeTo()', () => rs().pipeTo());
P('rs.pipeTo({})', () => rs().pipeTo({}));
P('rs.pipeTo(5)', () => rs().pipeTo(5));
P('rs.pipeTo(ws, 5)', () => rs().pipeTo(new WritableStream(), 5));
P('rs.pipeTo(ws, {signal:5})', () => rs().pipeTo(new WritableStream(), { signal: 5 }));
P('rs.pipeTo(ws, {signal:{}})', () => rs().pipeTo(new WritableStream(), { signal: {} }));
P('rs.pipeThrough()', () => rs().pipeThrough());
P('rs.pipeThrough(5)', () => rs().pipeThrough(5));
P('rs.pipeThrough({})', () => rs().pipeThrough({}));
P('rs.pipeThrough({readable:rs})', () => rs().pipeThrough({ readable: rs() }));
P('rs.pipeThrough({readable:{}, writable:ws})', () => rs().pipeThrough({ readable: {}, writable: new WritableStream() }));
P('rs.pipeThrough({readable:rs, writable:{}})', () => rs().pipeThrough({ readable: rs(), writable: {} }));
P('rs.pipeThrough(ts, 5)', () => rs().pipeThrough(new TransformStream(), 5));
P('rs.pipeThrough(ts, {signal:5})', () => rs().pipeThrough(new TransformStream(), { signal: 5 }));
P('rs.values(5)', () => rs().values(5));
P("rs.values('x')", () => rs().values('x'));
P('rs[Symbol.asyncIterator](5)', () => rs()[Symbol.asyncIterator](5));
// ── constructors ──
P('new ReadableStream(5)', () => new ReadableStream(5));
P('new ReadableStream(null)', () => new ReadableStream(null));
P("new ReadableStream({type:'x'})", () => new ReadableStream({ type: 'x' }));
P('new ReadableStream({}, 5)', () => new ReadableStream({}, 5));
P('new ReadableStream({pull:5})', () => new ReadableStream({ pull: 5 }));
P('new ReadableStream({start:5})', () => new ReadableStream({ start: 5 }));
P('new ReadableStream({cancel:5})', () => new ReadableStream({ cancel: 5 }));
P('new ReadableStream({}, {size:5})', () => new ReadableStream({}, { size: 5 }));
P('new ReadableStream({}, {highWaterMark:-1})', () => new ReadableStream({}, { highWaterMark: -1 }));
P('new ReadableStream({}, {highWaterMark:NaN})', () => new ReadableStream({}, { highWaterMark: NaN }));
P("new ReadableStream({type:'bytes'}, {size(){}})", () => new ReadableStream({ type: 'bytes' }, { size() {} }));
P("new ReadableStream({type:'bytes', autoAllocateChunkSize:0})", () => new ReadableStream({ type: 'bytes', autoAllocateChunkSize: 0 }));
P('new WritableStream(5)', () => new WritableStream(5));
P("new WritableStream({type:'x'})", () => new WritableStream({ type: 'x' }));
P('new WritableStream({write:5})', () => new WritableStream({ write: 5 }));
P('new WritableStream({start:5})', () => new WritableStream({ start: 5 }));
P('new WritableStream({close:5})', () => new WritableStream({ close: 5 }));
P('new WritableStream({abort:5})', () => new WritableStream({ abort: 5 }));
P('new WritableStream({}, 5)', () => new WritableStream({}, 5));
P('new WritableStream({}, {size:5})', () => new WritableStream({}, { size: 5 }));
P('new WritableStream({}, {highWaterMark:-1})', () => new WritableStream({}, { highWaterMark: -1 }));
P('new TransformStream(5)', () => new TransformStream(5));
P("new TransformStream({readableType:'x'})", () => new TransformStream({ readableType: 'x' }));
P("new TransformStream({writableType:'x'})", () => new TransformStream({ writableType: 'x' }));
P('new TransformStream({transform:5})', () => new TransformStream({ transform: 5 }));
P('new TransformStream({flush:5})', () => new TransformStream({ flush: 5 }));
P('new TransformStream({start:5})', () => new TransformStream({ start: 5 }));
P('new TransformStream({}, 5)', () => new TransformStream({}, 5));
P('new TransformStream({}, {}, 5)', () => new TransformStream({}, {}, 5));
P('new TransformStream({}, {highWaterMark:-1})', () => new TransformStream({}, { highWaterMark: -1 }));
P('new ReadableStreamDefaultReader()', () => new ReadableStreamDefaultReader());
P('new ReadableStreamDefaultReader(5)', () => new ReadableStreamDefaultReader(5));
P('new ReadableStreamDefaultReader({})', () => new ReadableStreamDefaultReader({}));
P('new ReadableStreamDefaultReader(locked)', () => { const s = rs(); s.getReader(); return new ReadableStreamDefaultReader(s); });
P('new ReadableStreamBYOBReader()', () => new ReadableStreamBYOBReader());
P('new ReadableStreamBYOBReader({})', () => new ReadableStreamBYOBReader({}));
P('new ReadableStreamBYOBReader(default stream)', () => new ReadableStreamBYOBReader(rs()));
P('new ReadableStreamBYOBReader(locked)', () => { const { s } = bytes(); s.getReader(); return new ReadableStreamBYOBReader(s); });
P('new WritableStreamDefaultWriter()', () => new WritableStreamDefaultWriter());
P('new WritableStreamDefaultWriter({})', () => new WritableStreamDefaultWriter({}));
P('new WritableStreamDefaultWriter(locked)', () => { const s = new WritableStream(); s.getWriter(); return new WritableStreamDefaultWriter(s); });
P('new ReadableStreamDefaultController()', () => new ReadableStreamDefaultController());
P('new ReadableByteStreamController()', () => new ReadableByteStreamController());
P('new ReadableStreamBYOBRequest()', () => new ReadableStreamBYOBRequest());
P('new WritableStreamDefaultController()', () => new WritableStreamDefaultController());
P('new TransformStreamDefaultController()', () => new TransformStreamDefaultController());
P('new ByteLengthQueuingStrategy()', () => new ByteLengthQueuingStrategy());
P('new ByteLengthQueuingStrategy(5)', () => new ByteLengthQueuingStrategy(5));
P('new ByteLengthQueuingStrategy({})', () => new ByteLengthQueuingStrategy({}));
P('new CountQueuingStrategy()', () => new CountQueuingStrategy());
P('new CountQueuingStrategy(5)', () => new CountQueuingStrategy(5));
P('new CountQueuingStrategy({})', () => new CountQueuingStrategy({}));
P("new CompressionStream('x')", () => new CompressionStream('x'));
P('new CompressionStream()', () => new CompressionStream());
P("new DecompressionStream('x')", () => new DecompressionStream('x'));
P('new DecompressionStream()', () => new DecompressionStream());
P("new TextDecoderStream('bad-label')", () => new TextDecoderStream('bad-label'));
P("new TextDecoderStream('utf-8', 5)", () => new TextDecoderStream('utf-8', 5));
P('ReadableStream()', () => ReadableStream());
P('WritableStream()', () => WritableStream());
P('TransformStream()', () => TransformStream());
// ── state errors ──
P('rs.cancel() locked', () => { const s = rs(); s.getReader(); return s.cancel(); });
P('rs.getReader() locked', () => { const s = rs(); s.getReader(); return s.getReader(); });
P('rs.tee() locked', () => { const s = rs(); s.getReader(); return s.tee(); });
P('rs.pipeTo(ws) locked', () => { const s = rs(); s.getReader(); return s.pipeTo(new WritableStream()); });
P('rs.pipeTo(locked ws)', () => { const w = new WritableStream(); w.getWriter(); return rs().pipeTo(w); });
P('rs.pipeThrough(ts) locked', () => { const s = rs(); s.getReader(); return s.pipeThrough(new TransformStream()); });
P('rs.pipeThrough(locked writable)', () => { const t = new TransformStream(); t.writable.getWriter(); return rs().pipeThrough(t); });
P('rs.values() locked', () => { const s = rs(); s.getReader(); return s.values(); });
P('reader.read() released', () => { const r = rs().getReader(); r.releaseLock(); return r.read(); });
P('reader.cancel() released', () => { const r = rs().getReader(); r.releaseLock(); return r.cancel(); });
P('reader.closed released', () => { const r = rs().getReader(); r.releaseLock(); return r.closed; });
P('reader.read() pending then released', () => { const r = rs().getReader(); const p = r.read(); r.releaseLock(); return p; });
P('byobReader.read() released', () => { const r = byobReader(); r.releaseLock(); return r.read(new Uint8Array(1)); });
P('byobReader.cancel() released', () => { const r = byobReader(); r.releaseLock(); return r.cancel(); });
P('byobReader.closed released', () => { const r = byobReader(); r.releaseLock(); return r.closed; });
P('byobReader.read() pending then released', () => { const { s } = bytes(); const r = s.getReader({ mode: 'byob' }); const p = r.read(new Uint8Array(1)); r.releaseLock(); return p; });
P('defController.enqueue() after close', () => { const { c } = defCtl(); c.close(); return c.enqueue(1); });
P('defController.close() twice', () => { const { c } = defCtl(); c.close(); return c.close(); });
P('defController.enqueue() after error', () => { const { c } = defCtl(); c.error(new Error('x')); return c.enqueue(1); });
P('defController.close() after error', () => { const { c } = defCtl(); c.error(new Error('x')); return c.close(); });
P('byteController.enqueue() after close', () => { const { c } = bytes(); c.close(); return c.enqueue(new Uint8Array(1)); });
P('byteController.close() twice', () => { const { c } = bytes(); c.close(); return c.close(); });
P('byteController.close() after error', () => { const { c } = bytes(); c.error(new Error('x')); return c.close(); });
P('byteController.enqueue() after error', () => { const { c } = bytes(); c.error(new Error('x')); return c.enqueue(new Uint8Array(1)); });
P('byteController.close() with partial pending pull', () => { const { c, req } = (() => { let c; const s = new ReadableStream({ type: 'bytes', start(x) { c = x; } }); const r = s.getReader({ mode: 'byob' }); r.read(new Uint16Array(2)).catch(() => {}); c.enqueue(new Uint8Array(1)); return { c }; })(); return c.close(); });
P('byobRequest.respond(1) after invalidated', () => { const { c, req } = pendingByob(); c.enqueue(new Uint8Array(8)); return req.respond(1); });
P('byobRequest.respondWithNewView() after invalidated', () => { const { c, req } = pendingByob(); c.enqueue(new Uint8Array(8)); return req.respondWithNewView(new Uint8Array(1)); });
P('byobRequest.respond(1) closed stream', () => { const { c, req } = pendingByob(); c.close(); return req.respond(1); });
P('byobRequest.respond(0) readable stream', () => { const { req } = pendingByob(); return req.respond(0); });
P('ws.abort() locked', () => { const w = new WritableStream(); w.getWriter(); return w.abort(); });
P('ws.close() locked', () => { const w = new WritableStream(); w.getWriter(); return w.close(); });
P('ws.close() twice', () => { const w = new WritableStream(); w.close(); return w.close(); });
P('ws.getWriter() locked', () => { const w = new WritableStream(); w.getWriter(); return w.getWriter(); });
P('writer.write() released', () => { const w = new WritableStream().getWriter(); w.releaseLock(); return w.write(1); });
P('writer.close() released', () => { const w = new WritableStream().getWriter(); w.releaseLock(); return w.close(); });
P('writer.abort() released', () => { const w = new WritableStream().getWriter(); w.releaseLock(); return w.abort(); });
P('writer.desiredSize released', () => { const w = new WritableStream().getWriter(); w.releaseLock(); return w.desiredSize; });
P('writer.ready released', () => { const w = new WritableStream().getWriter(); w.releaseLock(); return w.ready; });
P('writer.closed released', () => { const w = new WritableStream().getWriter(); w.releaseLock(); return w.closed; });
P('writer.close() twice', () => { const w = new WritableStream().getWriter(); w.close(); return w.close(); });
P('writer.write() after close', () => { const w = new WritableStream().getWriter(); w.close(); return w.write(1); });
P('writer.write() after abort', () => { const w = new WritableStream().getWriter(); w.abort(); return w.write(1); });
P('wsController.error() twice', () => { const { c } = wsCtl(); c.error(new Error('x')); return c.error(new Error('y')); });
P('tsController.enqueue() after terminate', () => { const { c } = tsCtl(); c.terminate(); return c.enqueue(1); });
P('tsController.enqueue() after error', () => { const { c } = tsCtl(); c.error(new Error('x')); return c.enqueue(1); });
P('tsController.terminate() twice', () => { const { c } = tsCtl(); c.terminate(); return c.terminate(); });
P('ts writable write after terminate', () => { const { s, c } = tsCtl(); c.terminate(); return s.writable.getWriter().write(1); });
P('ReadableStream.from()', () => ReadableStream.from());
P('ReadableStream.from(5)', () => ReadableStream.from(5));
P('ReadableStream.from({})', () => ReadableStream.from({}));
P('ReadableStream.from(null)', () => ReadableStream.from(null));
P('queuing strategy size() returns NaN', () => { let c; const s = new ReadableStream({ start(x) { c = x; } }, { size() { return NaN; } }); return c.enqueue(1); });
P('queuing strategy size() returns -1', () => { let c; const s = new ReadableStream({ start(x) { c = x; } }, { size() { return -1; } }); return c.enqueue(1); });
P('ws size() returns -1: writer.write', () => { const w = new WritableStream({}, { size() { return -1; } }).getWriter(); return w.write(1); });
const only = process.argv[2];
for (const [label, fn] of rows) {
if (only && !label.includes(only)) continue;
let how, e, v;
try {
v = fn();
if (v && typeof v.then === 'function') {
let timer;
const pending = new Promise(res => { timer = setTimeout(() => res('__pending__'), 150); });
try { const r = await Promise.race([v, pending]); how = r === '__pending__' ? 'pending' : 'fulfilled'; } catch (x) { how = 'rejects'; e = x; }
clearTimeout(timer);
} else how = 'returns';
} catch (x) { how = 'throws'; e = x; }
const isErr = e !== undefined;
console.log(JSON.stringify({ label, how, name: isErr ? (e && e.name) ?? typeof e : null, code: isErr ? (e && e.code) ?? null : null, message: isErr ? String((e && e.message) ?? e) : null }));
}
process.exit(0); |
Contributor
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Shared invalid-receiver errors src/jsc/bindings/ErrorCode.cpp, src/jsc/bindings/ErrorCode.h, src/jsc/bindings/webcore/streams/JSReadableStream*.cpp, src/jsc/bindings/webcore/streams/JSWritableStream*.cpp, test/js/web/streams/streams.test.js |
A shared helper creates ERR_INVALID_THIS errors. Readable and writable stream methods use it for invalid receivers. Tests check rejected promises, error codes, messages, and promise uniqueness. |
Byte-stream argument errors src/jsc/bindings/webcore/streams/JSReadableByteStreamController.cpp, src/jsc/bindings/webcore/streams/JSReadableStreamBYOBReader.cpp, test/js/web/streams/streams.test.js |
BYOB read() and byte-stream enqueue() use invalid-argument errors for missing or invalid values. Tests also check BYOB error messages and confirm queued bytes remain available after a rejected read. |
Suggested reviewers: cirospaciari
Merge Risk: 
c89ac
-
No actionable merge-blocking issue is established for the shared error-helper change. Merge readiness remains subject to normal build and stream-test checks.
Pre-merge checks: 
-
Check name Status Explanation Title check The title names the two error codes the PR restores, ERR_INVALID_THIS and ERR_INVALID_ARG_TYPE, which are the main change in the streams bindings. It fits the changeset, which adds createInvalidThisEr… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description follows the repository template. It has both required sections, "What does this PR do?" and "How did you verify your code works?". The Problem, Fix, and Background parts explain the ch… Full details: Title check
-
Explanation
The title names the two error codes the PR restores, ERR_INVALID_THIS and ERR_INVALID_ARG_TYPE, which are the main change in the streams bindings. It fits the changeset, which adds createInvalidThisErrorForType and switches the BYOB read and enqueue argument checks to ERR::INVALID_ARG_INSTANCE. The title is concise and specific, so a teammate scanning history can see the primary change.
Full details: Description check
-
Explanation
The description follows the repository template. It has both required sections, "What does this PR do?" and "How did you verify your code works?". The Problem, Fix, and Background parts explain the change and its scope. The Verified part names the new tests, the WPT run, and the other affected test files. It also gives a reproduction table, a list of the 15 changed members, and measurements. Its verification section is detailed, and the evidence note says the streams test file did not run on the author's machine, so CI platform validation is deferred. The description is complete enough to pass. The remaining gap is that CI-only validation is deferred, and the description states this.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also checked the new BYOB read() argument path for exception handling: INVALID_ARG_INSTANCE propagates a throw from the value's constructor getter via RELEASE_RETURN_IF_EXCEPTION, and the caller's RETURN_IF_EXCEPTION inside promiseFromSteps turns it into the rejection the new test pins. I also grepped the streams bindings for any remaining bespoke wrong-this rejections (none left; the two remaining createTypeError rejections are argument/state errors the PR scopes out) and the test suite for assertions on the old message text (none).
Extended reasoning...
The diff is a mechanical swap of 15 wrong-receiver rejections in the C++ WHATWG stream bindings to a new Bun::createInvalidThisErrorForType helper, plus two byte-stream argument sites routed through Bun::ERR::INVALID_ARG_INSTANCE, with a new test enumerating every node:stream/web prototype member. It touches no security-sensitive surface. The one inline finding (the enqueue() error naming a "buffer" argument where Node says "chunk") is now pinned by the new test while the PR claims message parity with Node, so a human should decide whether to rename before merging; that is why this is a defer rather than an approve.
Collaborator
Author
|
On the one review finding (
|
Collaborator
Author
|
Merged main (da867c6) into this branch so that CI runs on a green main. There was no conflict.
|
…eturning members The 15 web stream methods and getters that return a promise rejected a wrong `this` with a plain TypeError. Bun::ERR::INVALID_THIS can only throw, so each rejecting arm built its own error, with no code. Bun::createInvalidThisErrorForType makes that error as a value. The thrower now calls it, and the 15 arms reject with it. The code and the message equal Node.js: ERR_INVALID_THIS, `Value of "this" must be of type <Class>`.
ReadableStreamBYOBReader.prototype.read() with a value that is not a view, and ReadableByteStreamController.prototype.enqueue() with no argument, failed with a TypeError that had no code. Both now use Bun::ERR::INVALID_ARG_INSTANCE, as respondWithNewView() and enqueue(value) already do. A missing argument reads as undefined.
robobun
force-pushed
the
robobun/7580a254/streams-invalid-this-code
branch
from
November 2, 2026 21:45
04f876e to
c89ac33
Compare
Contributor
There was a problem hiding this comment.
LGTM — a mechanical swap of 15 wrong-this rejections to a shared ERR_INVALID_THIS creator plus two argument sites moved to the existing INVALID_ARG_INSTANCE helper, with a test that pins all of it.
What was reviewed:
- Exception/scope handling at the two new throw sites:
convertBYOBReadArgumentsleaves the exception pending and thepromiseFromStepslambda checks it;enqueuereturns the helper's value directly andargument(0)makes the zero-arg caseundefinedrather than a Not-enough-arguments throw. createInvalidThisErrorForTypeismakeStringover ASCIILiterals with no JS entry, so the getters calling it without a ThrowScope are fine;ERR::INVALID_THISkeeps the same code and text.- Grepped the suite for the old "can only be called on" / "requires an ArrayBufferView" strings — no stale assertions remain.
- Considered the sync-throw vs rejection change for a
viewwhoseconstructorgetter throws: WebIDL rejects for promise-returning operations and the new test pins it, so not a defect.
Extended reasoning...
The diff touches src/jsc/bindings/ErrorCode.{h,cpp} (one new helper, INVALID_THIS refactored to use it) and six hand-written WHATWG stream bindings, changing only the error code and message of calls that already fail, plus 151 lines of tests in test/js/web/streams/streams.test.js. It touches no security-sensitive surface. The changes are small, repetitive, follow the existing INVALID_ARG_INSTANCE pattern used by respondWithNewView, and the new test enumerates every prototype member against a receiver matrix with a non-vacuous expected list. No CODEOWNERS entry covers the changed files, the hunt ran dry, and the only prior inline thread (argument name "buffer") matches Node's validateBuffer default, so nothing blocks approval.
This branch has not been deployed
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
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.
Problem
thiswith aTypeErrorwithout acode:ReadableStreamDefaultReader.prototype.read can only be called on a ReadableStreamDefaultReader. Node.js givesERR_INVALID_THIS. Bun 1.3.14 did too for 12 of the 15.ERR_INVALID_ARG_TYPEthe same way:ReadableStreamBYOBReader.read(x)wherexis not a view, andReadableByteStreamController.enqueue().Bun::ERR::INVALID_THIS(ErrorCode.cpp:1913) can only throw, so each rejecting branch built a plaincreateTypeError.Fix
Bun::createInvalidThisErrorForTypemakes the error ofERR::INVALID_THISas a value. The thrower and the 15 rejecting branches use it.Bun::ERR::INVALID_ARG_INSTANCE, asrespondWithNewView()does.test/js/web/streams/streams.test.js(4 new tests, main fails all 4), WPT streams, and 8 files on otherERR::INVALID_THIScallers. Self-reviewed: 10 concerns raised, 10 addressed.Background
this. It does not throw.code: 'ERR_INVALID_THIS'on 14 of the 15 rejections.Bun::createErrorcalls (they repeat Node's sentence), and a throw insidepromiseFromSteps(oneJSC::Exceptionper wrong call).Downsides
console.log(WritableStreamDefaultWriter.prototype)still exits 1 until Handle the rejection of a native getter that the formatter or a debugger preview read #41630, and its error no longer names the getter.read(x)now describesx, asenqueue(x)does. A constructor-less object nested 20,000 deep rejects with aRangeError(node errors: render -0 and constructor-less objects in determineSpecificType like node #38642).thiscall makes one more GC cell, aJSString.Notes
Rebase. The branch is rebased onto main a4c3a5e (head c89ac33). It had one conflict. #47316 put
NEVER_INLINEonBun::throwInvalidThisCallError, the function below the new creator inErrorCode.cpp. That function is the one call in the wrong-thisbranch of each generated host function. The creator has the same role in the 15 hand-written branches, so it isNEVER_INLINEtoo. No other line of the diff changed. Each number and test result in these notes is from the rebased head and from main a4c3a5e.Repro.
read.call({})ERR_INVALID_THISReadableStreamDefaultReader.prototype.read can only be called on a ReadableStreamDefaultReaderERR_INVALID_THIS,Value of "this" must be of type ReadableStreamDefaultReaderread("x")ERR_INVALID_ARG_TYPEReadableStreamBYOBReader.prototype.read requires an ArrayBufferViewERR_INVALID_ARG_TYPE,The "view" argument must be an instance of Buffer, TypedArray, or DataView. Received type string ('x')enqueue()ERR_INVALID_ARG_TYPENot enough argumentsERR_INVALID_ARG_TYPE,The "buffer" argument must be an instance of Buffer, TypedArray, or DataView. Received undefinedReach. No issue reports this. A differential run against Node.js and Bun 1.3.14 found it. Each affected call already fails with a
TypeError, as a rejection or a throw as before. Onlycodeandmessagechange.The 15 members.
ReadableStream:cancel,pipeTo.ReadableStreamDefaultReaderandReadableStreamBYOBReader:closed,cancel,read.WritableStream:abort,close.WritableStreamDefaultWriter:closed,ready,abort,close,write.12 are regressions from 1.3.14. The three
closedgetters never had the code in Bun. They are the same wrong-thisbranch with the same line shape, Node.js gives them the code, and Node's tests assert it, so they get it here too. I called every own method and getter of the 17 classes ofnode:stream/webon wrong receivers. These 15 are the only members that answer a wrongthiswith a promise, on main, on this PR and on Node.js.Excluded on purpose.
ReadableStream.prototype.text(),json(),bytes(),blob()andReadableStreamDefaultReader.prototype.readMany()return a promise on a valid receiver and throw on a wrong one. The four throwERR_INVALID_THIS,readMany()throws aTypeErrorwithout a code. 1.3.14 does the same, and Node.js has none of them. This PR does not change how they fail.ReadableStreamasync iterator. Itsnext()andreturn()already reject withERR_INVALID_THIS.Receivers that Node.js accepts. Node.js accepts a
Proxyof an instance, and an object that inherits from an instance, asthisof all 15 members. Its brand check reads a symbol, which both forward. WebIDL refuses both, and so does Bun, on 1.3.14, on main and here. The new test has both receivers and pins the rejection.Not in this PR. The rule: this PR restores the codes that 1.3.14 had, plus the three
closedgetters. Argument and state errors that never had a code stay as they are. These neighbours of the two argument sites have a code on Node.js and none on Bun. 1.3.14 raised no error for any of them, so they are not regressions:JSReadableStreamBYOBReader.cpp:131,read(view, 5): Node.jsERR_INVALID_ARG_TYPE.JSReadableStreamBYOBReader.cpp:138,read(view, { min: "a" }): Node.jsERR_INVALID_ARG_TYPE.JSReadableStreamBYOBReader.cpp:433,read(view, { min: 0 }): Node.jsERR_INVALID_ARG_VALUE.JSReadableStreamBYOBReader.cpp:437,read(view, { min: 2 })on a 1-byte view: Node.jsERR_OUT_OF_RANGE.JSReadableByteStreamController.cpp:564,enqueue()of a view over aSharedArrayBuffer: Node.jsERR_INVALID_ARG_VALUE.A survey of 179 wrong calls across the stream classes found 80 such rows in total: arguments of constructors,
pipeTo(),pipeThrough()andrespond(), theminoption,write()afterclose(). Each needs a decision on its message. A comment on this PR has the survey script and its results.BYOB
read(x)and the value describer.Bun::ERR::INVALID_ARG_INSTANCEdescribes the value for theReceived ...part. That readsx.constructor. What a getter throws there is the rejection, as on Node.js (main never ran it, 1.3.14 threw it synchronously). For an object without a constructor, the describer prints the whole object on one line:ERR_INVALID_ARG_TYPE, 109 charactersERR_INVALID_ARG_TYPE, 5,392 charactersERR_INVALID_ARG_TYPE, 109 charactersRangeError: Maximum call stack size exceeded., no codeenqueue(x)gives the same two results on main.read(-0)saystype number (0)where Node.js says(-0). #38642 changes the describer for both cases. The second commit (read()andenqueue()) can move to its own PR unchanged if this is a concern.Measurements. Release builds of main a4c3a5e and of this PR (c89ac33), linux-x64, same machine and flags:
--profile=release --trimmable=on, the link that CI uses.enqueue(): no member runs more instructions, 3 run 2 fewer (gdbnextiin the frame of the member, 3 identical runs per binary).closed10 to 10,ready27 to 27,read()61 to 61 (queued chunk) and 94 to 94 (pending),write()31 to 31,enqueue()94 to 94. The 3 arecancel()of both readers (31 to 29) and BYOBread()(169 to 167)..text110,637,772 to 110,637,196 B (-576),.rodata26,845,264 to 26,843,920 B (-1,344), file 140,680,312 to 140,672,120 B (-8,192). The 15 member symbols 8,244 to 7,688 B.ERR::INVALID_THIS216 to 48 B. The new creator is 137 B. BYOBreadgrows 66 B, in its cold branch (size -A,nm -S.bloatyis not installed).bun build --compile --aot, mapped bytes astest/bundler/compile-aot-size.test.tscounts them: 68,116,932 to 68,116,868 B, and with--trim25,964,996 to 25,960,836 B. CI built the same two commits (builds 137005 and 137037): 68,117,668 to 68,113,572 B, and with--trim25,957,540 to 25,961,636 B.--trimremoves whole pages only (src/trim/apply.rs), so a change of a few hundred bytes moves that total by 4,096 B in either direction. The--trimbound of that test (25,880,000 B) fails on main without this PR.streams/), prototype table rows +0, startup work +0. New C++ functions: 1.thiscall: 4mi_malloccalls (main 4), 4 GC cells (main 3: a string, aPromise, anError. The fourth is theJSStringthatBun::createErrormakes from itsStringargument), 0JSC::Exceptioncells, 0 calls ofVM::throwException. Counted withheapStats().objectTypeCountsover 400 calls with no collection between the readings, and withgdbbreakpoints on the main thread.ERR::INVALID_THISthrow sites (67 call sites): mimalloc calls per throw 7 to 5, on 5 sync stream members (locked,getReader(),desiredSize, bothreleaseLock()).globalThisand in 8node:modules, each called on{}): 15 rows differ between main and this PR, the 15 members. The 325 rows that already giveERR_INVALID_THISon main keep name, code and message.ErrorCode.hedit recompiles 216 of the 290 objects outsidevendor/.this0 of 15 (main 15), wrong-argument 0 of 9 (main 9). The survey of 179 wrong calls changes in exactly those 9 rows.Tests.
thistest enumerates the classes ofnode:stream/weband their members. A new member that answers a wrongthiswith a promise and rejects without the code fails it. Its logic, run as a script on Node.js v26.3.0 without the two receivers that Node.js accepts, gives the same 15 names and no differing outcome.BUN_JSC_validateExceptionChecks=1.streams.test.js: 731 pass, 2 skip. WPT streams: 1175 pass. With thesrc/of main the 4 new tests fail and the other 729 pass (release build).streams.test.js733 pass, WPT streams 1175 pass. These 8 files pass on both builds, except the any-receiver file on the debug build:readablestream-helpers.test.ts,globals.test.js,histogram.test.ts,members-of-prototype-objects-for-any-receiver.test.ts, andtest-whatwg-readablebytestream.js,test-crypto-keyobject-brand-check.js,test-vm-module-errors.js,test-broadcastchannel-custom-inspect.js.node:events,node:stream,node:http,node:zlib,node:async_hooks). A debug build of main a4c3a5e fails the same 5 with the same output, so this diff is not the cause.test-whatwg-readablestream.jsandtest-whatwg-writablestream.jsassert these codes. They need--expose-internals, so this PR cannot add them.Merge order with #41630. #41630 changes the same 4 getter lines (the three
closedgetters andready). It sets JSC's native-getter flag on aTypeErrorthat has no code, so that a print ofReadableStreamDefaultReader.prototypedoes not end the process. The two PRs conflict on those lines, and each fails the other's test. The line that serves both is the same in either order:This PR is the smaller one and has no open question, so it can land first. Then #41630 takes that line, and its two expectations in
test/js/bun/util/inspect.test.jsthat pin the old getter text take the new sentence.no test proof · iteration 11 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/streams/streams.test.js