Skip to content

http: fix ClientRequest unhandled errors #36970

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
Closed
wants to merge 7 commits into from
Closed

http: fix ClientRequest unhandled errors #36970

wants to merge 7 commits into from

Conversation

@ronag
Copy link
Member

@ronag ronag commented Jan 16, 2021 •
edited
Loading

ClientRequest could somtimes cause an unhandled error from socket.

Fixes: #36931
@ronag
http: fix ClientRequest unhandled errors
4fe8533 
ClientRequest could someone cause an unhandled error from socket. Fixes: nodejs#36931
@ronag ronag added http Issues or PRs related to the http subsystem. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 16, 2021
@ronag ronag requested a review from lpinca January 16, 2021 17:19
ronag
ronag commented Jan 16, 2021
View reviewed changes
ronag
ronag commented Jan 16, 2021
View reviewed changes
@ronag ronag requested a review from dnlup January 16, 2021 17:21
lpinca
lpinca reviewed Jan 16, 2021
View reviewed changes
lpinca
lpinca approved these changes Jan 16, 2021
View reviewed changes
@ronag ronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 16, 2021
@ronag
Copy link
Member Author

ronag commented Jan 16, 2021

@nodejs/http
lpinca
lpinca reviewed Jan 16, 2021
View reviewed changes
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 16, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/35583/
mcollina
mcollina approved these changes Jan 17, 2021
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
@ronag ronag removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 17, 2021
@ronag
Copy link
Member Author

ronag commented Jan 17, 2021

Test needs improvment.
@dnlup
Copy link
Contributor

dnlup commented Jan 18, 2021

I cannot reproduce the timeout error on test/parallel/test-cluster-primary-kill.js, it might be a temporary error.
dnlup
dnlup reviewed Jan 18, 2021
View reviewed changes
Copy link
Contributor

@dnlup dnlup left a comment

Choose a reason for hiding this comment

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

lgtm
@ronag ronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 3, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 3, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/35918/
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 3, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/35919/
lpinca
lpinca reviewed Feb 3, 2021
View reviewed changes
@ronag @lpinca
Update test/parallel/test-http-client-abort3.js
d103ec1 
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 3, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/35920/
lpinca
lpinca reviewed Feb 3, 2021
View reviewed changes
@ronag @lpinca
Update test/parallel/test-http-client-abort3.js
a5cb23a 
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 3, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/35921/
ronag added a commit that referenced this pull request Feb 3, 2021
@ronag
http: fix ClientRequest unhandled errors
fe43bd8 
ClientRequest could someone cause an unhandled error from socket. Fixes: #36931 PR-URL: #36970 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@ronag
Copy link
Member Author

ronag commented Feb 3, 2021

Landed in fe43bd8
@ronag ronag closed this Feb 3, 2021
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
@ronag @danielleadams
http: fix ClientRequest unhandled errors
4cdc5ea 
ClientRequest could someone cause an unhandled error from socket. Fixes: #36931 PR-URL: #36970 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This was referenced Feb 16, 2021
Closed
Merged
@panva panva removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 21, 2023
@Changqing-JING Changqing-JING mentioned this pull request Jun 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. http Issues or PRs related to the http subsystem.

8 participants

@ronag @nodejs-github-bot @dnlup @mcollina @benjamingr @lpinca @panva @targos