From 2f831dc825e9dd1761b53fb74895c3881c168c6f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 24 May 2022 16:50:47 +0200 Subject: [PATCH 1/2] Wait a macrotick to resume without pipelining This change makes the Client wait for a full macrotick before executing up the next request if pipelining is disabled. This is to account for socket errors events that might be waiting to be processed in the event queue. This is the expected behavior without pipelining. This will slow us down a bit without pipelinig. Fixes: https://github.com/nodejs/undici/issues/1415 Signed-off-by: Matteo Collina --- lib/client.js | 5 +++++ test/inflight-and-close.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 test/inflight-and-close.js diff --git a/lib/client.js b/lib/client.js index 1e23e7aad34..fea887e92b2 100644 --- a/lib/client.js +++ b/lib/client.js @@ -873,6 +873,11 @@ class Parser { // have been queued since then. util.destroy(socket, new InformationalError('reset')) return constants.ERROR.PAUSED + } else if (client[kPipelining] === 1) { + // We must wait a full event loop cycle to reuse this socket to make sure + // that non-spec compliant servers are not closing the connection even if they + // said they won't. + setImmediate(resume, client) } else { resume(client) } diff --git a/test/inflight-and-close.js b/test/inflight-and-close.js new file mode 100644 index 00000000000..3fb699bc0cd --- /dev/null +++ b/test/inflight-and-close.js @@ -0,0 +1,31 @@ +'use strict' + +const t = require('tap') +const { fetch, request } = require('..') +const http = require('http') + +const server = http.createServer((req, res) => { + res.writeHead(200); + res.end("Response body"); + res.socket.end(); // Close the connection immediately with every response +}).listen(0, '127.0.0.1', function () { + const url = `http://127.0.0.1:${this.address().port}` + request(url) + .then(({ statusCode, headers, body }) => { + t.pass('first response') + body.resume() + body.on('close', function () { + t.pass('first body closed') + }) + return request(url) + .then(({ statusCode, headers, body }) => { + t.pass('second response') + body.resume() + body.on('close', function () { + server.close() + }) + }) + }).catch((err) => { + t.error(err) + }) +}) From 661ac4106a4cf4e195aec6fdf5dd2aa4c621ce9e Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 24 May 2022 16:57:50 +0200 Subject: [PATCH 2/2] fixup Signed-off-by: Matteo Collina --- test/inflight-and-close.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/inflight-and-close.js b/test/inflight-and-close.js index 3fb699bc0cd..49fbb10f7fc 100644 --- a/test/inflight-and-close.js +++ b/test/inflight-and-close.js @@ -1,27 +1,27 @@ 'use strict' const t = require('tap') -const { fetch, request } = require('..') +const { request } = require('..') const http = require('http') const server = http.createServer((req, res) => { - res.writeHead(200); - res.end("Response body"); - res.socket.end(); // Close the connection immediately with every response + res.writeHead(200) + res.end('Response body') + res.socket.end() // Close the connection immediately with every response }).listen(0, '127.0.0.1', function () { const url = `http://127.0.0.1:${this.address().port}` request(url) .then(({ statusCode, headers, body }) => { t.pass('first response') body.resume() - body.on('close', function () { + body.on('close', function () { t.pass('first body closed') }) return request(url) .then(({ statusCode, headers, body }) => { t.pass('second response') body.resume() - body.on('close', function () { + body.on('close', function () { server.close() }) })