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: prevent stream unexpected pause when highWaterMark set to 0 #53261

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

jakecastelli
Copy link
Contributor

@jakecastelli jakecastelli commented Jun 2, 2024

Fixes: #51930

p.s. I noticed that the bot knows to require review from the correct team but the label stream isn't added 👀

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jun 2, 2024
@jakecastelli
Copy link
Contributor Author

I tend to agree with @ronag on highWaterMark set to 0 does not make much sense but I guess that will be a sermver-major?

Co-authored-by: Robert Nagy <ronagy@icloud.com>
@vweevers
Copy link
Contributor

vweevers commented Jun 2, 2024

See also whatwg/streams#1158 (I'm not sure what the conclusion is there, long read).

@benjamingr benjamingr added the stream Issues and PRs related to the stream subsystem. label Jun 2, 2024
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@H4ad H4ad added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 7, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 7, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@H4ad H4ad added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 8, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 8, 2024
@nodejs-github-bot nodejs-github-bot merged commit 50695e5 into nodejs:main Jun 8, 2024
59 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 50695e5

targos pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Robert Nagy <ronagy@icloud.com>
PR-URL: #53261
Fixes: #51930
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
EliphazBouye pushed a commit to EliphazBouye/node that referenced this pull request Jun 20, 2024
Co-authored-by: Robert Nagy <ronagy@icloud.com>
PR-URL: nodejs#53261
Fixes: nodejs#51930
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
Co-authored-by: Robert Nagy <ronagy@icloud.com>
PR-URL: nodejs#53261
Fixes: nodejs#51930
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
Co-authored-by: Robert Nagy <ronagy@icloud.com>
PR-URL: #53261
Fixes: #51930
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
Co-authored-by: Robert Nagy <ronagy@icloud.com>
PR-URL: #53261
Fixes: #51930
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

piping to a stream with highWaterMark: 0 and objectMode: false is broken since node v20.10.0
6 participants