FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http: don't emit error for stream destroyed by ronag · Pull Request #33654 · nodejs/node · GitHub

/ node Public

http: don't emit error for stream destroyed - #33654

Closed
ronag wants to merge 3 commits into
nodejs:masterfrom
nxtedition:outgoing-error-close
ConversationCommits (3)ChecksFiles changed
Closed

http: don't emit error for stream destroyed#33654
ronag wants to merge 3 commits into
nodejs:masterfrom
nxtedition:outgoing-error-close

Conversation

ronag commented May 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Aligns with streams

Refs: #33591

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

ronag added http Issues or PRs related to the http subsystem. v14.x labels May 30, 2020
ronag requested a review from addaleax May 30, 2020 08:41
ronag force-pushed the outgoing-error-close branch from 72c7eac to 734bf3a Compare May 30, 2020 08:42

nodejs-github-bot commented May 31, 2020

Copy link
Copy Markdown
Collaborator

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

ronag commented May 31, 2020

Copy link
Copy Markdown
Member Author

@nodejs/http @nodejs/web-server-frameworks

nodejs-github-bot commented May 31, 2020

Copy link
Copy Markdown
Collaborator

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

BridgeAR force-pushed the master branch 2 times, most recently from 8ae28ff to 2935f72 Compare May 31, 2020 12:18
ronag changed the title http: don't emit error after close http: don't emit error for stream destroyed May 31, 2020
ronag force-pushed the outgoing-error-close branch 2 times, most recently from b152c48 to a21385e Compare May 31, 2020 19:14

ronag commented May 31, 2020

Copy link
Copy Markdown
Member Author

unsure about the semversiness of this one.

ronag force-pushed the outgoing-error-close branch from a21385e to 48a58b1 Compare May 31, 2020 19:45

nodejs-github-bot commented May 31, 2020

Copy link
Copy Markdown
Collaborator

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

ronag commented Jun 1, 2020
View reviewed changes
Comment thread test/parallel/test-http-server-write-end-after-end.js Outdated
ronag force-pushed the outgoing-error-close branch 2 times, most recently from 92d8ad6 to 73b6a74 Compare June 1, 2020 15:38
ronag mentioned this pull request Jun 1, 2020
Closed
4 tasks
This was referenced Jun 4, 2020
Closed
Open
Open
Open
Open
Open

BethGriggs commented Jun 9, 2020

Copy link
Copy Markdown
Member

Should this land in the next v14.x release?

ronag requested a review from mcollina June 9, 2020 15:30
mcollina approved these changes Jun 9, 2020
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm, crossing fingers we do not break anything.

ronag force-pushed the outgoing-error-close branch from 0d71e89 to 7191d18 Compare June 15, 2020 20:56

ronag commented Jun 15, 2020

Copy link
Copy Markdown
Member Author

@mcollina: I've updated the test so that it doesn't have to be commented out. I don't think #33684 should be blocking this PR.

nodejs-github-bot commented Jun 15, 2020

Copy link
Copy Markdown
Collaborator

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

mcollina approved these changes Jun 16, 2020
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

This was referenced Jun 18, 2020
Open
Open
Open
ronag force-pushed the outgoing-error-close branch from 7191d18 to 402fc26 Compare June 20, 2020 11:58

ronag commented Jun 20, 2020

Copy link
Copy Markdown
Member Author

This needs another CI run + CITGM

addaleax added needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run. author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed v14.x needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run. labels Jun 20, 2020

addaleax commented Jun 20, 2020

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/31991/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2394/

nodejs-github-bot commented Jun 21, 2020 •
edited by ronag
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32001/ ✅
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2395/
CITGM (master): https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2396/

ronag commented Jun 21, 2020

Copy link
Copy Markdown
Member Author

Landed in 30cc542

ronag closed this Jun 21, 2020
ronag added a commit that referenced this pull request Jun 21, 2020
http: don't emit error after close
30cc542 
Refs: #33591

PR-URL: #33654
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

MylesBorins commented Jul 14, 2020

Copy link
Copy Markdown
Contributor

This doesn't land cleanly on v14.x. @ronag would you be willing to backport?

ronag self-assigned this Jul 14, 2020
Lxxyx mentioned this pull request Jan 5, 2021
Closed
3 tasks

targos commented May 16, 2021

Copy link
Copy Markdown
Member

Should this be backported with #32933 ?

ronag commented May 16, 2021

Copy link
Copy Markdown
Member Author

Yes. I'm a little overloaded at the moment though. If you need help ping me again in a. week :).

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
jasnell jasnell approved these changes
addaleax Awaiting requested review from addaleax

Assignees

ronag

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. http Issues or PRs related to the http subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL