GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — http: Improved timeout defaults handling. by ShogunPanda · Pull Request #45778 · nodejs/node

Conversation

@ShogunPanda

@ShogunPanda ShogunPanda commented Dec 7, 2022

Copy link
Copy Markdown
Contributor

This fixes the default of http.Server.headersTimeout by setting it to the minimum between 60 seconds (as it is today) or http.Server.headersTimeout.

Supersedes: #43354
Fixes: #43355

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@ShogunPanda
ShogunPanda requested review from aduh95, mcollina and ronag December 7, 2022 21:12
@nodejs-github-bot

nodejs-github-bot commented Dec 7, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Dec 7, 2022
@ShogunPanda ShogunPanda mentioned this pull request Dec 7, 2022
Fixed http_server timeout logic for requestTimeout and headerTimeout #43354
Closed
mcollina
mcollina approved these changes Dec 7, 2022
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
santigimeno
santigimeno approved these changes Dec 7, 2022
View reviewed changes
aduh95
aduh95 approved these changes Dec 7, 2022
View reviewed changes
Comment thread test/parallel/test-http-server-timeouts-validation.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@aduh95 aduh95 added semver-minor PRs that contain new features and should be released in the next minor version. author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Dec 7, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 8, 2022
@nodejs-github-bot

nodejs-github-bot commented Dec 8, 2022

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anonrig
anonrig approved these changes Dec 8, 2022
View reviewed changes
lpinca
lpinca approved these changes Dec 8, 2022
View reviewed changes
Comment thread lib/_http_server.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Dec 12, 2022

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Dec 13, 2022
CI Reliability 2022-12-13 nodejs/reliability#456
Open
CI Reliability 2022-12-14 nodejs/reliability#457
Open
@nodejs-github-bot

nodejs-github-bot commented Dec 14, 2022

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Dec 14, 2022

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Dec 14, 2022

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Dec 15, 2022
CI Reliability 2022-12-15 nodejs/reliability#458
Open
CI Reliability 2022-12-16 nodejs/reliability#459
Open
CI Reliability 2022-12-17 nodejs/reliability#460
Open
CI Reliability 2022-12-18 nodejs/reliability#461
Open
CI Reliability 2022-12-19 nodejs/reliability#462
Open
jasnell
jasnell approved these changes Dec 19, 2022
View reviewed changes
targos pushed a commit that referenced this pull request Jan 1, 2023
@ShogunPanda @lpinca @targos
http: improved timeout defaults handling
5a334c5
Co-authored-by: Luigi Pinca <[email protected]>
PR-URL: #45778
Fixes: #43355
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@github-actions github-actions Bot mentioned this pull request Jan 2, 2023
CI Reliability 2023-01-02 nodejs/reliability#476
Open
22 tasks
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2023
v19.4.0 proposal #46061
Merged
RafaelGSS added a commit that referenced this pull request Jan 2, 2023
@RafaelGSS
2023-01-03, Version 19.4.0 (Current)
fd77454
Notable changes:

buffer:
  * (SEMVER-MINOR) add buffer.isUtf8 for utf8 validation (Yagiz Nizipli) #45947
deps:
  * disable avx512 for simutf on benchmark ci (Yagiz Nizipli) #45803
  * add simdutf dependency (Yagiz Nizipli) #45803
http:
  * (SEMVER-MINOR) improved timeout defaults handling (Paolo Insogna) #45778
net
  * add autoSelectFamily global getter and setter (Paolo Insogna) #45777
os:
  * (SEMVER-MINOR) add availableParallelism() (Colin Ihrig) #45895
util:
  * add fast path for text-decoder fatal flag (Yagiz Nizipli) #45803

PR-URL: #46061
RafaelGSS added a commit that referenced this pull request Jan 2, 2023
@RafaelGSS
2023-01-03, Version 19.4.0 (Current)
3d55e4b
Notable changes:

buffer:
  * (SEMVER-MINOR) add buffer.isUtf8 for utf8 validation (Yagiz Nizipli) #45947
deps:
  * disable avx512 for simutf on benchmark ci (Yagiz Nizipli) #45803
  * add simdutf dependency (Yagiz Nizipli) #45803
http:
  * (SEMVER-MINOR) improved timeout defaults handling (Paolo Insogna) #45778
net
  * add autoSelectFamily global getter and setter (Paolo Insogna) #45777
os:
  * (SEMVER-MINOR) add availableParallelism() (Colin Ihrig) #45895
util:
  * add fast path for text-decoder fatal flag (Yagiz Nizipli) #45803

PR-URL: #46061
RafaelGSS pushed a commit that referenced this pull request Jan 4, 2023
@ShogunPanda @lpinca @RafaelGSS
http: improved timeout defaults handling
bf254f6
Co-authored-by: Luigi Pinca <[email protected]>
PR-URL: #45778
Fixes: #43355
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 4, 2023
@RafaelGSS
2023-01-05, Version 19.4.0 (Current)
790cc5b
Notable changes:

buffer:
  * (SEMVER-MINOR) add buffer.isUtf8 for utf8 validation (Yagiz Nizipli) #45947
http:
  * (SEMVER-MINOR) improved timeout defaults handling (Paolo Insogna) #45778
net
  * add autoSelectFamily global getter and setter (Paolo Insogna) #45777
os:
  * (SEMVER-MINOR) add availableParallelism() (Colin Ihrig) #45895
util:
  * add fast path for text-decoder fatal flag (Yagiz Nizipli) #45803

