Skip to content

Commit

Permalink
fix: wait a macrotick to resume without pipelining (nodejs#1465)
Browse files Browse the repository at this point in the history
* 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: nodejs#1415
Signed-off-by: Matteo Collina <hello@matteocollina.com>

* fixup

Signed-off-by: Matteo Collina <hello@matteocollina.com>
  • Loading branch information
mcollina authored and KhafraDev committed Jun 23, 2022
1 parent 85740b6 commit ab5f159
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 0 deletions.
5 changes: 5 additions & 0 deletions lib/client.js
Expand Up @@ -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)
}
Expand Down
31 changes: 31 additions & 0 deletions test/inflight-and-close.js
@@ -0,0 +1,31 @@
'use strict'

const t = require('tap')
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
}).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)
})
})

0 comments on commit ab5f159

Please sign in to comment.