Skip to content

[v24.x] Revert "stream: noop pause/resume on destroyed streams"#63834

Closed
sxa wants to merge 1 commit into
nodejs:v24.x-stagingfrom
sxa:revert_for_63487.1
Closed

[v24.x] Revert "stream: noop pause/resume on destroyed streams"#63834
sxa wants to merge 1 commit into
nodejs:v24.x-stagingfrom
sxa:revert_for_63487.1

Conversation

@sxa

@sxa sxa commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

This reverts commit 29b1966 from #62557

Should fix #63487 based on a git bisect run

@nodejs-github-bot

nodejs-github-bot commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch. labels Jun 10, 2026
@nodejs-github-bot

nodejs-github-bot commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74035/

@sxa

sxa commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

Revert on top of v24.x-staging confirmed to have resolved the problem on my local system therefore I am taking this out of draft status.

@sxa sxa marked this pull request as ready for review June 10, 2026 12:56
@sxa

sxa commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

FYI @nodejs/tsc @nodejs/releasers

mcollina
mcollina approved these changes Jun 10, 2026
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm confirmed

@aduh95 aduh95 changed the title Revert "stream: noop pause/resume on destroyed streams" [v24.x] Revert "stream: noop pause/resume on destroyed streams" Jun 10, 2026
@MikeMcC399

MikeMcC399 commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Successfully tested against cypress@15.15.0👍🏻

Cypress already produced a fix for this issue in cypress@15.16.0 so I'm testing against the previous unfixed version cypress@15.15.0.

The Cypress analysis of why it was failing on Windows (always) and Linux (when no unzip installed) is in cypress-io/cypress#33891 (comment). In both cases, extract-zip was being invoked and it failed on Node.js 24.16.0 and Node.js >=26.1.0 with the symptom that unzipping either hung or silently exited, depending on how the installation of the Cypress binary was invoked.

Verification

On Windows 11 25H2, in Git Bash with Node.js v24.16.1-pre, built from #63834, installed

cd $(mktemp -d)
npm install cypress@15.15.0 --ignore-scripts # fixed versions >=15.16.0
npx cypress install --force
npx cypress verify

Logs

$ cd $(mktemp -d)
npm install cypress@15.15.0 --ignore-scripts # fixed versions >=15.16.0
npx cypress install --force
npx cypress verify
npm warn cli npm v11.7.0 does not support Node.js v24.16.1-pre. This version of npm supports the following node versions: `^20.17.0 || >=22.9.0`. You can find the latest version at https://nodejs.org/.

added 175 packages in 6s

53 packages are looking for funding
  run `npm fund` for details
npm warn cli npm v11.7.0 does not support Node.js v24.16.1-pre. This version of npm supports the following node versions: `^20.17.0 || >=22.9.0`. You can find the latest version at https://nodejs.org/.

Cypress 15.15.0 is installed in C:\Users\mikem\AppData\Local\Cypress\Cache\15.15.0

Installing Cypress (version: 15.15.0)

√  Downloaded Cypress
√  Unzipped Cypress
√  Finished Installation C:\Users\mikem\AppData\Local\Cypress\Cache\15.15.0

You can now open Cypress by running one of the following, depending on your package manager:

- npx cypress open
- yarn cypress open
- pnpm cypress open

https://on.cypress.io/opening-the-app

npm warn cli npm v11.7.0 does not support Node.js v24.16.1-pre. This version of npm supports the following node versions: `^20.17.0 || >=22.9.0`. You can find the latest version at https://nodejs.org/.

√  Verified Cypress! C:\Users\mikem\AppData\Local\Cypress\Cache\15.15.0\Cypress

@MikeMcC399 MikeMcC399 mentioned this pull request Jun 10, 2026
Open
@nodejs-github-bot

nodejs-github-bot commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74050/

@nodejs-github-bot

nodejs-github-bot commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74051/

@drbarq drbarq mentioned this pull request Jun 13, 2026
Open
3 tasks
@MikeMcC399 MikeMcC399 mentioned this pull request Jun 16, 2026
Open
@silverwind silverwind mentioned this pull request Jun 17, 2026
Merged
@sxa sxa force-pushed the revert_for_63487.1 branch from a1f17ea to 5b397b5 Compare June 18, 2026 12:19
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jun 18, 2026
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor
Failed to start CI
   ⚠  Commits were pushed since the last approving review:
   ⚠  - Revert "stream: noop pause/resume on destroyed streams"
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/27762715319

@sxa sxa added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2026
@sxa sxa removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Jun 18, 2026
@sxa sxa force-pushed the revert_for_63487.1 branch 2 times, most recently from e2bc24c to 896d7d5 Compare June 18, 2026 15:32
@sxa
Revert "stream: noop pause/resume on destroyed streams"
18f3731 
This reverts commit 29b1966.

Signed-off-by: Stewart X Addison <sxa@ibm.com>
@sxa sxa force-pushed the revert_for_63487.1 branch from 896d7d5 to 18f3731 Compare June 18, 2026 15:34
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jun 18, 2026
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor
Failed to start CI
   ⚠  Commits were pushed since the last approving review:
   ⚠  - Revert "stream: noop pause/resume on destroyed streams"
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/27771938741

@sxa

sxa commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Landed in 8471808

@sxa sxa closed this Jun 18, 2026
@MattiasBuelens

MattiasBuelens commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@sxa 8471808 doesn't seem to be on v24.x-staging? Or am I missing something?

sxa added a commit that referenced this pull request Jun 18, 2026
@sxa
Revert "stream: noop pause/resume on destroyed streams"
3f54c8b 
This reverts commit 29b1966.

Signed-off-by: Stewart X Addison <sxa@ibm.com>
PR-URL: #63834
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
@sxa

sxa commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

There was an issue with the signing on the commit not occurring properly so it got rejected as it was pushed up which I've been trying to resolve. Pushed again as 3f54c8b

@github-actions github-actions Bot mentioned this pull request Jun 19, 2026
Closed
@sxa sxa mentioned this pull request Jun 22, 2026
Open
sxa added a commit to sxa/node that referenced this pull request Jun 22, 2026
@sxa
2026-06-23, Version 24.18.0 'Krypton' (LTS)
76f17d9 
Notable changes:

buffer:
  * (SEMVER-MINOR) increase Buffer.poolSize default to 64 KiB (Matteo Collina) nodejs#63597
crypto:
  * update root certificates to NSS 3.123.1 (Node.js GitHub Bot) nodejs#63527
  * (SEMVER-MINOR)  align key argument names in docs and error messages (Filip Skokan) nodejs#62527
  * (SEMVER-MINOR)  accept key data in crypto.diffieHellman() and cleanup DH jobs (Filip Skokan) nodejs#62527
  * (SEMVER-MINOR)  add TurboSHAKE and KangarooTwelve Web Cryptography algorithms (Filip Skokan) nodejs#62183
http:
  * http: avoid stream listeners on idle agent sockets (Matteo Collina) nodejs#64004
  * (SEMVER-MINOR) add writeInformation to send arbitrary 1xx status codes (Tim Perry) nodejs#63155
inspector:
  * (SEMVER-MINOR) expose precise coverage start to JS runtime (sangwook) nodejs#63079
stream:
  * stream: Revert noop pause/resume on destroyed streams" (Stewart X Addison) nodejs#63834

PR-URL: nodejs#64062
sxa added a commit that referenced this pull request Jun 22, 2026
@sxa
2026-06-23, Version 24.18.0 'Krypton' (LTS)
3c6a428 
Notable changes:

buffer:
  * (SEMVER-MINOR) increase Buffer.poolSize default to 64 KiB (Matteo Collina) #63597
crypto:
  * update root certificates to NSS 3.123.1 (Node.js GitHub Bot) #63527
  * (SEMVER-MINOR)  align key argument names in docs and error messages (Filip Skokan) #62527
  * (SEMVER-MINOR)  accept key data in crypto.diffieHellman() and cleanup DH jobs (Filip Skokan) #62527
  * (SEMVER-MINOR)  add TurboSHAKE and KangarooTwelve Web Cryptography algorithms (Filip Skokan) #62183
http:
  * http: avoid stream listeners on idle agent sockets (Matteo Collina) #64004
  * (SEMVER-MINOR) add writeInformation to send arbitrary 1xx status codes (Tim Perry) #63155
inspector:
  * (SEMVER-MINOR) expose precise coverage start to JS runtime (sangwook) #63079
stream:
  * stream: Revert noop pause/resume on destroyed streams" (Stewart X Addison) #63834

PR-URL: #64062
sxa added a commit that referenced this pull request Jun 22, 2026
@sxa
2026-06-23, Version 24.18.0 'Krypton' (LTS)
01ecf45 
Notable changes:

buffer:
  * (SEMVER-MINOR) increase Buffer.poolSize default to 64 KiB (Matteo Collina) #63597
crypto:
  * update root certificates to NSS 3.123.1 (Node.js GitHub Bot) #63527
  * (SEMVER-MINOR)  align key argument names in docs and error messages (Filip Skokan) #62527
  * (SEMVER-MINOR)  accept key data in crypto.diffieHellman() and cleanup DH jobs (Filip Skokan) #62527
  * (SEMVER-MINOR)  add TurboSHAKE and KangarooTwelve Web Cryptography algorithms (Filip Skokan) #62183
http:
  * http: avoid stream listeners on idle agent sockets (Matteo Collina) #64004
  * (SEMVER-MINOR) add writeInformation to send arbitrary 1xx status codes (Tim Perry) #63155
inspector:
  * (SEMVER-MINOR) expose precise coverage start to JS runtime (sangwook) #63079
stream:
  * stream: Revert noop pause/resume on destroyed streams" (Stewart X Addison) #63834

PR-URL: #64062
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes

@panva panva panva approved these changes

@lpinca lpinca lpinca approved these changes

@richardlau richardlau richardlau approved these changes

@StefanStojanovic StefanStojanovic StefanStojanovic approved these changes

@RafaelGSS RafaelGSS RafaelGSS approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. stream Issues and PRs related to the stream subsystem. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.