Fix fetch parameters not being applied correctly (nodejs#1870)

* Fix default fetch parameters

* Preserve existing behavior with 300 second timeout if not defined

* Add test for 300 second timeout as default

* Cleanup old unused tests

* Simplify how fetch utilizes timeouts from agent

lib/fetch/index.js

@@ -1951,8 +1951,6 @@ async function httpNetworkFetch (
         body: fetchParams.controller.dispatcher.isMockActive ? request.body && request.body.source : body,
         headers: request.headersList[kHeadersCaseInsensitive],
         maxRedirections: 0,
-        bodyTimeout: 300_000,
-        headersTimeout: 300_000,
         upgrade: request.mode === 'websocket' ? 'websocket' : undefined
       },
       {

test/fetch/fetch-timeouts.js

@@ -0,0 +1,49 @@
+'use strict'
+
+const { test } = require('tap')
+
+const { fetch, Agent } = require('../..')
+const { createServer } = require('http')
+const FakeTimers = require('@sinonjs/fake-timers')
+
+test('Fetch very long request, timeout overridden so no error', (t) => {
+  const minutes = 6
+  const msToDelay = 1000 * 60 * minutes
+
+  t.setTimeout(undefined)
+  t.plan(1)
+
+  const clock = FakeTimers.install()
+  t.teardown(clock.uninstall.bind(clock))
+
+  const server = createServer((req, res) => {
+    setTimeout(() => {
+      res.end('hello')
+    }, msToDelay)
+    clock.tick(msToDelay + 1)
+  })
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, () => {
+    fetch(`http://localhost:${server.address().port}`, {
+      path: '/',
+      method: 'GET',
+      dispatcher: new Agent({
+        headersTimeout: 0,
+        connectTimeout: 0,
+        bodyTimeout: 0
+      })
+    })
+      .then((response) => response.text())
+      .then((response) => {
+        t.equal('hello', response)
+        t.end()
+      })
+      .catch((err) => {
+        // This should not happen, a timeout error should not occur
+        t.error(err)
+      })
+
+    clock.tick(msToDelay - 1)
+  })
+})

test/node-fetch/main.js

@@ -1660,20 +1660,4 @@ describe('node-fetch', () => {
       expect(res.ok).to.be.false
     })
   })
-
-  // it('should not time out waiting for a response 60 seconds', function () {
-  //   this.timeout(65_000)
-  //   return fetch(`${base}timeout60s`).then(res => {
-  //     expect(res.status).to.equal(200)
-  //     expect(res.ok).to.be.true
-  //     return res.text().then(result => {
-  //       expect(result).to.equal('text')
-  //     })
-  //   })
-  // })
-
-  // it('should time out waiting for more than 300 seconds', function () {
-  //   this.timeout(305_000)
-  //   return expect(fetch(`${base}timeout300s`)).to.eventually.be.rejectedWith(TypeError)
-  // })
 })

test/node-fetch/utils/server.js

@@ -227,22 +227,6 @@ module.exports = class TestServer {
       }, 1000)
     }
 
-    if (p === '/timeout60s') {
-      setTimeout(() => {
-        res.statusCode = 200
-        res.setHeader('Content-Type', 'text/plain')
-        res.end('text')
-      }, 60_000)
-    }
-
-    if (p === '/timeout300s') {
-      setTimeout(() => {
-        res.statusCode = 200
-        res.setHeader('Content-Type', 'text/plain')
-        res.end('text')
-      }, 300_000)
-    }
-
     if (p === '/slow') {
       res.statusCode = 200
       res.setHeader('Content-Type', 'text/plain')