From cfc47c2c3d30284e88be45094610288275034313 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Thu, 30 Jul 2020 17:01:39 +0200 Subject: [PATCH 1/4] fix: set default idle timeout to 4 seconds Node HTTP server seem to have a default of 5 seconds and we should be below that. Fixes: https://github.com/mcollina/undici/issues/276 --- README.md | 2 +- lib/client.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 214401322a7..f2626c28a4e 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ Options: - `idleTimeout`, the timeout after which a socket with no active requests will be released and no longer re-used for subsequent requests. This value is an upper bound and might be reduced by keep-alive hints from the server. - Default: `30e3` milliseconds (30s). + Default: `4e3` milliseconds (4s). - `requestTimeout`, the timeout after which a request will time out, in milliseconds. Monitors time between request being enqueued and receiving diff --git a/lib/client.js b/lib/client.js index 9566404c4f8..d996255034e 100644 --- a/lib/client.js +++ b/lib/client.js @@ -133,7 +133,7 @@ class Client extends EventEmitter { this[kUrl] = url this[kSocketPath] = socketPath this[kSocketTimeout] = socketTimeout == null ? 30e3 : socketTimeout - this[kIdleTimeout] = idleTimeout == null ? 30e3 : idleTimeout + this[kIdleTimeout] = idleTimeout == null ? 4e3 : idleTimeout this[kKeepAliveTimeout] = this[kIdleTimeout] this[kRequestTimeout] = requestTimeout == null ? 30e3 : requestTimeout this[kClosed] = false From 04b8e279537282969729388be8ac0c76e1c82954 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Thu, 30 Jul 2020 17:07:58 +0200 Subject: [PATCH 2/4] fixup --- lib/client.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index d996255034e..9c645ee0333 100644 --- a/lib/client.js +++ b/lib/client.js @@ -443,8 +443,10 @@ class Parser extends HTTPParser { const m = headers['keep-alive'].match(/timeout=(\d+)/) if (m) { const keepAliveTimeout = Number(m[1]) * 1000 - // Set timeout to half of hint to account for timing inaccuracies. - client[kKeepAliveTimeout] = Math.min(keepAliveTimeout - 500, client[kIdleTimeout]) + // Set timeout to 1 second less than hint to account for timing inaccuracies. + client[kKeepAliveTimeout] = Math.min(keepAliveTimeout - 1000, client[kIdleTimeout]) + + // TODO: What if client[kKeepAliveTimeout] === 0? } } else { client[kKeepAliveTimeout] = client[kIdleTimeout] From 8317a384589225035e52b41a806b9e2952e5f721 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Thu, 30 Jul 2020 17:25:58 +0200 Subject: [PATCH 3/4] fixup --- lib/client.js | 5 +++-- test/client-keep-alive.js | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/client.js b/lib/client.js index 9c645ee0333..b78d36997ca 100644 --- a/lib/client.js +++ b/lib/client.js @@ -442,9 +442,10 @@ class Parser extends HTTPParser { if (headers['keep-alive']) { const m = headers['keep-alive'].match(/timeout=(\d+)/) if (m) { - const keepAliveTimeout = Number(m[1]) * 1000 + let keepAliveTimeout = Number(m[1]) * 1000 + keepAliveTimeout = keepAliveTimeout > 2000 ? keepAliveTimeout - 1000 : keepAliveTimeout / 2 // Set timeout to 1 second less than hint to account for timing inaccuracies. - client[kKeepAliveTimeout] = Math.min(keepAliveTimeout - 1000, client[kIdleTimeout]) + client[kKeepAliveTimeout] = Math.min(keepAliveTimeout, client[kIdleTimeout]) // TODO: What if client[kKeepAliveTimeout] === 0? } diff --git a/test/client-keep-alive.js b/test/client-keep-alive.js index 3edc5af78c1..d39795befee 100644 --- a/test/client-keep-alive.js +++ b/test/client-keep-alive.js @@ -10,7 +10,7 @@ test('keep-alive header', (t) => { const server = createServer((socket) => { socket.write('HTTP/1.1 200 OK\r\n') socket.write('Content-Length: 0\r\n') - socket.write('Keep-Alive: timeout=1s\r\n') + socket.write('Keep-Alive: timeout=2s\r\n') socket.write('Connection: keep-alive\r\n') socket.write('\r\n\r\n') }) @@ -29,7 +29,7 @@ test('keep-alive header', (t) => { body.on('end', () => { const timeout = setTimeout(() => { t.fail() - }, 1e3) + }, 3e3) client.on('disconnect', () => { t.pass() clearTimeout(timeout) From de2e3938229ea99c77d9f04ad1e346e9c20fac5b Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Thu, 30 Jul 2020 17:30:39 +0200 Subject: [PATCH 4/4] fixup --- lib/client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index b78d36997ca..dfc5f616747 100644 --- a/lib/client.js +++ b/lib/client.js @@ -442,9 +442,9 @@ class Parser extends HTTPParser { if (headers['keep-alive']) { const m = headers['keep-alive'].match(/timeout=(\d+)/) if (m) { - let keepAliveTimeout = Number(m[1]) * 1000 - keepAliveTimeout = keepAliveTimeout > 2000 ? keepAliveTimeout - 1000 : keepAliveTimeout / 2 + const timeout = Number(m[1]) * 1000 // Set timeout to 1 second less than hint to account for timing inaccuracies. + const keepAliveTimeout = timeout > 2000 ? timeout - 1000 : timeout / 2 client[kKeepAliveTimeout] = Math.min(keepAliveTimeout, client[kIdleTimeout]) // TODO: What if client[kKeepAliveTimeout] === 0?