http: refactor headersTimeout and requestTimeout logic #41263

ShogunPanda · 2021-12-21T15:07:02Z

This PR completely refactors the timeout handling of http.Server as follow:

The server maintains a list of connections, backed by a std::set wrapper in the C++ layer.
The server starts a new interval that every connectionsCheckingInterval milliseconds (a new option in the http.createServer) checks if there any connection for which headersTimeout or requestTimeout has gone off. If that's the case, a timeout event is thrown. The default handler for this event is to respond to the client with status code 408 and then close the connection. This also unifies the behavior for both timeouts.
headersTimeout, requestTimeout and keepAliveTimeout are now supported directly in http.createServer function and they are sanitized.
Default for headersTimeout is now 60 seconds, default for requestTimeout is now 5 minutes.

The use of the interval is needed for performance reason in order to check for the timeouts even if no data is received (as it is at the moment) and this fixes #33440.

The use of a connection list also constitutes a preparatory work for #41578.

nodejs-github-bot · 2021-12-21T15:07:06Z

Review requested:

@nodejs/http
@nodejs/net

mcollina

lgtm

Have you benchmarked it?

mcollina · 2021-12-21T16:24:20Z

@ronag wdyt?

ShogunPanda · 2021-12-21T16:26:35Z

lgtm

Have you benchmarked it?

No, I did not. 🤦
Tomorrow I will post benchmarks here.

mcollina · 2021-12-23T09:10:03Z

Perf CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1075/

nodejs-github-bot · 2021-12-23T17:29:19Z

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

nodejs-github-bot · 2021-12-27T08:43:18Z

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

mcollina · 2021-12-28T11:50:37Z

It seems the benchmark got stuck, here is another try: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1076/

mcollina · 2021-12-29T09:40:58Z

@nodejs/build could you take a look what's problematic about the http benchmarks? they get stuck on the bench machine: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1076/console (this happened twice)

mcollina · 2022-01-05T08:53:08Z

Bench CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1081/

(limited to the simple test)

PR-URL: nodejs#41263 Fixes: nodejs#33440 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>

