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: make response.setTimeout() work #34913

Closed
wants to merge 1 commit into from
Closed

http: make response.setTimeout() work #34913

wants to merge 1 commit into from

Conversation

juanarbol
Copy link
Member

@juanarbol juanarbol commented Aug 24, 2020 •
edited
Loading

Fixes: #33734

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

Sorry, I just saw #33734 "inactive" but solved, just re-do what Ben did.

@juanarbol juanarbol requested a review from a team as a code owner August 24, 2020 21:15
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 24, 2020 •
edited by juanarbol
Loading

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added the http Issues or PRs related to the http subsystem. label Aug 24, 2020
@aduh95
Copy link
Contributor

aduh95 commented Aug 24, 2020

Why not include the original commit to credit the author?

@juanarbol
Copy link
Member Author

@aduh95 I don't have that git skill (yet), hold on. I'm doing that.

@juanarbol juanarbol force-pushed the juanarbol/incomingmessage-timeout branch from 0084931 to 9c588f2 Compare August 24, 2020 21:57
@juanarbol
Copy link
Member Author

juanarbol commented Aug 24, 2020 •
edited
Loading

@aduh95 I've cherry picked the @bnoordhuis commit (BTW sorry, I did not mean to take credits of anything), now the next question for Ben is if he wants this to be landed.

@rickyes rickyes added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Aug 27, 2020
Open
10 tasks
@juanarbol juanarbol force-pushed the juanarbol/incomingmessage-timeout branch from 9c588f2 to 9755a47 Compare September 9, 2020 01:29
@gengjiawen gengjiawen added needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Sep 12, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 12, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@juanarbol juanarbol added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 12, 2020
@juanarbol juanarbol added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 29, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 29, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 5, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 5, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@juanarbol juanarbol added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Oct 7, 2020
juanarbol pushed a commit to juanarbol/node that referenced this pull request Oct 7, 2020
@bnoordhuis @juanarbol
http: make response.setTimeout() work
7b26322 
Fixes: nodejs#33734

PR-URL: nodejs#34913
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@juanarbol
Copy link
Member Author

Landed in 7b26322

@juanarbol juanarbol closed this Oct 7, 2020
BethGriggs pushed a commit that referenced this pull request Oct 13, 2020
@bnoordhuis @BethGriggs
http: make response.setTimeout() work
2f13199 
Fixes: #33734

PR-URL: #34913
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 14, 2020
Merged
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@bnoordhuis @joesepi
http: make response.setTimeout() work
d54a696 
Fixes: nodejs#33734

PR-URL: nodejs#34913
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@juanarbol juanarbol deleted the juanarbol/incomingmessage-timeout branch January 19, 2021 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rickyes rickyes rickyes approved these changes

@trivikr trivikr trivikr approved these changes

@lpinca lpinca lpinca approved these changes

@bnoordhuis bnoordhuis Awaiting requested review from bnoordhuis

Assignees
No one assigned
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

http.IncomingMessage doesn't fire callback on 'timeout'
8 participants
@juanarbol @nodejs-github-bot @aduh95 @lpinca @trivikr @rickyes @gengjiawen @bnoordhuis