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: #27556 PR-URL: #27558 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
nodejs · May 9, 2019 · c30ef3c · c30ef3c
1 parent e582d11
commit c30ef3c
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 17 deletions.
diff --git 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])
 <!-- YAML
 added: v0.9.12
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/27558
+    description: The default timeout changed from 120s to 0 (no timeout).
 -->
 
-* `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
 <!-- YAML
 added: v0.9.12
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/27558
+    description: The default timeout changed from 120s to 0 (no 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
@@ -1722,11 +1722,15 @@ server.on('stream', (stream, headers, flags) => {
 #### Event: 'timeout'
 <!-- YAML
 added: v8.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/27558
+    description: The default timeout changed from 120s to 0 (no 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])
 <!-- YAML
@@ -1743,9 +1747,13 @@ consider also using [`http2session.close()`] on active sessions.
 #### server.setTimeout([msecs][, callback])
 <!-- YAML
 added: v8.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/27558
+    description: The default timeout changed from 120s to 0 (no timeout).
 -->
 
-* `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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
       }));