From 92b1005cc4098a71f9ac8f984b61c07180ff77e8 Mon Sep 17 00:00:00 2001
From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>
Date: Fri, 5 Apr 2019 16:03:03 +0200
Subject: [PATCH] Throw on canceled request with incomplete response (#767)

---
 source/as-promise.ts             |  5 +++
 source/as-stream.ts              |  5 ++-
 test/cancel.ts                   | 57 ++++++++++++++++++++++++++++++++
 test/helpers/slow-data-stream.ts | 18 ++++++++++
 test/timeout.ts                  | 19 +----------
 5 files changed, 85 insertions(+), 19 deletions(-)
 create mode 100644 test/helpers/slow-data-stream.ts

diff --git a/source/as-promise.ts b/source/as-promise.ts
index a619a0734..305aa6d4f 100644
--- a/source/as-promise.ts
+++ b/source/as-promise.ts
@@ -40,6 +40,11 @@ export default function asPromise(options: Options) {
 				return;
 			}
 
+			if (response.req && response.req.aborted) {
+				// Canceled while downloading - will throw a CancelError or TimeoutError
+				return;
+			}
+
 			const limitStatusCode = options.followRedirect ? 299 : 399;
 
 			response.body = data;
diff --git a/source/as-stream.ts b/source/as-stream.ts
index e6daf1f31..7b3a8d7cb 100644
--- a/source/as-stream.ts
+++ b/source/as-stream.ts
@@ -23,7 +23,10 @@ export default function asStream(options: MergedOptions) {
 	const emitter = requestAsEventEmitter(options, input);
 
 	// Cancels the request
-	proxy._destroy = emitter.abort;
+	proxy._destroy = (error, callback) => {
+		callback(error);
+		emitter.abort();
+	};
 
 	emitter.on('response', (response: Response) => {
 		const {statusCode} = response;
diff --git a/test/cancel.ts b/test/cancel.ts
index 2e9b1e329..3e0f923b3 100644
--- a/test/cancel.ts
+++ b/test/cancel.ts
@@ -2,9 +2,11 @@ import {EventEmitter} from 'events';
 import {Readable as ReadableStream} from 'stream';
 import test from 'ava';
 import pEvent from 'p-event';
+import getStream from 'get-stream';
 // @ts-ignore
 import got, {CancelError} from '../source';
 import withServer from './helpers/with-server';
+import slowDataStream from './helpers/slow-data-stream';
 
 const prepareServer = server => {
 	const emitter = new EventEmitter();
@@ -34,6 +36,14 @@ const prepareServer = server => {
 	return {emitter, promise};
 };
 
+const downloadHandler = (_request, response) => {
+	response.writeHead(200, {
+		'transfer-encoding': 'chunked'
+	});
+	response.flushHeaders();
+	slowDataStream().pipe(response);
+};
+
 test('does not retry after cancelation', withServer, async (t, server, got) => {
 	const {emitter, promise} = prepareServer(server);
 
@@ -145,3 +155,50 @@ test('recover from cancellation using error instance', async t => {
 
 	await t.notThrowsAsync(recover);
 });
+
+test('throws on incomplete (canceled) response - promise', withServer, async (t, server, got) => {
+	server.get('/', downloadHandler);
+
+	await t.throwsAsync(got({
+		timeout: {request: 500}
+	}), got.TimeoutError);
+});
+
+test('throws on incomplete (canceled) response - promise #2', withServer, async (t, server, got) => {
+	server.get('/', downloadHandler);
+
+	const promise = got('').on('response', () => {
+		setTimeout(() => promise.cancel(), 500);
+	});
+
+	await t.throwsAsync(promise, got.CancelError);
+});
+
+test('throws on incomplete (canceled) response - stream', withServer, async (t, server, got) => {
+	server.get('/', downloadHandler);
+
+	const errorString = 'Foobar';
+
+	const stream = got.stream('').on('response', () => {
+		setTimeout(() => stream.destroy(new Error(errorString)), 500);
+	});
+
+	await t.throwsAsync(getStream(stream), errorString);
+});
+
+// Note: it will throw, but the response is loaded already.
+test('throws when canceling cached request', withServer, async (t, server, got) => {
+	server.get('/', (_request, response) => {
+		response.setHeader('Cache-Control', 'public, max-age=60');
+		response.end(Date.now().toString());
+	});
+
+	const cache = new Map();
+	await got({cache});
+
+	const promise = got({cache}).on('response', () => {
+		promise.cancel();
+	});
+
+	await t.throwsAsync(promise, got.CancelError);
+});
diff --git a/test/helpers/slow-data-stream.ts b/test/helpers/slow-data-stream.ts
new file mode 100644
index 000000000..eb39235f3
--- /dev/null
+++ b/test/helpers/slow-data-stream.ts
@@ -0,0 +1,18 @@
+import {PassThrough} from 'stream';
+
+export default (): PassThrough => {
+	const slowStream = new PassThrough();
+	let count = 0;
+
+	const interval = setInterval(() => {
+		if (count++ < 10) {
+			slowStream.push('data\n'.repeat(100));
+			return;
+		}
+
+		clearInterval(interval);
+		slowStream.push(null);
+	}, 100);
+
+	return slowStream;
+};
diff --git a/test/timeout.ts b/test/timeout.ts
index 97feb5ce9..7c8560f79 100644
--- a/test/timeout.ts
+++ b/test/timeout.ts
@@ -1,29 +1,12 @@
 import http from 'http';
 import net from 'net';
-import stream from 'stream';
 import getStream from 'get-stream';
 import test from 'ava';
 import pEvent from 'p-event';
 import delay from 'delay';
 import got from '../source';
 import withServer from './helpers/with-server';
-
-const slowDataStream = () => {
-	const slowStream = new stream.PassThrough();
-	let count = 0;
-
-	const interval = setInterval(() => {
-		if (count++ < 10) {
-			slowStream.push('data\n'.repeat(100));
-			return;
-		}
-
-		clearInterval(interval);
-		slowStream.push(null);
-	}, 100);
-
-	return slowStream;
-};
+import slowDataStream from './helpers/slow-data-stream';
 
 const requestDelay = 800;