From 52594f434c6edd8a3d482c138fad670de37710b3 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Fri, 3 May 2019 16:45:57 -0700 Subject: [PATCH] http, http2: remove default server timeout Timing out and closing the socket after two minutes have elapsed is surprising and problematic for users. This behavior was specific to Node.js, and doesn't seem to be common in other language runtimes. Fixes: https://github.com/nodejs/node/issues/27556 --- doc/api/http.md | 18 +++++++++++++----- doc/api/http2.md | 12 ++++++++++-- lib/_http_server.js | 2 +- lib/https.js | 2 +- lib/internal/http2/core.js | 6 ++---- test/async-hooks/test-graph.http.js | 3 --- .../test-child-process-http-socket-leak.js | 2 +- 7 files changed, 28 insertions(+), 17 deletions(-) diff --git a/doc/api/http.md b/doc/api/http.md index ccd5e4b0bd4b30..0588d7f408f194 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1013,9 +1013,13 @@ Limits maximum incoming headers count. If set to 0, no limit will be applied. ### server.setTimeout([msecs][, callback]) -* `msecs` {number} **Default:** `120000` (2 minutes) +* `msecs` {number} **Default:** 0 (no timeout) * `callback` {Function} * Returns: {http.Server} @@ -1026,16 +1030,20 @@ occurs. If there is a `'timeout'` event listener on the Server object, then it will be called with the timed-out socket as an argument. -By default, the Server's timeout value is 2 minutes, and sockets are -destroyed automatically if they time out. However, if a callback is assigned -to the Server's `'timeout'` event, timeouts must be handled explicitly. +By default, the Server does not timeout sockets. However, if a callback +is assigned to the Server's `'timeout'` event, timeouts must be handled +explicitly. ### server.timeout -* {number} Timeout in milliseconds. **Default:** `120000` (2 minutes). +* {number} Timeout in milliseconds. **Default:** 0 (no timeout) The number of milliseconds of inactivity before a socket is presumed to have timed out. diff --git a/doc/api/http2.md b/doc/api/http2.md index 1416c976362afe..5ba4943a4d9636 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1722,11 +1722,15 @@ server.on('stream', (stream, headers, flags) => { #### Event: 'timeout' The `'timeout'` event is emitted when there is no activity on the Server for a given number of milliseconds set using `http2server.setTimeout()`. -**Default:** 2 minutes. +**Default:** 0 (no timeout) #### server.close([callback]) -* `msecs` {number} **Default:** `120000` (2 minutes) +* `msecs` {number} **Default:** 0 (no timeout) * `callback` {Function} * Returns: {Http2Server} diff --git a/lib/_http_server.js b/lib/_http_server.js index 9f62872e8ec5e8..f7884d2626f499 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -315,7 +315,7 @@ function Server(options, requestListener) { this.on('connection', connectionListener); - this.timeout = 2 * 60 * 1000; + this.timeout = 0; this.keepAliveTimeout = 5000; this.maxHeadersCount = null; this.headersTimeout = 40 * 1000; // 40 seconds diff --git a/lib/https.js b/lib/https.js index 4e649017312a24..e1fc91fd966ad9 100644 --- a/lib/https.js +++ b/lib/https.js @@ -71,7 +71,7 @@ function Server(opts, requestListener) { conn.destroy(err); }); - this.timeout = 2 * 60 * 1000; + this.timeout = 0; this.keepAliveTimeout = 5000; this.maxHeadersCount = null; this.headersTimeout = 40 * 1000; // 40 seconds diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 58193b41cd3217..6ad1668c4f59b7 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -171,8 +171,6 @@ const kState = Symbol('state'); const kType = Symbol('type'); const kWriteGeneric = Symbol('write-generic'); -const kDefaultSocketTimeout = 2 * 60 * 1000; - const { paddingBuffer, PADDING_BUF_FRAME_LENGTH, @@ -2680,7 +2678,7 @@ class Http2SecureServer extends TLSServer { options = initializeTLSOptions(options); super(options, connectionListener); this[kOptions] = options; - this.timeout = kDefaultSocketTimeout; + this.timeout = 0; this.on('newListener', setupCompat); if (typeof requestListener === 'function') this.on('request', requestListener); @@ -2702,7 +2700,7 @@ class Http2Server extends NETServer { constructor(options, requestListener) { super(connectionListener); this[kOptions] = initializeOptions(options); - this.timeout = kDefaultSocketTimeout; + this.timeout = 0; this.on('newListener', setupCompat); if (typeof requestListener === 'function') this.on('request', requestListener); diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js index 55b9b055a0f050..12862467a6947e 100644 --- a/test/async-hooks/test-graph.http.js +++ b/test/async-hooks/test-graph.http.js @@ -43,9 +43,6 @@ process.on('exit', function() { { type: 'HTTPINCOMINGMESSAGE', id: 'httpincomingmessage:1', triggerAsyncId: 'tcp:2' }, - { type: 'Timeout', - id: 'timeout:2', - triggerAsyncId: 'tcp:2' }, { type: 'SHUTDOWNWRAP', id: 'shutdown:1', triggerAsyncId: 'tcp:2' } ] diff --git a/test/parallel/test-child-process-http-socket-leak.js b/test/parallel/test-child-process-http-socket-leak.js index 553a3277532b04..9b284f285c4572 100644 --- a/test/parallel/test-child-process-http-socket-leak.js +++ b/test/parallel/test-child-process-http-socket-leak.js @@ -46,7 +46,7 @@ server.listen(0, common.mustCall(() => { }, common.mustCall((res) => { res.on('data', () => {}); res.on('end', common.mustCall(() => { - assert.strictEqual(socket[kTimeout]._idleTimeout, -1); + assert.strictEqual(socket[kTimeout], null); assert.strictEqual(socket.parser, null); assert.strictEqual(socket._httpMessage, null); }));