From 9876da129c216b99ad9f5760d38ba0ed1876d20e Mon Sep 17 00:00:00 2001 From: Seth Holladay Date: Mon, 28 Sep 2020 15:35:59 -0400 Subject: [PATCH] Fix handling of network errors for `beforeRetry` hook (#276) --- index.d.ts | 8 ++-- index.js | 21 ++++++---- readme.md | 8 ++-- test/hooks.js | 110 +++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 132 insertions(+), 15 deletions(-) diff --git a/index.d.ts b/index.d.ts index c7a2d77f..8653db00 100644 --- a/index.d.ts +++ b/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; diff --git a/index.js b/index.js index 5a5a51eb..37339171 100644 --- a/index.js +++ b/index.js @@ -303,7 +303,7 @@ class Ky { const modifiedResponse = await hook( this.request, this._options, - response.clone() + this._decorateResponse(response.clone()) ); if (modifiedResponse instanceof globals.Response) { @@ -311,6 +311,8 @@ class Ky { } } + 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 }); diff --git a/readme.md b/readme.md index c79890ec..1c32bf20 100644 --- a/readme.md +++ b/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; diff --git a/test/hooks.js b/test/hooks.js index ab077f7e..a583aea6 100644 --- a/test/hooks.js +++ b/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;