Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: delay when timeout is on the Agent #2297

Merged
merged 1 commit into from Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/intercepted_request_router.js
Expand Up @@ -51,8 +51,12 @@ class InterceptedRequestRouter {

// support setting `timeout` using request `options`
// https://nodejs.org/docs/latest-v12.x/api/http.html#http_http_request_url_options_callback
if (options.timeout) {
this.socket.setTimeout(options.timeout)
// any timeout in the request options override any timeout in the agent options.
// per https://github.com/nodejs/node/pull/21204
const timeout =
options.timeout || (options.agent && options.agent.options.timeout)
if (timeout) {
this.socket.setTimeout(timeout)
}

this.response = new IncomingMessage(this.socket)
Expand Down
4 changes: 2 additions & 2 deletions lib/playback_interceptor.js
Expand Up @@ -79,7 +79,7 @@ class ReadableBuffers extends stream.Readable {
this.buffers = buffers
}

_read(size) {
_read(_size) {
while (this.buffers.length) {
if (!this.push(this.buffers.shift())) {
return
Expand Down Expand Up @@ -315,7 +315,7 @@ function playbackInterceptor({

// Calling `start` immediately could take the request all the way to the connection delay
// during a single microtask execution. This setImmediate stalls the playback to ensure the
// correct events are emitted first ('socket', 'finish') and any aborts in the in the queue or
// correct events are emitted first ('socket', 'finish') and any aborts in the queue or
// called during a 'finish' listener can be called.
common.setImmediate(() => {
if (!common.isRequestDestroyed(req)) {
Expand Down
40 changes: 40 additions & 0 deletions tests/test_delay.js
Expand Up @@ -348,6 +348,46 @@ describe('`delayConnection()`', () => {
req.end()
})

it('emits a timeout - with Agent.timeout', done => {
nock('http://example.test').get('/').delayConnection(10000).reply(200, 'OK')

const onEnd = sinon.spy()
const agent = new http.Agent({ timeout: 5000 })

const req = http.request('http://example.test', { agent }, res => {
res.once('end', onEnd)
})

req.on('timeout', function () {
expect(onEnd).not.to.have.been.called()
done()
})

req.end()
})

it('emits a timeout - options.timeout takes precedence over Agent.timeout', done => {
nock('http://example.test').get('/').delayConnection(10000).reply(200, 'OK')

const onEnd = sinon.spy()
const agent = new http.Agent({ timeout: 30000 })

const req = http.request(
'http://example.test',
{ agent, timeout: 5000 },
res => {
res.once('end', onEnd)
}
)

req.on('timeout', function () {
expect(onEnd).not.to.have.been.called()
done()
})

req.end()
})

it('does not emit a timeout when timeout > delayConnection', done => {
const responseText = 'okeydoke!'
const scope = nock('http://example.test')
Expand Down