From 7b19e8f32c90984a168ac2e29585bcd637535e9a Mon Sep 17 00:00:00 2001
From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>
Date: Sat, 2 May 2020 16:10:42 +0200
Subject: [PATCH] Fix hanging promise when using cache

Fixes #1193
---
 source/core/index.ts | 39 +++++++++++++++++++++++----------------
 test/cache.ts        | 25 +++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/source/core/index.ts b/source/core/index.ts
index 65d14a7a0..5e2b6950b 100644
--- a/source/core/index.ts
+++ b/source/core/index.ts
@@ -1160,7 +1160,9 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 
 		this[kCancelTimeouts] = timedOut(request, timeout, url);
 
-		request.once('response', response => {
+		const responseEventName = options.cache ? 'cacheableResponse' : 'response';
+
+		request.once(responseEventName, (response: IncomingMessage) => {
 			this._onResponse(response);
 		});
 
@@ -1220,19 +1222,21 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 			delete (options as unknown as NormalizedOptions).url;
 
 			// This is ugly
-			const cacheRequest = cacheableStore.get((options as any).cache)!(options, resolve as any);
-
-			// Restore options
-			(options as unknown as NormalizedOptions).url = url;
+			const cacheRequest = cacheableStore.get((options as any).cache)!(options, response => {
+				const typedResponse = response as unknown as IncomingMessage & {req: ClientRequest};
+				const {req} = typedResponse;
 
-			cacheRequest.once('error', (error: Error) => {
-				if (error instanceof CacheableRequest.CacheError) {
-					reject(new CacheError(error, this));
-					return;
+				if (req) {
+					req.emit('cacheableResponse', typedResponse);
 				}
 
-				reject(error);
+				resolve(typedResponse as unknown as ResponseLike);
 			});
+
+			// Restore options
+			(options as unknown as NormalizedOptions).url = url;
+
+			cacheRequest.once('error', reject);
 			cacheRequest.once('request', resolve);
 		});
 	}
@@ -1295,6 +1299,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 
 		const isHttps = url.protocol === 'https:';
 
+		// Fallback function
 		let fallbackFn: HttpRequestFunction;
 		if (options.http2) {
 			fallbackFn = http2wrapper.auto;
@@ -1303,20 +1308,22 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 		}
 
 		const realFn = options.request ?? fallbackFn;
-		const fn = options.cache ? this._createCacheableRequest.bind(this) : realFn;
 
+		// Cache support
+		const fn = options.cache ? this._createCacheableRequest : realFn;
+
+		// Pass an agent directly when HTTP2 is disabled
 		if (agent && !options.http2) {
 			(options as unknown as RequestOptions).agent = agent[isHttps ? 'https' : 'http'];
 		}
 
+		// Prepare plain HTTP request options
 		options[kRequest] = realFn as HttpRequestFunction;
 		delete options.request;
 		delete options.timeout;
 
-		let requestOrResponse: ReturnType<RequestFunction>;
-
 		try {
-			requestOrResponse = await fn(url, options as unknown as RequestOptions);
+			let requestOrResponse = await fn(url, options as unknown as RequestOptions);
 
 			if (is.undefined(requestOrResponse)) {
 				requestOrResponse = fallbackFn(url, options as unknown as RequestOptions);
@@ -1343,8 +1350,8 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 				this._onResponse(requestOrResponse as IncomingMessage);
 			}
 		} catch (error) {
-			if (error instanceof RequestError) {
-				throw error;
+			if (error instanceof CacheableRequest.CacheError) {
+				throw new CacheError(error, this);
 			}
 
 			throw new RequestError(error.message, error, this);
diff --git a/test/cache.ts b/test/cache.ts
index 379f88837..342b4d877 100644
--- a/test/cache.ts
+++ b/test/cache.ts
@@ -7,6 +7,7 @@ import {Handler} from 'express';
 import {Response} from '../source';
 import withServer from './helpers/with-server';
 import CacheableLookup from 'cacheable-lookup';
+import delay = require('delay');
 
 const cacheEndpoint: Handler = (_request, response) => {
 	response.setHeader('Cache-Control', 'public, max-age=60');
@@ -285,3 +286,27 @@ test('can replace the instance\'s HTTP cache', withServer, async (t, server, got
 	t.is(cache.size, 1);
 	t.is(secondCache.size, 1);
 });
+
+test('does not hang on huge response', withServer, async (t, server, got) => {
+	const bufferSize = 3 * 16 * 1024;
+	const times = 10;
+
+	const buffer = Buffer.alloc(bufferSize);
+
+	server.get('/', async (_request, response) => {
+		for (let i = 0; i < 10; i++) {
+			response.write(buffer);
+
+			// eslint-disable-next-line no-await-in-loop
+			await delay(100);
+		}
+
+		response.end();
+	});
+
+	const body = await got('', {
+		cache: new Map()
+	}).buffer();
+
+	t.is(body.length, bufferSize * times);
+});