fix: disambiguate timeout behavior for response headers and body (#770)

There is a single timeout option which applies to both the receiving of
response headers and the receiving of the entire response body. Once
the response is received, the socket timeout must be cleared to allow
the timeout to apply to the receiving of the entire response body.

Without clearing the socket timeout, if the nearing the idle timeout
when the Promise for the body is created, then the request's timeout
handler will abort the request, emitting ERR_STREAM_PREMATURE_CLOSE in
the body Promise.

By clearing the socket timeout once the response headers are received,
the timeout for the entire body can be started when the body is awaited.
Since the request will no longer be aborted by the socket timeout,
destroy is called on the body to prevent it continuing to emit data
events.

src/body.js

@@ -200,7 +200,9 @@ function consumeBody() {
 		if (this.timeout) {
 			resTimeout = setTimeout(() => {
 				abort = true;
-				reject(new FetchError(`Response timeout while trying to fetch ${this.url} (over ${this.timeout}ms)`, 'body-timeout'));
+				const err = new FetchError(`Response timeout while trying to fetch ${this.url} (over ${this.timeout}ms)`, 'body-timeout');
+				reject(err);
+				body.destroy(err);
 			}, this.timeout);
 		}
 

src/index.js

@@ -117,6 +117,7 @@ export default function fetch(url, options_) {
 		});
 
 		request_.on('response', res => {
+			request_.setTimeout(0);
 			const headers = createHeadersLenient(res.headers);
 
 			// HTTP fetch step 5

test/main.js

@@ -35,6 +35,7 @@ import HeadersOrig, {createHeadersLenient} from '../src/headers.js';
 import RequestOrig from '../src/request.js';
 import ResponseOrig from '../src/response.js';
 import Body, {getTotalBytes, extractContentType} from '../src/body.js';
+import delay from './utils/delay.js';
 import TestServer from './utils/server.js';
 
 const {
@@ -798,16 +799,8 @@ describe('node-fetch', () => {
 	});
 
 	it('should collect handled errors on the body stream to reject if the body is used later', () => {
-		function delay(value) {
-			return new Promise(resolve => {
-				setTimeout(() => {
-					resolve(value);
-				}, 20);
-			});
-		}
-
 		const url = `${base}invalid-content-encoding`;
-		return fetch(url).then(delay).then(res => {
+		return fetch(url).then(delay(20)).then(res => {
 			expect(res.headers.get('content-type')).to.equal('text/plain');
 			return expect(res.text()).to.eventually.be.rejected
 				.and.be.an.instanceOf(FetchError)
@@ -865,6 +858,21 @@ describe('node-fetch', () => {
 		});
 	});
 
+	it('should not allow socket timeout before body is read', () => {
+		const url = `${base}slow`;
+		const options = {
+			timeout: 100
+		};
+		// Await the response, then delay, allowing enough time for the timeout
+		// to be created just before the socket timeout
+		return fetch(url, options).then(delay(75)).then(res => {
+			expect(res.ok).to.be.true;
+			return expect(res.text()).to.eventually.be.rejected
+				.and.be.an.instanceOf(FetchError)
+				.and.have.property('type', 'body-timeout');
+		});
+	});
+
 	it('should allow custom timeout on redirected requests', () => {
 		const url = `${base}redirect/slow-chain`;
 		const options = {

test/utils/delay.js

@@ -0,0 +1,7 @@
+export default function delay(ms) {
+	return value => new Promise(resolve => {
+		setTimeout(() => {
+			resolve(value);
+		}, ms);
+	});
+}