fix: delay when timeout is on the Agent (#2297)

mastermatt · web-flow · commit 616d90358b37 · 2022-02-03T11:54:05.000-07:00
resolves #2296
diff --git a/lib/intercepted_request_router.js b/lib/intercepted_request_router.js
@@ -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)
diff --git a/lib/playback_interceptor.js b/lib/playback_interceptor.js
@@ -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
@@ -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)) {
diff --git a/tests/test_delay.js b/tests/test_delay.js
@@ -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')