Notable Changes: Deprecations and Removals: - (SEMVER-MAJOR) fs: runtime deprecate string coercion in `fs.write`, `fs.writeFileSync` (Livia Medeiros) (#42607) - (SEMVER-MAJOR) dns: remove `dns.lookup` and `dnsPromises.lookup` options type coercion (Antoine du Hamel) (#41431) - (SEMVER-MAJOR) process: runtime deprecate multipleResolves (Benjamin Gruenbaum) (#41896) - (SEMVER-MAJOR) stream: remove thenable support (Robert Nagy) (#40773) - (SEMVER-MAJOR) tls: move tls.parseCertString to end-of-life (Tobias Nießen) (#41479) fetch (experimental): An experimental fetch API is available on the global scope by default. The implementation is based upon https://undici.nodejs.org/#/, an HTTP/1.1 client written for Node.js by contributors to the project. Through this addition, the following globals are made available: `fetch` , `FormData`, `Headers`, `Request`, `Response`. Disable this API with the `--no-experimental-fetch` command-line flag. Contributed by Michaël Zasso in #41811. HTTP Timeouts: `server.headersTimeout`, which limits the amount of time the parser will wait to receive the complete HTTP headers, is now set to `60000` (60 seconds) by default. `server.requestTimeout`, which sets the timeout value in milliseconds for receiving the entire request from the client, is now set to `300000` (5 minutes) by default. If these timeouts expire, the server responds with status 408 without forwarding the request to the request listener and then closes the connection. Both timeouts must be set to a non-zero value to protect against potential Denial-of-Service attacks in case the server is deployed without a reverse proxy in front. Contributed by Paolo Insogna in #41263. Test Runner module (experimental): The `node:test` module facilitates the creation of JavaScript tests that report results in TAP format. This module is only available under the `node:` scheme. Contributed by Colin Ihrig in #42325. Toolchain and Compiler Upgrades: - Prebuilt binaries for Linux are now built on Red Hat Enterprise Linux (RHEL) 8 and are compatible with Linux distributions based on glibc 2.28 or later, for example, Debian 10, RHEL 8, Ubuntu 20.04. - Prebuilt binaries for macOS now require macOS 10.15 or later. - For AIX the minimum supported architecture has been raised from Power 7 to Power 8. Prebuilt binaries for 32-bit Windows will initially not be available due to issues building the V8 dependency in Node.js. We hope to restore 32-bit Windows binaries for Node.js 18 with a future V8 update. Node.js does not support running on operating systems that are no longer supported by their vendor. For operating systems where their vendor has planned to end support earlier than April 2025, such as Windows 8.1 (January 2023) and Windows Server 2012 R2 (October 2023), support for Node.js 18 will end at the earlier date. Full details about the supported toolchains and compilers are documented in the Node.js `BUILDING.md` file. Contributed by Richard Lau in #42292, #42604 and #42659 , and Michaël Zasso in #42105 and #42666. V8 10.1: The V8 engine is updated to version 10.1, which is part of Chromium 101. Compared to the version included in Node.js 17.9.0, the following new features are included: - The `findLast` and `findLastIndex` array methods. - Improvements to the `Intl.Locale` API. - The `Intl.supportedValuesOf` function. - Improved performance of class fields and private class methods (the initialization of them is now as fast as ordinary property stores). The data format returned by the serialization API (`v8.serialize(value)`) has changed, and cannot be deserialized by earlier versions of Node.js. On the other hand, it is still possible to deserialize the previous format, as the API is backwards-compatible. Contributed by Michaël Zasso in #42657. Web Streams API (experimental): Node.js now exposes the experimental implementation of the Web Streams API on the global scope. This means the following APIs are now globally available: - `ReadableStream`, `ReadableStreamDefaultReader`, `ReadableStreamBYOBReader`, `ReadableStreamBYOBRequest`, `ReadableByteStreamController`, `ReadableStreamDefaultController`, `TransformStream`, `TransformStreamDefaultController`, `WritableStream`, `WritableStreamDefaultWriter`, `WritableStreamDefaultController`, `ByteLengthQueuingStrategy`, `CountQueuingStrategy`, `TextEncoderStream`, `TextDecoderStream`, `CompressionStream`, `DecompressionStream`. Contributed James Snell in #39062, and Antoine du Hamel in #42225. Other Notable Changes: - (SEMVER-MAJOR) buffer: expose Blob as a global (James M Snell) (#41270) - (SEMVER-MAJOR) child\_process: improve argument validation (Rich Trott) (#41305) - doc: add RafaelGSS to collaborators (RafaelGSS) (#42718) - (SEMVER-MAJOR) http: make TCP noDelay enabled by default (Paolo Insogna) (#42163) - (SEMVER-MAJOR) net: make `server.address()` return an integer for `family` (Antoine du Hamel) (#41431) - (SEMVER-MAJOR) worker: expose BroadcastChannel as a global (James M Snell) (#41271) - (SEMVER-MAJOR) worker: graduate BroadcastChannel to supported (James M Snell) (#41271) Semver-Major Commits: - (SEMVER-MAJOR) assert,util: compare RegExp.lastIndex while using deep equal checks (Ruben Bridgewater) (#41020) - (SEMVER-MAJOR) buffer: refactor `byteLength` to remove outdated optimizations (Rongjian Zhang) (#38545) - (SEMVER-MAJOR) buffer: expose Blob as a global (James M Snell) (#41270) - (SEMVER-MAJOR) buffer: graduate Blob from experimental (James M Snell) (#41270) - (SEMVER-MAJOR) build: make x86 Windows support temporarily experimental (Michaël Zasso) (#42666) - (SEMVER-MAJOR) build: bump macOS deployment target to 10.15 (Richard Lau) (#42292) - (SEMVER-MAJOR) build: downgrade Windows 8.1 and server 2012 R2 to experimental (Michaël Zasso) (#42105) - (SEMVER-MAJOR) child\_process: improve argument validation (Rich Trott) (#41305) - (SEMVER-MAJOR) cluster: make `kill` to be just `process.kill` (Bar Admoni) (#34312) - (SEMVER-MAJOR) crypto: cleanup validation (Mohammed Keyvanzadeh) (#39841) - (SEMVER-MAJOR) crypto: prettify othername in PrintGeneralName (Tobias Nießen) (#42123) - (SEMVER-MAJOR) crypto: fix X509Certificate toLegacyObject (Tobias Nießen) (#42124) - (SEMVER-MAJOR) crypto: use RFC2253 format in PrintGeneralName (Tobias Nießen) (#42002) - (SEMVER-MAJOR) crypto: change default check(Host|Email) behavior (Tobias Nießen) (#41600) - (SEMVER-MAJOR) deps: V8: cherry-pick semver-major commits from 10.2 (Michaël Zasso) (#42657) - (SEMVER-MAJOR) deps: update V8 to 10.1.124.6 (Michaël Zasso) (#42657) - (SEMVER-MAJOR) deps: update V8 to 9.8.177.9 (Michaël Zasso) (#41610) - (SEMVER-MAJOR) deps: update V8 to 9.7.106.18 (Michaël Zasso) (#40907) - (SEMVER-MAJOR) dns: remove `dns.lookup` and `dnsPromises.lookup` options type coercion (Antoine du Hamel) (#41431) - (SEMVER-MAJOR) doc: update minimum glibc requirements for Linux (Richard Lau) (#42659) - (SEMVER-MAJOR) doc: update AIX minimum supported arch (Richard Lau) (#42604) - (SEMVER-MAJOR) fs: runtime deprecate string coercion in `fs.write`, `fs.writeFileSync` (Livia Medeiros) (#42607) - (SEMVER-MAJOR) http: refactor headersTimeout and requestTimeout logic (Paolo Insogna) (#41263) - (SEMVER-MAJOR) http: make TCP noDelay enabled by default (Paolo Insogna) (#42163) - (SEMVER-MAJOR) lib: enable fetch by default (Michaël Zasso) (#41811) - (SEMVER-MAJOR) lib: replace validator and error (Mohammed Keyvanzadeh) (#41678) - (SEMVER-MAJOR) module,repl: support 'node:'-only core modules (Colin Ihrig) (#42325) - (SEMVER-MAJOR) net: make `server.address()` return an integer for `family` (Antoine du Hamel) (#41431) - (SEMVER-MAJOR) process: disallow some uses of Object.defineProperty() on process.env (Himself65) (#28006) - (SEMVER-MAJOR) process: runtime deprecate multipleResolves (Benjamin Gruenbaum) (#41896) - (SEMVER-MAJOR) readline: fix question still called after closed (Xuguang Mei) (#42464) - (SEMVER-MAJOR) stream: remove thenable support (Robert Nagy) (#40773) - (SEMVER-MAJOR) stream: expose web streams globals, remove runtime experimental warning (Antoine du Hamel) (#42225) - (SEMVER-MAJOR) stream: need to cleanup event listeners if last stream is readable (Xuguang Mei) (#41954) - (SEMVER-MAJOR) stream: revert revert `map` spec compliance (Benjamin Gruenbaum) (#41933) - (SEMVER-MAJOR) stream: throw invalid arg type from End Of Stream (Jithil P Ponnan) (#41766) - (SEMVER-MAJOR) stream: don't emit finish after destroy (Robert Nagy) (#40852) - (SEMVER-MAJOR) stream: add errored and closed props (Robert Nagy) (#40696) - (SEMVER-MAJOR) test: add initial test module (Colin Ihrig) (#42325) - (SEMVER-MAJOR) timers: refactor internal classes to ES2015 syntax (Rabbit) (#37408) - (SEMVER-MAJOR) tls: represent registeredID numerically always (Tobias Nießen) (#41561) - (SEMVER-MAJOR) tls: move tls.parseCertString to end-of-life (Tobias Nießen) (#41479) - (SEMVER-MAJOR) url: throw on NULL in IPv6 hostname (Rich Trott) (#42313) - (SEMVER-MAJOR) v8: make v8.writeHeapSnapshot() error codes consistent (Darshan Sen) (#42577) - (SEMVER-MAJOR) v8: make writeHeapSnapshot throw if fopen fails (Antonio Román) (#41373) - (SEMVER-MAJOR) worker: expose BroadcastChannel as a global (James M Snell) (#41271) - (SEMVER-MAJOR) worker: graduate BroadcastChannel to supported (James M Snell) (#41271) PR-URL: #42262

Refs: nodejs#41263

Refs: #41263 PR-URL: #42846 Fixes: #42741 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>

PR-URL: nodejs#41263 Fixes: nodejs#33440 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>

Notable Changes: Deprecations and Removals: - (SEMVER-MAJOR) fs: runtime deprecate string coercion in `fs.write`, `fs.writeFileSync` (Livia Medeiros) (nodejs#42607) - (SEMVER-MAJOR) dns: remove `dns.lookup` and `dnsPromises.lookup` options type coercion (Antoine du Hamel) (nodejs#41431) - (SEMVER-MAJOR) process: runtime deprecate multipleResolves (Benjamin Gruenbaum) (nodejs#41896) - (SEMVER-MAJOR) stream: remove thenable support (Robert Nagy) (nodejs#40773) - (SEMVER-MAJOR) tls: move tls.parseCertString to end-of-life (Tobias Nießen) (nodejs#41479) fetch (experimental): An experimental fetch API is available on the global scope by default. The implementation is based upon https://undici.nodejs.org/#/, an HTTP/1.1 client written for Node.js by contributors to the project. Through this addition, the following globals are made available: `fetch` , `FormData`, `Headers`, `Request`, `Response`. Disable this API with the `--no-experimental-fetch` command-line flag. Contributed by Michaël Zasso in nodejs#41811. HTTP Timeouts: `server.headersTimeout`, which limits the amount of time the parser will wait to receive the complete HTTP headers, is now set to `60000` (60 seconds) by default. `server.requestTimeout`, which sets the timeout value in milliseconds for receiving the entire request from the client, is now set to `300000` (5 minutes) by default. If these timeouts expire, the server responds with status 408 without forwarding the request to the request listener and then closes the connection. Both timeouts must be set to a non-zero value to protect against potential Denial-of-Service attacks in case the server is deployed without a reverse proxy in front. Contributed by Paolo Insogna in nodejs#41263. Test Runner module (experimental): The `node:test` module facilitates the creation of JavaScript tests that report results in TAP format. This module is only available under the `node:` scheme. Contributed by Colin Ihrig in nodejs#42325. Toolchain and Compiler Upgrades: - Prebuilt binaries for Linux are now built on Red Hat Enterprise Linux (RHEL) 8 and are compatible with Linux distributions based on glibc 2.28 or later, for example, Debian 10, RHEL 8, Ubuntu 20.04. - Prebuilt binaries for macOS now require macOS 10.15 or later. - For AIX the minimum supported architecture has been raised from Power 7 to Power 8. Prebuilt binaries for 32-bit Windows will initially not be available due to issues building the V8 dependency in Node.js. We hope to restore 32-bit Windows binaries for Node.js 18 with a future V8 update. Node.js does not support running on operating systems that are no longer supported by their vendor. For operating systems where their vendor has planned to end support earlier than April 2025, such as Windows 8.1 (January 2023) and Windows Server 2012 R2 (October 2023), support for Node.js 18 will end at the earlier date. Full details about the supported toolchains and compilers are documented in the Node.js `BUILDING.md` file. Contributed by Richard Lau in nodejs#42292, nodejs#42604 and nodejs#42659 , and Michaël Zasso in nodejs#42105 and nodejs#42666. V8 10.1: The V8 engine is updated to version 10.1, which is part of Chromium 101. Compared to the version included in Node.js 17.9.0, the following new features are included: - The `findLast` and `findLastIndex` array methods. - Improvements to the `Intl.Locale` API. - The `Intl.supportedValuesOf` function. - Improved performance of class fields and private class methods (the initialization of them is now as fast as ordinary property stores). The data format returned by the serialization API (`v8.serialize(value)`) has changed, and cannot be deserialized by earlier versions of Node.js. On the other hand, it is still possible to deserialize the previous format, as the API is backwards-compatible. Contributed by Michaël Zasso in nodejs#42657. Web Streams API (experimental): Node.js now exposes the experimental implementation of the Web Streams API on the global scope. This means the following APIs are now globally available: - `ReadableStream`, `ReadableStreamDefaultReader`, `ReadableStreamBYOBReader`, `ReadableStreamBYOBRequest`, `ReadableByteStreamController`, `ReadableStreamDefaultController`, `TransformStream`, `TransformStreamDefaultController`, `WritableStream`, `WritableStreamDefaultWriter`, `WritableStreamDefaultController`, `ByteLengthQueuingStrategy`, `CountQueuingStrategy`, `TextEncoderStream`, `TextDecoderStream`, `CompressionStream`, `DecompressionStream`. Contributed James Snell in nodejs#39062, and Antoine du Hamel in nodejs#42225. Other Notable Changes: - (SEMVER-MAJOR) buffer: expose Blob as a global (James M Snell) (nodejs#41270) - (SEMVER-MAJOR) child\_process: improve argument validation (Rich Trott) (nodejs#41305) - doc: add RafaelGSS to collaborators (RafaelGSS) (nodejs#42718) - (SEMVER-MAJOR) http: make TCP noDelay enabled by default (Paolo Insogna) (nodejs#42163) - (SEMVER-MAJOR) net: make `server.address()` return an integer for `family` (Antoine du Hamel) (nodejs#41431) - (SEMVER-MAJOR) worker: expose BroadcastChannel as a global (James M Snell) (nodejs#41271) - (SEMVER-MAJOR) worker: graduate BroadcastChannel to supported (James M Snell) (nodejs#41271) Semver-Major Commits: - (SEMVER-MAJOR) assert,util: compare RegExp.lastIndex while using deep equal checks (Ruben Bridgewater) (nodejs#41020) - (SEMVER-MAJOR) buffer: refactor `byteLength` to remove outdated optimizations (Rongjian Zhang) (nodejs#38545) - (SEMVER-MAJOR) buffer: expose Blob as a global (James M Snell) (nodejs#41270) - (SEMVER-MAJOR) buffer: graduate Blob from experimental (James M Snell) (nodejs#41270) - (SEMVER-MAJOR) build: make x86 Windows support temporarily experimental (Michaël Zasso) (nodejs#42666) - (SEMVER-MAJOR) build: bump macOS deployment target to 10.15 (Richard Lau) (nodejs#42292) - (SEMVER-MAJOR) build: downgrade Windows 8.1 and server 2012 R2 to experimental (Michaël Zasso) (nodejs#42105) - (SEMVER-MAJOR) child\_process: improve argument validation (Rich Trott) (nodejs#41305) - (SEMVER-MAJOR) cluster: make `kill` to be just `process.kill` (Bar Admoni) (nodejs#34312) - (SEMVER-MAJOR) crypto: cleanup validation (Mohammed Keyvanzadeh) (nodejs#39841) - (SEMVER-MAJOR) crypto: prettify othername in PrintGeneralName (Tobias Nießen) (nodejs#42123) - (SEMVER-MAJOR) crypto: fix X509Certificate toLegacyObject (Tobias Nießen) (nodejs#42124) - (SEMVER-MAJOR) crypto: use RFC2253 format in PrintGeneralName (Tobias Nießen) (nodejs#42002) - (SEMVER-MAJOR) crypto: change default check(Host|Email) behavior (Tobias Nießen) (nodejs#41600) - (SEMVER-MAJOR) deps: V8: cherry-pick semver-major commits from 10.2 (Michaël Zasso) (nodejs#42657) - (SEMVER-MAJOR) deps: update V8 to 10.1.124.6 (Michaël Zasso) (nodejs#42657) - (SEMVER-MAJOR) deps: update V8 to 9.8.177.9 (Michaël Zasso) (nodejs#41610) - (SEMVER-MAJOR) deps: update V8 to 9.7.106.18 (Michaël Zasso) (nodejs#40907) - (SEMVER-MAJOR) dns: remove `dns.lookup` and `dnsPromises.lookup` options type coercion (Antoine du Hamel) (nodejs#41431) - (SEMVER-MAJOR) doc: update minimum glibc requirements for Linux (Richard Lau) (nodejs#42659) - (SEMVER-MAJOR) doc: update AIX minimum supported arch (Richard Lau) (nodejs#42604) - (SEMVER-MAJOR) fs: runtime deprecate string coercion in `fs.write`, `fs.writeFileSync` (Livia Medeiros) (nodejs#42607) - (SEMVER-MAJOR) http: refactor headersTimeout and requestTimeout logic (Paolo Insogna) (nodejs#41263) - (SEMVER-MAJOR) http: make TCP noDelay enabled by default (Paolo Insogna) (nodejs#42163) - (SEMVER-MAJOR) lib: enable fetch by default (Michaël Zasso) (nodejs#41811) - (SEMVER-MAJOR) lib: replace validator and error (Mohammed Keyvanzadeh) (nodejs#41678) - (SEMVER-MAJOR) module,repl: support 'node:'-only core modules (Colin Ihrig) (nodejs#42325) - (SEMVER-MAJOR) net: make `server.address()` return an integer for `family` (Antoine du Hamel) (nodejs#41431) - (SEMVER-MAJOR) process: disallow some uses of Object.defineProperty() on process.env (Himself65) (nodejs#28006) - (SEMVER-MAJOR) process: runtime deprecate multipleResolves (Benjamin Gruenbaum) (nodejs#41896) - (SEMVER-MAJOR) readline: fix question still called after closed (Xuguang Mei) (nodejs#42464) - (SEMVER-MAJOR) stream: remove thenable support (Robert Nagy) (nodejs#40773) - (SEMVER-MAJOR) stream: expose web streams globals, remove runtime experimental warning (Antoine du Hamel) (nodejs#42225) - (SEMVER-MAJOR) stream: need to cleanup event listeners if last stream is readable (Xuguang Mei) (nodejs#41954) - (SEMVER-MAJOR) stream: revert revert `map` spec compliance (Benjamin Gruenbaum) (nodejs#41933) - (SEMVER-MAJOR) stream: throw invalid arg type from End Of Stream (Jithil P Ponnan) (nodejs#41766) - (SEMVER-MAJOR) stream: don't emit finish after destroy (Robert Nagy) (nodejs#40852) - (SEMVER-MAJOR) stream: add errored and closed props (Robert Nagy) (nodejs#40696) - (SEMVER-MAJOR) test: add initial test module (Colin Ihrig) (nodejs#42325) - (SEMVER-MAJOR) timers: refactor internal classes to ES2015 syntax (Rabbit) (nodejs#37408) - (SEMVER-MAJOR) tls: represent registeredID numerically always (Tobias Nießen) (nodejs#41561) - (SEMVER-MAJOR) tls: move tls.parseCertString to end-of-life (Tobias Nießen) (nodejs#41479) - (SEMVER-MAJOR) url: throw on NULL in IPv6 hostname (Rich Trott) (nodejs#42313) - (SEMVER-MAJOR) v8: make v8.writeHeapSnapshot() error codes consistent (Darshan Sen) (nodejs#42577) - (SEMVER-MAJOR) v8: make writeHeapSnapshot throw if fopen fails (Antonio Román) (nodejs#41373) - (SEMVER-MAJOR) worker: expose BroadcastChannel as a global (James M Snell) (nodejs#41271) - (SEMVER-MAJOR) worker: graduate BroadcastChannel to supported (James M Snell) (nodejs#41271) PR-URL: nodejs#42262

Refs: nodejs#41263 PR-URL: nodejs#42846 Fixes: nodejs#42741 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>

Refs: #41263 PR-URL: #42846 Fixes: #42741 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>

bnoordhuis · 2022-07-07T10:48:28Z

I'm looking into optimizing the C++ parser and this...

The server maintains a list of connections, backed by a std::set wrapper in the C++ layer.

...makes that unnecessarily hard.

IMO, it conflates things. The parser is, well, a parser. The concepts of connections and timeouts belong at the I/O layer.

(In defense of this PR: node_http_parser.cc was already conflating them but now it's worse. edit: using a weak object also makes it costlier than it needs to be.)

Anyway, just a heads up that I plan on redoing the C++ bits from this PR.

mcollina · 2022-07-07T11:02:54Z

That'd be amazing @bnoordhuis!

ShogunPanda · 2022-07-07T12:14:15Z

That would be great sir!
The less I mess with C++ the best I sleep!

julien-f · 2023-01-10T16:47:45Z

doc/api/http.md

 -->

-* {number} **Default:** `0`
+* {number} **Default:** `300000`


Hey! I believe this change should be added to https.md as well 🙂

@julien-f Yes, it should. Are you willing to send a PR about this or will I do that?

Please do it, I don't have the time at the moment.

Thank you 🙂

mcollina added the semver-major PRs that contain breaking changes and should be released in the next major version. label Dec 21, 2021

mcollina approved these changes Dec 21, 2021

View reviewed changes

Mesteery added the needs-benchmark-ci PR that need a benchmark CI run. label Dec 21, 2021

ronag approved these changes Dec 23, 2021

View reviewed changes

This was referenced Dec 24, 2021

CI Reliability 2021-12-24 nodejs/reliability#154

Open

CI Reliability 2021-12-25 nodejs/reliability#155

Open

CI Reliability 2021-12-26 nodejs/reliability#156

Open

CI Reliability 2021-12-27 nodejs/reliability#157

Open

github-actions bot mentioned this pull request Dec 28, 2021

CI Reliability 2021-12-28 nodejs/reliability#158

Open

17 tasks

github-actions bot mentioned this pull request Dec 29, 2021

CI Reliability 2021-12-29 nodejs/reliability#159

Open

17 tasks

BridgeAR removed the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Apr 13, 2022

github-actions bot mentioned this pull request Apr 14, 2022

CI Reliability 2022-04-14 nodejs/reliability#251

Open

24 tasks

ShogunPanda deleted the headers-timeout-improvement branch April 14, 2022 13:38

richardlau mentioned this pull request Apr 14, 2022

Investigate flaky test-http-server-request-timeout-keepalive #42741

Open

This was referenced Apr 15, 2022

CI Reliability 2022-04-15 nodejs/reliability#252

Open

CI Reliability 2022-04-16 nodejs/reliability#253

Open

CI Reliability 2022-04-17 nodejs/reliability#254

Open

CI Reliability 2022-04-18 nodejs/reliability#255

Open

github-actions bot mentioned this pull request Apr 19, 2022

CI Reliability 2022-04-19 nodejs/reliability#256

Open

16 tasks

tniessen added a commit to tniessen/node that referenced this pull request Apr 24, 2022

test: fix flaky HTTP server tests

50690e9

Refs: nodejs#41263

tniessen mentioned this pull request Apr 24, 2022

test: fix flaky HTTP server tests #42846

Merged

juanarbol mentioned this pull request May 30, 2022

http: added closeAllConnections and closeIdleConnections to http.server #42812

Closed

juanarbol mentioned this pull request Sep 29, 2022

http: fix error message when specifying headerTimeout for createServer #44163

Merged

julien-f reviewed Jan 10, 2023

View reviewed changes

marco-ippolito mentioned this pull request Jan 12, 2023

doc: https update default request timeout #46184

Merged

smaye81 mentioned this pull request May 24, 2023

Disable flaky timeouts in test server connectrpc/connect-es#651

Merged

franciszek-koltuniuk-red mentioned this pull request Jun 2, 2023

http: fix for handling on boot timers headers and request #48291

Merged

climba03003 mentioned this pull request Feb 18, 2024

http: fix close return value mismatch between doc and implementation #51797

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

http: refactor headersTimeout and requestTimeout logic #41263

http: refactor headersTimeout and requestTimeout logic #41263

ShogunPanda commented Dec 21, 2021 •

edited

Loading

nodejs-github-bot commented Dec 21, 2021

mcollina left a comment

mcollina commented Dec 21, 2021

ShogunPanda commented Dec 21, 2021

mcollina commented Dec 23, 2021

nodejs-github-bot commented Dec 23, 2021

nodejs-github-bot commented Dec 27, 2021

mcollina commented Dec 28, 2021

mcollina commented Dec 29, 2021 •

edited

Loading

mcollina commented Jan 5, 2022

bnoordhuis commented Jul 7, 2022 •

edited

Loading

mcollina commented Jul 7, 2022

ShogunPanda commented Jul 7, 2022

julien-f Jan 10, 2023

ShogunPanda Jan 12, 2023

julien-f Jan 12, 2023

ShogunPanda Jan 12, 2023

http: refactor headersTimeout and requestTimeout logic #41263

http: refactor headersTimeout and requestTimeout logic #41263

Conversation

ShogunPanda commented Dec 21, 2021 • edited Loading

nodejs-github-bot commented Dec 21, 2021

mcollina left a comment

Choose a reason for hiding this comment

mcollina commented Dec 21, 2021

ShogunPanda commented Dec 21, 2021

mcollina commented Dec 23, 2021

nodejs-github-bot commented Dec 23, 2021

nodejs-github-bot commented Dec 27, 2021

mcollina commented Dec 28, 2021

mcollina commented Dec 29, 2021 • edited Loading

mcollina commented Jan 5, 2022

bnoordhuis commented Jul 7, 2022 • edited Loading

mcollina commented Jul 7, 2022

ShogunPanda commented Jul 7, 2022

julien-f Jan 10, 2023

Choose a reason for hiding this comment

ShogunPanda Jan 12, 2023

Choose a reason for hiding this comment

julien-f Jan 12, 2023

Choose a reason for hiding this comment

ShogunPanda Jan 12, 2023

Choose a reason for hiding this comment

ShogunPanda commented Dec 21, 2021 •

edited

Loading

mcollina commented Dec 29, 2021 •

edited

Loading

bnoordhuis commented Jul 7, 2022 •

edited

Loading