PR-URL: #46061
RafaelGSS pushed a commit that referenced this pull request Jan 5, 2023
@ShogunPanda @lpinca @RafaelGSS
http: improved timeout defaults handling
7ea72ee
Co-authored-by: Luigi Pinca <[email protected]>
PR-URL: #45778
Fixes: #43355
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 5, 2023
@RafaelGSS
2023-01-05, Version 19.4.0 (Current)
b181545
Notable changes:

buffer:
  * (SEMVER-MINOR) add buffer.isUtf8 for utf8 validation (Yagiz Nizipli) #45947
http:
  * (SEMVER-MINOR) improved timeout defaults handling (Paolo Insogna) #45778
net
  * add autoSelectFamily global getter and setter (Paolo Insogna) #45777
os:
  * (SEMVER-MINOR) add availableParallelism() (Colin Ihrig) #45895
util:
  * add fast path for text-decoder fatal flag (Yagiz Nizipli) #45803

PR-URL: #46061
RafaelGSS added a commit that referenced this pull request Jan 5, 2023
@RafaelGSS
2023-01-06, Version 19.4.0 (Current)
e58ed6d
Notable changes:

buffer:
  * (SEMVER-MINOR) add buffer.isUtf8 for utf8 validation (Yagiz Nizipli) #45947
http:
  * (SEMVER-MINOR) improved timeout defaults handling (Paolo Insogna) #45778
net
  * add autoSelectFamily global getter and setter (Paolo Insogna) #45777
os:
  * (SEMVER-MINOR) add availableParallelism() (Colin Ihrig) #45895
util:
  * add fast path for text-decoder fatal flag (Yagiz Nizipli) #45803

PR-URL: #46061
RafaelGSS added a commit that referenced this pull request Jan 6, 2023
@RafaelGSS
2023-01-06, Version 19.4.0 (Current)
22a2ec6
Notable changes:

buffer:
  * (SEMVER-MINOR) add buffer.isUtf8 for utf8 validation (Yagiz Nizipli) #45947
http:
  * (SEMVER-MINOR) improved timeout defaults handling (Paolo Insogna) #45778
net
  * add autoSelectFamily global getter and setter (Paolo Insogna) #45777
os:
  * (SEMVER-MINOR) add availableParallelism() (Colin Ihrig) #45895
util:
  * add fast path for text-decoder fatal flag (Yagiz Nizipli) #45803

PR-URL: #46061
@juanarbol juanarbol added the backport-blocked-v18.x label Jan 24, 2023
@juanarbol

This comment was marked as off-topic.

Sign in to view
@juanarbol juanarbol removed the backport-blocked-v18.x label Jan 24, 2023
juanarbol pushed a commit that referenced this pull request Jan 26, 2023
@ShogunPanda @lpinca @juanarbol
http: improved timeout defaults handling
ac649a7
Co-authored-by: Luigi Pinca <[email protected]>
PR-URL: #45778
Fixes: #43355
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@juanarbol juanarbol mentioned this pull request Jan 28, 2023
V18.14.0 proposal #46396
Merged
juanarbol pushed a commit that referenced this pull request Jan 31, 2023
@ShogunPanda @lpinca @juanarbol
http: improved timeout defaults handling
3c0c5e0
Co-authored-by: Luigi Pinca <[email protected]>
PR-URL: #45778
Fixes: #43355
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
juanarbol added a commit that referenced this pull request Feb 1, 2023
@juanarbol
2023-02-02, Version 18.14.0 'Hydrogen' (LTS)
220a5a1
Notable changes:

* deps:
  * upgrade npm to 9.3.1 (npm team) #46242
* doc:
  * add parallelism note to os.cpus() (Colin Ihrig) #45895
* http:
  * join authorization headers (Marco Ippolito) #45982
  * improved timeout defaults handling (Paolo Insogna) #45778
* stream:
  * implement finished() for ReadableStream and WritableStream (Debadree Chatterjee) #46205

PR-URL: #46396
juanarbol added a commit that referenced this pull request Feb 2, 2023
@juanarbol
2023-02-02, Version 18.14.0 'Hydrogen' (LTS)
66ab03d
Notable changes:

* deps:
  * upgrade npm to 9.3.1 (npm team) #46242
* doc:
  * add parallelism note to os.cpus() (Colin Ihrig) #45895
* http:
  * join authorization headers (Marco Ippolito) #45982
  * improved timeout defaults handling (Paolo Insogna) #45778
* stream:
  * implement finished() for ReadableStream and WritableStream (Debadree Chatterjee) #46205

PR-URL: #46396
@ShogunPanda
ShogunPanda deleted the http-server-timeout-logic branch February 9, 2023 15:31
@slvrtrn slvrtrn mentioned this pull request Feb 15, 2023
Address Node.js 18.14.0 breaking changes ClickHouse/clickhouse-js#136
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes
@jasnell jasnell jasnell approved these changes
@santigimeno santigimeno santigimeno approved these changes
@lpinca lpinca lpinca approved these changes
@anonrig anonrig anonrig approved these changes
@aduh95 aduh95 aduh95 approved these changes
@ronag ronag Awaiting requested review from ronag

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-squash PRs the Commit Queue should land as one squashed commit. http Issues and PRs related to the http subsystem. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

http_server handles headersTimeout and requestTimeout checking wrong

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@ShogunPanda @nodejs-github-bot @juanarbol @mcollina @jasnell @santigimeno @lpinca @anonrig @aduh95
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.