Skip to content
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.

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

stream.Readable.toWeb(readable) pauses the original readable stream #45545

Closed
cola119 opened this issue Nov 20, 2022 · 3 comments
Closed

stream.Readable.toWeb(readable) pauses the original readable stream #45545

cola119 opened this issue Nov 20, 2022 · 3 comments
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@cola119
Copy link
Member

cola119 commented Nov 20, 2022

Version

v19.1.0

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

// index.mjs
import { Stream } from "node:stream";

function* generate() {
  yield "hello";
  yield "streams";
}
const readable = Stream.Readable.from(generate());

const webReadable = Stream.Readable.toWeb(readable);

readable.on("data", (chunk) => {
  console.log(chunk);
});

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

$ node index.mjs
hello
streams

What do you see instead?

$ node index.mjs
hello

Additional information

This is because the newReadableStreamFromStreamReadable function called during toWeb pauses the original readable stream at the following:

streamReadable.pause();
}
streamReadable.pause();

Is this a bug or is this an intentional behavior that has a side effect on the original stream?

@cola119 cola119 changed the title stream.Readable.toWeb(readable) pauses the original readable stream.Readable.toWeb(readable) pauses the original readable stream Nov 20, 2022
@cola119 cola119 added the stream Issues and PRs related to the stream subsystem. label Nov 20, 2022
@cola119
Copy link
Member Author

cola119 commented Nov 21, 2022

Maybe the document mentions something related?
https://nodejs.org/api/stream.html#choose-one-api-style

The Readable stream API evolved across multiple Node.js versions and provides multiple methods of consuming stream data. In general, developers should choose one of the methods of consuming data and should never use multiple methods to consume data from a single stream. Specifically, using a combination of on('data'), on('readable'), pipe(), or async iterators could lead to unintuitive behavior.

@jakecastelli
Copy link
Member

jakecastelli commented Jun 17, 2024

I think this is a side effect on the origin stream, IMO either we should not see "hello" or we should not pause the original readable stream.

Could clone be a good candidate for it as well? like here Update: I think the original stream should be paused based on my understanding of the whatgw spec.
@mcollina 👀

@mcollina
Copy link
Member

this is intentional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants