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.

Sign up for GitHub

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

Jump to bottom

webstream: convert premature close to AbortError #39524

Closed
wants to merge 5 commits into from
Closed

webstream: convert premature close to AbortError #39524

wants to merge 5 commits into from

Conversation

ronag
Copy link
Member

@ronag ronag commented Jul 26, 2021

AbortError is a more "web" align alternative to
ERR_STREAM_PREMATURE_CLOSE.

@ronag
webstream: convert premature close to AbortError
f024765 
AbortError is a more "web" align alternative to
ERR_STREAM_PREMATURE_CLOSE.
@ronag ronag requested a review from jasnell July 26, 2021 07:32
@ronag ronag added stream Issues and PRs related to the stream subsystem. web streams labels Jul 26, 2021
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jul 26, 2021
@ronag @aduh95
Update lib/internal/webstreams/adapters.js
18ba68a 
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@ronag ronag requested a review from aduh95 July 26, 2021 14:15
@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 26, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 26, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@ronag ronag added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Jul 26, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 26, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 27, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 27, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 28, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 28, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@ronag
Copy link
Member Author

ronag commented Jul 28, 2021

Landed in 3f0b623

@ronag ronag closed this Jul 28, 2021
ronag added a commit that referenced this pull request Jul 28, 2021
@ronag
stream: convert premature close to AbortError
3f0b623 
AbortError is a more "web" align alternative to
ERR_STREAM_PREMATURE_CLOSE.

PR-URL: #39524
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@aduh95
Copy link
Contributor

aduh95 commented Jul 28, 2021

What should be the semverness of this PR? Changing error code is usually semver-major, but maybe semver-minor is fine since WHATWG streams are still experimental.
Also adding don't land labels as I beleive WHATWG streams are not available on LTS release lines, please remove them if that's not correct.

@aduh95 aduh95 added dont-land-on-v12.x semver-minor PRs that contain new features and should be released in the next minor version. labels Jul 28, 2021
@jasnell
Copy link
Member

jasnell commented Jul 28, 2021

Web streams are experimental so there is no semver status.

@aduh95 aduh95 removed the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 28, 2021
@targos
Copy link
Member

targos commented Aug 22, 2021

Depends on #39134

@BethGriggs BethGriggs mentioned this pull request Oct 14, 2021
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. stream Issues and PRs related to the stream subsystem. web streams
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ronag @nodejs-github-bot @aduh95 @jasnell @targos @danielleadams