Skip to content

Commit

Permalink
http, http2: remove default server timeout
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ofrobots committed May 6, 2019
1 parent 8876ac5 commit 52594f4
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 17 deletions.
18 changes: 13 additions & 5 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand All @@ -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.
Expand Down
12 changes: 10 additions & 2 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}

Expand Down
2 changes: 1 addition & 1 deletion lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions test/async-hooks/test-graph.http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' } ]
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-child-process-http-socket-leak.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}));
Expand Down

0 comments on commit 52594f4

Please sign in to comment.