From 5d695220bb90528eb99fed005a91d0b87e285d15 Mon Sep 17 00:00:00 2001
From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>
Date: Wed, 15 Apr 2020 12:27:32 +0200
Subject: [PATCH] Fix the retry logic

---
 source/as-promise/core.ts  | 19 +++++++++----------
 source/as-promise/index.ts |  4 ++++
 source/core/index.ts       |  9 +++++----
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/source/as-promise/core.ts b/source/as-promise/core.ts
index f4b60d24a..bd452c553 100644
--- a/source/as-promise/core.ts
+++ b/source/as-promise/core.ts
@@ -6,7 +6,7 @@ import {
 	Defaults,
 	ResponseType
 } from './types';
-import Request, {knownHookEvents, RequestError, HTTPError, Method} from '../core';
+import Request, {knownHookEvents, RequestError, Method} from '../core';
 
 if (!knownHookEvents.includes('beforeRetry' as any)) {
 	knownHookEvents.push('beforeRetry' as any, 'afterResponse' as any);
@@ -126,23 +126,22 @@ export default class PromisableRequest extends Request {
 		return mergedOptions!;
 	}
 
-	async _beforeError(error: RequestError): Promise<void> {
-		const isHTTPError = error instanceof HTTPError;
+	async _beforeError(error: Error): Promise<void> {
+		if (!(error instanceof RequestError)) {
+			error = new RequestError(error.message, error, this.options, this);
+		}
 
 		try {
 			for (const hook of this.options.hooks.beforeError) {
 				// eslint-disable-next-line no-await-in-loop
-				error = await hook(error);
+				error = await hook(error as RequestError);
 			}
 		} catch (error_) {
-			this.destroy(error_);
+			this.destroy(new RequestError(error_.message, error_, this.options, this));
 			return;
 		}
 
-		if (this._throwHttpErrors && !isHTTPError) {
-			this.destroy(error);
-		} else {
-			this.emit('error', error);
-		}
+		// Let the promise handle the error
+		this.emit('error', error);
 	}
 }
diff --git a/source/as-promise/index.ts b/source/as-promise/index.ts
index 80133f345..1d565bd84 100644
--- a/source/as-promise/index.ts
+++ b/source/as-promise/index.ts
@@ -34,6 +34,10 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
 			}
 
 			// Support retries
+			// `options.throwHttpErrors` needs to be always true,
+			// so the HTTP errors are caught and the request is retried.
+			// The error is **eventaully** thrown
+			// if the user value `options._throwHttpErrors` is true.
 			const {throwHttpErrors} = options;
 			if (!throwHttpErrors) {
 				options.throwHttpErrors = true;
diff --git a/source/core/index.ts b/source/core/index.ts
index 43e2182b7..7ef3d4e25 100644
--- a/source/core/index.ts
+++ b/source/core/index.ts
@@ -38,6 +38,7 @@ const kUnproxyEvents = Symbol('unproxyEvents');
 const kIsFromCache = Symbol('isFromCache');
 const kCancelTimeouts = Symbol('cancelTimeouts');
 const kStartedReading = Symbol('startedReading');
+const kStopReading = Symbol('stopReading');
 export const kIsNormalizedAlready = Symbol('isNormalizedAlready');
 
 const supportsBrotli = is.string((process.versions as any).brotli);
@@ -466,6 +467,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 	declare _cannotHaveBody: boolean;
 	[kDownloadedSize]: number;
 	[kUploadedSize]: number;
+	[kStopReading]: boolean;
 	[kBodySize]?: number;
 	[kServerResponsesPiped]: Set<ServerResponse>;
 	[kIsFromCache]?: boolean;
@@ -481,7 +483,6 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 	declare requestUrl: string;
 	finalized: boolean;
 	redirects: string[];
-	errored: boolean;
 
 	constructor(url: string | URL, options: Options = {}, defaults?: Defaults) {
 		super({
@@ -494,7 +495,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 		this.finalized = false;
 		this[kServerResponsesPiped] = new Set<ServerResponse>();
 		this.redirects = [];
-		this.errored = false;
+		this[kStopReading] = false;
 
 		// TODO: Remove this when targeting Node.js >= 12
 		this._progressCallbacks = [];
@@ -1299,7 +1300,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 	}
 
 	async _beforeError(error: Error): Promise<void> {
-		this.errored = true;
+		this[kStopReading] = true;
 
 		if (!(error instanceof RequestError)) {
 			error = new RequestError(error.message, error, this.options, this);
@@ -1328,7 +1329,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 	}
 
 	_read(): void {
-		if (kResponse in this && !this.errored) {
+		if (kResponse in this && !this[kStopReading]) {
 			let data;
 
 			while ((data = this[kResponse]!.read()) !== null) {