Fix handling of network errors for beforeRetry hook (#276)

index.d.ts

@@ -48,7 +48,9 @@ export interface Hooks {
 	beforeRequest?: BeforeRequestHook[];
 
 	/**
-	This hook enables you to modify the request right before retry. Ky will make no further changes to the request after this. The hook function receives the normalized input and options, an error instance and the retry count as arguments. You could, for example, modify `options.headers` here.
+	This hook enables you to modify the request right before retry. Ky will make no further changes to the request after this. The hook function receives an object with the normalized request and options, an error instance, and the retry count. You could, for example, modify `request.headers` here.
+
+	If the request received a response, it will be available at `error.response`. Be aware that some types of errors, such as network errors, inherently mean that a response was not received.
 
 	@example
 	```
@@ -58,7 +60,7 @@ export interface Hooks {
 		await ky('https://example.com', {
 			hooks: {
 				beforeRetry: [
-					async (input, options, errors, retryCount) => {
+					async ({request, options, error, retryCount}) => {
 						const token = await ky('https://example.com/refresh-token');
 						options.headers.set('Authorization', `token ${token}`);
 					}
@@ -522,7 +524,7 @@ declare const ky: {
 		await ky('https://example.com', {
 			hooks: {
 				beforeRetry: [
-					async (request, options, errors, retryCount) => {
+					async ({request, options, error, retryCount}) => {
 						const shouldStopRetry = await ky('https://example.com/api');
 						if (shouldStopRetry) {
 							return ky.stop;

index.js

@@ -303,14 +303,16 @@ class Ky {
 				const modifiedResponse = await hook(
 					this.request,
 					this._options,
-					response.clone()
+					this._decorateResponse(response.clone())
 				);
 
 				if (modifiedResponse instanceof globals.Response) {
 					response = modifiedResponse;
 				}
 			}
 
+			this._decorateResponse(response);
+
 			if (!response.ok && this._options.throwHttpErrors) {
 				throw new HTTPError(response);
 			}
@@ -329,12 +331,6 @@ class Ky {
 				return this._stream(response.clone(), this._options.onDownloadProgress);
 			}
 
-			if (this._options.parseJson) {
-				response.json = async () => {
-					return this._options.parseJson(await response.text());
-				};
-			}
-
 			return response;
 		};
 
@@ -401,6 +397,16 @@ class Ky {
 		return 0;
 	}
 
+	_decorateResponse(response) {
+		if (this._options.parseJson) {
+			response.json = async () => {
+				return this._options.parseJson(await response.text());
+			};
+		}
+
+		return response;
+	}
+
 	async _retry(fn) {
 		try {
 			return await fn();
@@ -415,7 +421,6 @@ class Ky {
 						request: this.request,
 						options: this._options,
 						error,
-						response: error.response.clone(),
 						retryCount: this._retryCount
 					});
 

readme.md

@@ -253,7 +253,9 @@ const api = ky.extend({
 Type: `Function[]`\
 Default: `[]`
 
-This hook enables you to modify the request right before retry. Ky will make no further changes to the request after this. The hook function receives the normalized request and options, the failed response, an error instance and the retry count as arguments. You could, for example, modify `request.headers` here.
+This hook enables you to modify the request right before retry. Ky will make no further changes to the request after this. The hook function receives an object with the normalized request and options, an error instance, and the retry count. You could, for example, modify `request.headers` here.
+
+If the request received a response, it will be available at `error.response`. Be aware that some types of errors, such as network errors, inherently mean that a response was not received.
 
 ```js
 import ky from 'ky';
@@ -262,7 +264,7 @@ import ky from 'ky';
 	await ky('https://example.com', {
 		hooks: {
 			beforeRetry: [
-				async ({request, response, options, errors, retryCount}) => {
+				async ({request, options, error, retryCount}) => {
 					const token = await ky('https://example.com/refresh-token');
 					request.headers.set('Authorization', `token ${token}`);
 				}
@@ -472,7 +474,7 @@ import ky from 'ky';
 	await ky('https://example.com', {
 		hooks: {
 			beforeRetry: [
-				async ({request, response, options, errors, retryCount}) => {
+				async ({request, options, error, retryCount}) => {
 					const shouldStopRetry = await ky('https://example.com/api');
 					if (shouldStopRetry) {
 						return ky.stop;

test/hooks.js

@@ -292,6 +292,34 @@ test('`afterResponse` hook is called with request, normalized options, and respo
 	await server.close();
 });
 
+test('afterResponse hook with parseJson and response.json()', async t => {
+	t.plan(5);
+
+	const server = await createTestServer();
+	server.get('/', async (request, response) => {
+		response.end('text');
+	});
+
+	const json = await ky.get(server.url, {
+		parseJson(text) {
+			t.is(text, 'text');
+			return {awesome: true};
+		},
+		hooks: {
+			afterResponse: [
+				async (request, options, response) => {
+					t.true(response instanceof Response);
+					t.deepEqual(await response.json(), {awesome: true});
+				}
+			]
+		}
+	}).json();
+
+	t.deepEqual(json, {awesome: true});
+
+	await server.close();
+});
+
 test('beforeRetry hook is never called for the initial request', async t => {
 	const fixture = 'fixture';
 	const server = await createTestServer();
@@ -368,7 +396,7 @@ test('beforeRetry hook is called with error and retryCount', async t => {
 		hooks: {
 			beforeRetry: [
 				({error, retryCount}) => {
-					t.truthy(error);
+					t.true(error instanceof ky.HTTPError);
 					t.true(retryCount >= 1);
 				}
 			]
@@ -378,6 +406,86 @@ test('beforeRetry hook is called with error and retryCount', async t => {
 	await server.close();
 });
 
+test('beforeRetry hook is called even if the error has no response', async t => {
+	t.plan(6);
+
+	let requestCount = 0;
+
+	const server = await createTestServer();
+	server.get('/', async (request, response) => {
+		requestCount++;
+		response.end('unicorn');
+	});
+
+	const text = await ky.get(server.url, {
+		retry: 2,
+		async fetch(request) {
+			if (requestCount === 0) {
+				requestCount++;
+				throw new Error('simulated network failure');
+			}
+
+			return global.fetch(request);
+		},
+		hooks: {
+			beforeRetry: [
+				({error, retryCount}) => {
+					t.is(error.message, 'simulated network failure');
+					t.is(error.response, undefined);
+					t.is(retryCount, 1);
+					t.is(requestCount, 1);
+				}
+			]
+		}
+	}).text();
+
+	t.is(text, 'unicorn');
+	t.is(requestCount, 2);
+
+	await server.close();
+});
+
+test('beforeRetry hook with parseJson and error.response.json()', async t => {
+	t.plan(10);
+
+	let requestCount = 0;
+
+	const server = await createTestServer();
+	server.get('/', async (request, response) => {
+		requestCount++;
+		if (requestCount === 1) {
+			response.status(502).end('text');
+		} else {
+			response.end('text');
+		}
+	});
+
+	const json = await ky.get(server.url, {
+		retry: 2,
+		parseJson(text) {
+			t.is(text, 'text');
+			return {awesome: true};
+		},
+		hooks: {
+			beforeRetry: [
+				async ({error, retryCount}) => {
+					t.true(error instanceof ky.HTTPError);
+					t.is(error.message, 'Bad Gateway');
+					t.true(error.response instanceof Response);
+					t.deepEqual(await error.response.json(), {awesome: true});
+					t.is(retryCount, 1);
+					t.is(requestCount, 1);
+				}
+			]
+		}
+	}).json();
+
+	t.deepEqual(json, {awesome: true});
+	t.is(requestCount, 2);
+
+	await server.close();
+});
+
 test('beforeRetry hook can cancel retries by returning `stop`', async t => {
 	let requestCount = 0;