Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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/js/internal/primordials.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
new Promise((a, b) => PromisePrototypeThen.$call(mapFn == null ? promise : mapFn(promise, i), a, b)),
),
);
const PromiseAll = Promise.all;
const PromiseAll = Promise.all.bind(Promise);

Check notice on line 96 in src/js/internal/primordials.js

View check run for this annotation

Claude / Claude Code Review

Same unbound-Promise-static pattern exists in fs/cp.ts and readline.ts

Heads up: this is a pre-existing issue not introduced by this PR, but the identical unbound-Promise-static pattern exists in two other spots — `src/js/internal/fs/cp.ts:21` (`const PromiseReject = Promise.$reject;`, called free at line 105) and `src/js/node/readline.ts:46` (called free at lines 2238/2685). Both will throw `|this| is not an object` instead of returning a rejected promise; `operators.ts:12` and `diagnostics_channel.ts:16` already `.bind(Promise)` for the same reason. Might be wort
Comment thread
robobun marked this conversation as resolved.
const PromiseResolve = Promise.$resolve.bind(Promise);
const SafePromiseAll = (promises, mapFn) => PromiseAll(arrayToSafePromiseIterable(promises, mapFn));
const SafePromiseAllReturnArrayLike = (promises, mapFn) =>
Expand Down
25 changes: 25 additions & 0 deletions test/js/node/stream/node-stream.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,31 @@ it("Readable.fromWeb", async () => {
expect(Buffer.concat(chunks).toString()).toBe("Hello World!\n");
});

// https://github.com/oven-sh/bun/issues/30939
// Multiple queued writes on a Transform.fromWeb stream trigger _writev, which
// internally calls SafePromiseAll. Promise.all must be bound in primordials;
// otherwise this throws "TypeError: |this| is not an object".
it("Transform.fromWeb _writev does not throw on buffered writes", async () => {
const s = Transform.fromWeb(new TransformStream());
const chunks = [];
s.on("data", chunk => chunks.push(chunk));
const { promise, resolve, reject } = Promise.withResolvers();
s.on("end", resolve);
s.on("error", reject);

// Five synchronous writes force the writable side to queue and coalesce
// into a single _writev call.
s.write(Buffer.from("hello"));
s.write(Buffer.from("hello"));
s.write(Buffer.from("hello"));
s.write(Buffer.from("hello"));
s.write(Buffer.from("hello"));
s.end();

await promise;
expect(Buffer.concat(chunks).toString()).toBe("hellohellohellohellohello");
});

it("#9242.5 Stream has constructor", () => {
const s = new Stream({});
expect(s.constructor).toBe(Stream);
Expand Down
Loading