GeezerVison

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

GeezerVison is showing github.com — http: fix error message when specifying headerTimeout for createServer by nicksia-vgw · Pull Request #44163 · nodejs/node

Conversation

nicksia-vgw commented Aug 7, 2022 •

edited by VoltrexKeyva

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

This change fixes a misleading error message provided by the error thrown when creating an http server with a headerTimeout that exceeds the requestTimeout.

Code

const server = createServer({
  headersTimeout: 6000, // Limit the amount of time the parser will wait to receive the complete HTTP headers.
  requestTimeout: 3000, // Sets the timeout value in milliseconds for receiving the entire request from the client.
}, () => {});

Error

node:_http_server:406
    throw new codes.ERR_OUT_OF_RANGE('headersTimeout', '>= requestTimeout', headersTimeout);
    ^

RangeError [ERR_OUT_OF_RANGE]: The value of "headersTimeout" is out of range. It must be >= requestTimeout. Received 6000

The header timeout is actually required to be < the request timeout (which is correctly validated), but the error message is wrong - see original snippet below.

_http_server.js.storeHTTPOptions (validation code)

  if (this.requestTimeout > 0 && this.headersTimeout > 0 && this.headersTimeout >= this.requestTimeout) {
    throw new codes.ERR_OUT_OF_RANGE('headersTimeout', '>= requestTimeout', headersTimeout);
  }

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


          http: fix error message when specifying headerTimeout for createServer

9dde596

This change fixes the message on the error received when calling 
http.createServer(...) with a header timeout greater than the request
timeout is incorrect.

The validation requires that the header timeout is lower than the 
request timeout, but the error message asks for the opposite.

nodejs-github-bot commented Aug 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 added http

Issues and PRs related to the http subsystem.

needs-ci PRs that need a full CI run. labels

Aug 7, 2022

ShogunPanda approved these changes

Aug 7, 2022

View reviewed changes

ShogunPanda left a comment

Copy link

Copy Markdown

Contributor

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

mcollina approved these changes

Aug 7, 2022

View reviewed changes

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

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 7, 2022

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

Aug 7, 2022

nodejs-github-bot commented Aug 7, 2022

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

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

lpinca approved these changes

Aug 7, 2022

View reviewed changes

nodejs-github-bot commented Aug 7, 2022

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

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

github-actions Bot mentioned this pull request

Aug 8, 2022

CI Reliability 2022-08-08 nodejs/reliability#340

Open

11 tasks

tniessen approved these changes

Aug 8, 2022

View reviewed changes

ErickWendel approved these changes

Aug 8, 2022

View reviewed changes

github-actions Bot mentioned this pull request

Aug 9, 2022

CI Reliability 2022-08-09 nodejs/reliability#341

Open

8 tasks

rickyes approved these changes

Aug 9, 2022

View reviewed changes

mcollina added commit-queue

PRs queued for automated landing through the Commit Queue.

author ready PRs with CI started, the required approvals, and no outstanding review comments. and removed needs-ci PRs that need a full CI run. labels

Aug 9, 2022

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 9, 2022

nodejs-github-bot merged commit b427924 into nodejs:main

Aug 9, 2022

nodejs-github-bot commented Aug 9, 2022

Copy link

Copy Markdown

Collaborator

Landed in b427924

All reactions

Sorry, something went wrong.

Uh oh!

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

nicksia-vgw deleted the fix-incorrect-headertimeout-error-message branch

August 9, 2022 09:33

This was referenced Aug 10, 2022

CI Reliability 2022-08-10 nodejs/reliability#342

Open

CI Reliability 2022-08-11 nodejs/reliability#343

Open

danielleadams pushed a commit that referenced this pull request

Aug 16, 2022


          http: fix error message when specifying headerTimeout for createServer

f5fcd49

This change fixes the message on the error received when calling
http.createServer(...) with a header timeout greater than the request
timeout is incorrect.

The validation requires that the header timeout is lower than the
request timeout, but the error message asks for the opposite.

PR-URL: #44163
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Erick Wendel <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>

danielleadams mentioned this pull request

Aug 16, 2022

v18.8.0 release proposal #44246

Closed

ruyadorno pushed a commit that referenced this pull request

Aug 23, 2022


          http: fix error message when specifying headerTimeout for createServer

340ca4d

This change fixes the message on the error received when calling
http.createServer(...) with a header timeout greater than the request
timeout is incorrect.

The validation requires that the header timeout is lower than the
request timeout, but the error message asks for the opposite.

PR-URL: #44163
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Erick Wendel <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>

ruyadorno mentioned this pull request

Aug 23, 2022

v18.8.0 release proposal #44353

Merged

juanarbol added dont-land-on-v14.x labels

Sep 29, 2022

juanarbol commented Sep 29, 2022

Copy link

Copy Markdown

Member

This change depends on #41263 which is a semver-major PR

All reactions

Sorry, something went wrong.

Uh oh!

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

ShogunPanda mentioned this pull request

Dec 7, 2022

Fixed http_server timeout logic for requestTimeout and headerTimeout #43354

Closed

coderabbitai Bot mentioned this pull request

Jul 26, 2026

Make Render health checks immediate and deterministic AlexAyvazian/Map#19

Merged

Reviewers

mcollina

mcollina approved these changes

ShogunPanda

ShogunPanda approved these changes

lpinca

lpinca approved these changes

tniessen

tniessen approved these changes

ErickWendel

ErickWendel approved these changes

+1 more reviewer

rickyes

rickyes approved these changes

Reviewers whose approvals may not affect merge requirements

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.

http

Issues and PRs related to the http subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

9 participants

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.

GeezerVison

Conversation

nicksia-vgw commented Aug 7, 2022 • edited by VoltrexKeyva Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 7, 2022

Uh oh!

ShogunPanda left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Aug 7, 2022

Uh oh!

nodejs-github-bot commented Aug 7, 2022

Uh oh!

nodejs-github-bot commented Aug 9, 2022

Uh oh!

juanarbol commented Sep 29, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

nicksia-vgw commented Aug 7, 2022 •

edited by VoltrexKeyva

Loading

Uh oh!

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