Skip to content

Commit 616d903

Browse files
authored
fix: delay when timeout is on the Agent (#2297)
resolves #2296
1 parent 51c9cf5 commit 616d903

File tree

3 files changed

+48
-4
lines changed

3 files changed

+48
-4
lines changed

lib/intercepted_request_router.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,12 @@ class InterceptedRequestRouter {
5151

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

5862
this.response = new IncomingMessage(this.socket)

lib/playback_interceptor.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class ReadableBuffers extends stream.Readable {
7979
this.buffers = buffers
8080
}
8181

82-
_read(size) {
82+
_read(_size) {
8383
while (this.buffers.length) {
8484
if (!this.push(this.buffers.shift())) {
8585
return
@@ -315,7 +315,7 @@ function playbackInterceptor({
315315

316316
// Calling `start` immediately could take the request all the way to the connection delay
317317
// during a single microtask execution. This setImmediate stalls the playback to ensure the
318-
// correct events are emitted first ('socket', 'finish') and any aborts in the in the queue or
318+
// correct events are emitted first ('socket', 'finish') and any aborts in the queue or
319319
// called during a 'finish' listener can be called.
320320
common.setImmediate(() => {
321321
if (!common.isRequestDestroyed(req)) {

tests/test_delay.js

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,46 @@ describe('`delayConnection()`', () => {
348348
req.end()
349349
})
350350

351+
it('emits a timeout - with Agent.timeout', done => {
352+
nock('http://example.test').get('/').delayConnection(10000).reply(200, 'OK')
353+
354+
const onEnd = sinon.spy()
355+
const agent = new http.Agent({ timeout: 5000 })
356+
357+
const req = http.request('http://example.test', { agent }, res => {
358+
res.once('end', onEnd)
359+
})
360+
361+
req.on('timeout', function () {
362+
expect(onEnd).not.to.have.been.called()
363+
done()
364+
})
365+
366+
req.end()
367+
})
368+
369+
it('emits a timeout - options.timeout takes precedence over Agent.timeout', done => {
370+
nock('http://example.test').get('/').delayConnection(10000).reply(200, 'OK')
371+
372+
const onEnd = sinon.spy()
373+
const agent = new http.Agent({ timeout: 30000 })
374+
375+
const req = http.request(
376+
'http://example.test',
377+
{ agent, timeout: 5000 },
378+
res => {
379+
res.once('end', onEnd)
380+
}
381+
)
382+
383+
req.on('timeout', function () {
384+
expect(onEnd).not.to.have.been.called()
385+
done()
386+
})
387+
388+
req.end()
389+
})
390+
351391
it('does not emit a timeout when timeout > delayConnection', done => {
352392
const responseText = 'okeydoke!'
353393
const scope = nock('http://example.test')

0 commit comments

Comments
 (0)