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

http: ensure client request emits close #33178

Closed
wants to merge 2 commits into from
Closed

http: ensure client request emits close #33178

wants to merge 2 commits into from

Conversation

ronag
Copy link
Member

@ronag ronag commented Apr 30, 2020

If socket creation failed then an error would be
emitted on the client request object, but not
'close' nor would destroyed be set to true.

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
http: ensure client request emits close
25bc845 
If socket creation failed then an error would be
emitted on the client request object, but not
'close' nor would destroyed be set to true.
@nodejs-github-bot nodejs-github-bot added the http Issues or PRs related to the http subsystem. label Apr 30, 2020
@ronag
Copy link
Member Author

ronag commented Apr 30, 2020

@nodejs/http

@nodejs-github-bot
Copy link
Collaborator

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

@ronag ronag requested a review from lundibundi May 2, 2020 07:38
mcollina
mcollina approved these changes May 3, 2020
View reviewed changes
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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@ronag
Copy link
Member Author

ronag commented May 4, 2020

Landed in 027e1c7

@ronag ronag closed this May 4, 2020
ronag added a commit that referenced this pull request May 4, 2020
@ronag
http: ensure client request emits close
027e1c7 
If socket creation failed then an error would be
emitted on the client request object, but not
'close' nor would destroyed be set to true.

PR-URL: #33178
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
targos pushed a commit that referenced this pull request May 4, 2020
@ronag @targos
http: ensure client request emits close
74b0e8c 
If socket creation failed then an error would be
emitted on the client request object, but not
'close' nor would destroyed be set to true.

PR-URL: #33178
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@targos targos mentioned this pull request May 4, 2020
Merged
@mastermatt mastermatt mentioned this pull request May 5, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@mcollina mcollina mcollina approved these changes

@lundibundi lundibundi Awaiting requested review from lundibundi

@jasnell jasnell Awaiting requested review from jasnell

@lpinca lpinca Awaiting requested review from lpinca

Assignees
No one assigned
Labels
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.
5 participants
@ronag @nodejs-github-bot @mcollina @cjihrig @targos