fix: do not setup the response timeout if the stream has already ended (

#53)

this change ensures we will not trigger a timeout caused by a slow
stream pipeline if the stream has already ended. this subtly shifts the
meaning of the timeout from "the response has been fully consumed" to
"the socket behind the response has finished" which i feel maintains the
spirit of the timeout while also not needlessly throwing errors when
we're not using the socket any more.

lib/body.js

@@ -127,8 +127,10 @@ class Body {
       : this.size ? new MinipassSized({ size: this.size })
       : new Minipass()
 
-    // allow timeout on slow response body
-    const resTimeout = this.timeout ? setTimeout(() => {
+    // allow timeout on slow response body, but only if the stream is still writable. this
+    // makes the timeout center on the socket stream from lib/index.js rather than the
+    // intermediary minipass stream we create to receive the data
+    const resTimeout = this.timeout && stream.writable ? setTimeout(() => {
       stream.emit('error', new FetchError(
         `Response timeout while trying to fetch ${
           this.url} (over ${this.timeout}ms)`, 'body-timeout'))

test/body.js

@@ -176,6 +176,33 @@ t.test('stream body too slow', async t => {
   })
 })
 
+t.test('no timeout if stream ends before we even start consuming', async t => {
+  // this test mimics how lib/index.js writes data into the intermediary minipass stream
+
+  // the SlowMinipass class delays the result from concat() mimicking a slow pipe downstream
+  class SlowMinipass extends Minipass {
+    async concat () {
+      // 10 millisecond delay before resolving
+      await new Promise((resolve) => setTimeout(resolve, 10))
+      return super.concat()
+    }
+  }
+  const networkStream = new Minipass()
+  const wrappedStream = new SlowMinipass()
+
+  networkStream.on('data', (chunk) => wrappedStream.write(chunk))
+  networkStream.on('end', () => wrappedStream.end())
+
+  for (let i = 0; i < 10; ++i) {
+    networkStream.write('some data')
+  }
+  networkStream.end()
+
+  // timeout of 1ms, must be lower than the 10ms used in SlowMinipass to trigger the bug
+  const b = new Body(wrappedStream, { timeout: 1 })
+  await t.resolves(b.text(), 'some data')
+})
+
 t.test('random toString-ing thing body', async t => {
   const b = new Body({ toString () {
     return 'a=1'