From f2bdf371a600066102207f34cd5d0006e0848252 Mon Sep 17 00:00:00 2001 From: paulambanks Date: Wed, 30 Sep 2026 12:02:51 +0100 Subject: [PATCH 1/2] Make retry work with Node's fetch, and only retry safe requests (BY-3620) --- README.MD | 5 ++- lib/index.js | 7 ++-- lib/index.spec.js | 100 ++++++++++++++++++++++++++++++++++++++++++++++ lib/types.ts | 2 + 4 files changed, 110 insertions(+), 4 deletions(-) diff --git a/README.MD b/README.MD index 9cd379a..f40d62b 100644 --- a/README.MD +++ b/README.MD @@ -270,11 +270,14 @@ Api.configure({ retry: { attempts: 3 // How many times to retry before giving up errors: [ 'ECONNRESET' ] // A list of error codes + methods: [ 'GET', 'HEAD', 'OPTIONS' ] // Optional, defaults to these idempotent methods } }) ``` -errors is an array of any number of the [nodejs network error codes](https://nodejs.org/api/errors.html#errors_common_system_errors) +errors is an array of any number of the [nodejs network error codes](https://nodejs.org/api/errors.html#errors_common_system_errors). The code is matched on the error or on its `cause`, which is where Node's `fetch` puts it (`TypeError: fetch failed`). + +Only idempotent methods are retried by default, so a write that reached the server isn't sent twice. Add other methods to `methods` only where the endpoint is safe to repeat. ## Parsing error payloads diff --git a/lib/index.js b/lib/index.js index 5565c37..e55566f 100644 --- a/lib/index.js +++ b/lib/index.js @@ -315,9 +315,10 @@ class Api { throw new ClientError(r.statusText, content, r.status) } catch (e) { // @ts-ignore - if (retry.attempts && retry.errors && attempt < retry.attempts && retry.errors.includes(e.code)) { - // @ts-ignore - console.warn(`Got ${e.code} when calling ${endpoint}. Retrying request (${attempt}/${retry.attempts})`) + const code = e.code || (e.cause && e.cause.code) + const methods = retry.methods || [ 'GET', 'HEAD', 'OPTIONS' ] + if (retry.attempts && retry.errors && attempt < retry.attempts && retry.errors.includes(code) && methods.includes(options.method)) { + console.warn(`Got ${code} when calling ${endpoint}. Retrying request (${attempt}/${retry.attempts})`) return this.#doQuery(++attempt, client, endpoint, options) } diff --git a/lib/index.spec.js b/lib/index.spec.js index 1c450f1..21c6d28 100644 --- a/lib/index.spec.js +++ b/lib/index.spec.js @@ -733,6 +733,106 @@ describe('util/api', () => { }) }) + context('fetch network errors', function () { + function fetchFailed (code) { + const cause = new Error(`read ${code}`) + cause.code = code + return new TypeError('fetch failed', { cause }) + } + + beforeEach(function () { + global.FormData = Map + this.ctx = {} + this.ctx.clientStub = stub() + this.ctx.clientStub.onFirstCall().rejects(fetchFailed('ECONNRESET')) + this.ctx.clientStub.onSecondCall().resolves({ + status: 200, + json: stub().resolves({ foo: 'bar' }), + headers: new Map() + }) + }) + + context('GET', function () { + beforeEach(async function () { + const api = new Api({ + baseUrl: 'https://example.com/api/v1', + retry: { errors: [ 'ECONNRESET' ], attempts: 3 } + }) + + this.ctx.res = await api + .context({ fetch: this.ctx.clientStub }) + .endpoint('some/url') + .get(({ foo }) => foo) + }) + + it('retries using the code on the error cause', function () { + expect(this.ctx.clientStub.callCount).to.equal(2) + }) + + it('returns value', function () { + expect(this.ctx.res).to.equal('bar') + }) + }) + + context('POST', function () { + beforeEach(async function () { + const api = new Api({ + baseUrl: 'https://example.com/api/v1', + retry: { errors: [ 'ECONNRESET' ], attempts: 3 } + }) + + await api + .context({ fetch: this.ctx.clientStub }) + .endpoint('some/url') + .default(() => {}) + .post() + }) + + it('does not retry', function () { + expect(this.ctx.clientStub.callCount).to.equal(1) + }) + }) + + context('GET with an error code that is not listed', function () { + beforeEach(async function () { + this.ctx.clientStub.onFirstCall().rejects(fetchFailed('ETIMEDOUT')) + + const api = new Api({ + baseUrl: 'https://example.com/api/v1', + retry: { errors: [ 'ECONNRESET' ], attempts: 3 } + }) + + await api + .context({ fetch: this.ctx.clientStub }) + .endpoint('some/url') + .default(() => {}) + .get() + }) + + it('does not retry', function () { + expect(this.ctx.clientStub.callCount).to.equal(1) + }) + }) + + context('POST with methods configured', function () { + beforeEach(async function () { + const api = new Api({ + baseUrl: 'https://example.com/api/v1', + retry: { errors: [ 'ECONNRESET' ], attempts: 3, methods: [ 'POST' ] } + }) + + await api + .context({ fetch: this.ctx.clientStub }) + .endpoint('some/url') + .post() + }) + + it('retries', function () { + expect(this.ctx.clientStub.callCount).to.equal(2) + }) + }) + }) + context('passes context to error handlers', () => { const clientStub = stub() const defaultErrorHandler = stub() diff --git a/lib/types.ts b/lib/types.ts index 51f1b27..b82c286 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -26,6 +26,8 @@ export type RetryOptions = { attempts: number; /** Error codes to retry on */ errors: string[]; + /** HTTP methods to retry, defaults to GET, HEAD and OPTIONS */ + methods?: string[]; }; /** From a813f4042bf4cfda656efcf9b4996756de0b4c0f Mon Sep 17 00:00:00 2001 From: paulambanks Date: Wed, 30 Sep 2026 13:14:40 +0100 Subject: [PATCH 2/2] comment --- README.MD | 34 +- lib/index.js | 29 +- lib/index.spec.js | 855 +++++++++++++++++++--------------------------- lib/types.ts | 12 +- 4 files changed, 412 insertions(+), 518 deletions(-) diff --git a/README.MD b/README.MD index f40d62b..6e7b5f6 100644 --- a/README.MD +++ b/README.MD @@ -259,26 +259,38 @@ A global `default` handler catches anything that no other handler does. A reques ## Retries -The http client can retry if a network error is encountered. The default is `retry: false`, and requests won't be retried. +Retries are on by default (since 13.0.0). Configure them with the `retry` option: -Configure it as follows: +```js +retry: { + attempts: 1, // retries after the first request + errors: [ 'ECONNRESET' ], // Node network error codes to retry on + methods: [ 'GET', 'HEAD', 'OPTIONS' ] // HTTP methods that may be retried +} +``` + +These are the defaults. Leave `retry` out to use them, pass only the settings you want to change, or pass `false` to never retry: ```js import Api from '@beyonk/http' -Api.configure({ - retry: { - attempts: 3 // How many times to retry before giving up - errors: [ 'ECONNRESET' ] // A list of error codes - methods: [ 'GET', 'HEAD', 'OPTIONS' ] // Optional, defaults to these idempotent methods - } -}) +Api.configure({}) // defaults: 1 retry on ECONNRESET for GET, HEAD, OPTIONS +Api.configure({ retry: { attempts: 2 } }) // up to 2 retries, other settings unchanged +Api.configure({ retry: { errors: [ 'ECONNRESET', 'EPIPE' ] } }) // also retry EPIPE +Api.configure({ retry: false }) // never retry ``` -errors is an array of any number of the [nodejs network error codes](https://nodejs.org/api/errors.html#errors_common_system_errors). The code is matched on the error or on its `cause`, which is where Node's `fetch` puts it (`TypeError: fetch failed`). +| Setting | Meaning | Default | +| --- | --- | --- | +| `attempts` | Number of retries after the first request. `1` sends at most 2 requests, `0` never retries | `1` | +| `errors` | [Node network error codes](https://nodejs.org/api/errors.html#errors_common_system_errors) to retry on, matched on the error or its `cause` | `[ 'ECONNRESET' ]` | +| `methods` | HTTP methods that may be retried. Only add a method if repeating it is safe for every endpoint | `[ 'GET', 'HEAD', 'OPTIONS' ]` | -Only idempotent methods are retried by default, so a write that reached the server isn't sent twice. Add other methods to `methods` only where the endpoint is safe to repeat. +Good to know: +- Timeouts and HTTP error responses (4xx, 5xx) aren't retried. +- Retries only apply server-side, as browsers don't expose network error codes. +- Each retry logs a warning, e.g. `Got ECONNRESET when calling https://…/api/v1/apps. Retrying request (1/1)`. ## Parsing error payloads diff --git a/lib/index.js b/lib/index.js index e55566f..5af27a5 100644 --- a/lib/index.js +++ b/lib/index.js @@ -95,6 +95,24 @@ function getErrorByCode (code) { return errorMapping[code] || HttpError } +/** + * @param {ApiOptions['retry']} retry + * @returns {{ attempts: number, errors: string[], methods: string[] }} + */ +function retrySettings (retry) { + if (retry === false) { + return { attempts: 0, errors: [], methods: [] } + } + + const { + attempts = 1, + errors = [ 'ECONNRESET' ], + methods = [ 'GET', 'HEAD', 'OPTIONS' ] + } = retry || {} + + return { attempts, errors, methods } +} + /** @type {RequestConfig} */ const DEFAULT_CONFIG = { endpoint: null, @@ -130,7 +148,6 @@ class Api { constructor (options) { /** @type {ApiOptions} */ this.options = Object.assign({ - retry: false, parseErrors: true, handlers: {} }, options) @@ -284,7 +301,6 @@ class Api { * @throws {HttpError} If the request fails */ async #doQuery (attempt, client, endpoint, options) { - const retry = this.options.retry || { attempts: 1 } try { const r = await client(endpoint, options) @@ -314,10 +330,15 @@ class Api { const ClientError = getErrorByCode(r.status) throw new ClientError(r.statusText, content, r.status) } catch (e) { + const retry = retrySettings(this.options.retry) // @ts-ignore const code = e.code || (e.cause && e.cause.code) - const methods = retry.methods || [ 'GET', 'HEAD', 'OPTIONS' ] - if (retry.attempts && retry.errors && attempt < retry.attempts && retry.errors.includes(code) && methods.includes(options.method)) { + const retriesCount = attempt - 1 + const hasAttemptsLeft = retriesCount < retry.attempts + const isRetryableError = retry.errors.includes(code) + const isRetryableMethod = retry.methods.includes(options.method) + + if (hasAttemptsLeft && isRetryableError && isRetryableMethod) { console.warn(`Got ${code} when calling ${endpoint}. Retrying request (${attempt}/${retry.attempts})`) return this.#doQuery(++attempt, client, endpoint, options) } diff --git a/lib/index.spec.js b/lib/index.spec.js index 21c6d28..8abe621 100644 --- a/lib/index.spec.js +++ b/lib/index.spec.js @@ -4,13 +4,34 @@ import { Api } from './index.js' const { stub } = sinon +const baseUrl = 'https://example.com/api/v1' + +function createApi (options = {}) { + return new Api({ baseUrl, ...options }) +} + +function response ({ status = 200, statusText, body, headers = new Map(), ...overrides } = {}) { + return { status, statusText, headers, json: stub().resolves(body), ...overrides } +} + +function fetchFailed (code) { + const cause = new Error(`read ${code}`) + cause.code = code + return new TypeError('fetch failed', { cause }) +} + +function errorWithCode (code) { + const error = new Error(`read ${code}`) + error.code = code + return error +} + describe('util/api', () => { describe('#get()', () => { context('error handling', () => { let clientStub beforeEach(async () => { - global.FormData = Map clientStub = stub() }) @@ -18,17 +39,9 @@ describe('util/api', () => { let error const expectedError = 'hit default handler' - const api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + const api = createApi() - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 419, - headers: new Map() - }) + clientStub.resolves(response({ status: 419, statusText: 'No', body: { error: 'no' } })) await api .context({ fetch: clientStub }) @@ -45,17 +58,9 @@ describe('util/api', () => { let error const expectedError = 'hit local 401 handler' - const api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + const api = createApi() - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 401, - headers: new Map() - }) + clientStub.resolves(response({ status: 401, statusText: 'No', body: { error: 'no' } })) await api .context({ fetch: clientStub }) @@ -72,8 +77,7 @@ describe('util/api', () => { let error const expectedError = 'hit global 401 handler' - const api = new Api({ - baseUrl: 'https://example.com/api/v1', + const api = createApi({ handlers: { accessDenied: () => { error = expectedError @@ -81,13 +85,7 @@ describe('util/api', () => { } }) - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 401, - headers: new Map() - }) + clientStub.resolves(response({ status: 401, statusText: 'No', body: { error: 'no' } })) await api .context({ fetch: clientStub }) @@ -101,8 +99,7 @@ describe('util/api', () => { let globalCalled = false let localCalled = false - const api = new Api({ - baseUrl: 'https://example.com/api/v1', + const api = createApi({ handlers: { accessDenied: () => { globalCalled = true @@ -110,13 +107,7 @@ describe('util/api', () => { } }) - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 401, - headers: new Map() - }) + clientStub.resolves(response({ status: 401, statusText: 'No', body: { error: 'no' } })) await api .context({ fetch: clientStub }) @@ -134,8 +125,7 @@ describe('util/api', () => { let globalCalled = false let defaultCalled = false - const api = new Api({ - baseUrl: 'https://example.com/api/v1', + const api = createApi({ handlers: { accessDenied: () => { globalCalled = true @@ -143,13 +133,7 @@ describe('util/api', () => { } }) - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 401, - headers: new Map() - }) + clientStub.resolves(response({ status: 401, statusText: 'No', body: { error: 'no' } })) await api .context({ fetch: clientStub }) @@ -166,8 +150,7 @@ describe('util/api', () => { it('falls back to global default handler', async () => { let received - const api = new Api({ - baseUrl: 'https://example.com/api/v1', + const api = createApi({ handlers: { default: (e) => { received = e @@ -176,13 +159,7 @@ describe('util/api', () => { } }) - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 404, - headers: new Map() - }) + clientStub.resolves(response({ status: 404, statusText: 'No', body: { error: 'no' } })) const result = await api .context({ fetch: clientStub }) @@ -196,8 +173,7 @@ describe('util/api', () => { it('prefers request default handler to global default handler', async () => { let globalCalled = false - const api = new Api({ - baseUrl: 'https://example.com/api/v1', + const api = createApi({ handlers: { default: () => { globalCalled = true @@ -205,13 +181,7 @@ describe('util/api', () => { } }) - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 404, - headers: new Map() - }) + clientStub.resolves(response({ status: 404, statusText: 'No', body: { error: 'no' } })) const result = await api .context({ fetch: clientStub }) @@ -226,17 +196,9 @@ describe('util/api', () => { it('attaches the status and request to http errors', async () => { let received - const api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + const api = createApi() - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 404, - headers: new Map() - }) + clientStub.resolves(response({ status: 404, statusText: 'No', body: { error: 'no' } })) await api .context({ fetch: clientStub }) @@ -255,9 +217,7 @@ describe('util/api', () => { it('attaches the request to errors thrown by fetch', async () => { let received - const api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + const api = createApi() clientStub.rejects(new TypeError('fetch failed')) @@ -279,17 +239,9 @@ describe('util/api', () => { const calls = [] const logger = { error: (details, message) => calls.push(['log', details, message]) } - const api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + const api = createApi() - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 404, - headers: new Map() - }) + clientStub.resolves(response({ status: 404, statusText: 'No', body: { error: 'no' } })) await api .context({ fetch: clientStub, logger }) @@ -310,8 +262,7 @@ describe('util/api', () => { it('logs to the console when the context has no logger', async () => { const consoleError = stub(console, 'error') - const api = new Api({ - baseUrl: 'https://example.com/api/v1', + const api = createApi({ handlers: { default: () => 'handled' } }) @@ -329,17 +280,9 @@ describe('util/api', () => { it('does not log when a status-specific handler handles the error', async () => { let logged = false - const api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + const api = createApi() - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 404, - headers: new Map() - }) + clientStub.resolves(response({ status: 404, statusText: 'No', body: { error: 'no' } })) await api .context({ fetch: clientStub, logger: { error: () => { logged = true } } }) @@ -356,19 +299,12 @@ describe('util/api', () => { let clientStub beforeEach(async () => { - global.FormData = Map clientStub = stub() - api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + api = createApi() }) it('fetches data from url and passes it to a function', async () => { - clientStub.resolves({ - status: 200, - json: stub().resolves({ foo: 'bar' }), - headers: new Map() - }) + clientStub.resolves(response({ body: { foo: 'bar' } })) expect( await api .context({ fetch: clientStub }) @@ -378,11 +314,7 @@ describe('util/api', () => { }) it('fetches data from url and passes it to an async function', async () => { - clientStub.resolves({ - status: 200, - json: stub().resolves({ foo: 'bar' }), - headers: new Map() - }) + clientStub.resolves(response({ body: { foo: 'bar' } })) expect( await api .context({ fetch: clientStub }) @@ -395,11 +327,7 @@ describe('util/api', () => { }) it('fetches data from url and returns it', async () => { - clientStub.resolves({ - status: 200, - json: stub().resolves({ foo: 'bar' }), - headers: new Map() - }) + clientStub.resolves(response({ body: { foo: 'bar' } })) expect( await api .context({ fetch: clientStub }) @@ -409,41 +337,25 @@ describe('util/api', () => { }) it('appends baseUrl if endpoint is relative', async () => { - clientStub.resolves({ - status: 200, - json: stub(), - headers: new Map() - }) + clientStub.resolves(response()) await api.context({ fetch: clientStub }).endpoint('some/url').get() expect(clientStub.firstCall.args[0]).to.equal('https://example.com/api/v1/some/url') }) it('fetches data from url with query params', async () => { - clientStub.resolves({ - status: 200, - json: stub(), - headers: new Map() - }) + clientStub.resolves(response()) await api.context({ fetch: clientStub }).endpoint('some/url').query({ foo: 'bar', baz: 'qux' }).get() expect(clientStub.firstCall.args[0]).to.endWith('/some/url?foo=bar&baz=qux') }) it('fetches data from url with query params multiple', async () => { - clientStub.resolves({ - status: 200, - json: stub(), - headers: new Map() - }) + clientStub.resolves(response()) await api.context({ fetch: clientStub }).endpoint('some/url').query({ foo: [ 'bar', 'qux' ] }).get() expect(clientStub.firstCall.args[0]).to.endWith('/some/url?foo=bar&foo=qux') }) it('fetches data from url ignoring undefined query params', async () => { - clientStub.resolves({ - status: 200, - json: stub(), - headers: new Map() - }) + clientStub.resolves(response()) await api.context({ fetch: clientStub }).endpoint('some/url').query({ foo: 'bar', baz: undefined }).get() expect(clientStub.firstCall.args[0]).to.endWith('/some/url?foo=bar') }) @@ -451,12 +363,7 @@ describe('util/api', () => { it('passes status code as second parameter', async () => { const httpStatusCode = 202 - clientStub.resolves({ - status: httpStatusCode, - json: stub().resolves({ foo: 'bar' }), - statusText: 'Accepted', - headers: new Map() - }) + clientStub.resolves(response({ status: httpStatusCode, statusText: 'Accepted', body: { foo: 'bar' } })) const code = await api .context({ fetch: clientStub }) @@ -469,34 +376,23 @@ describe('util/api', () => { }) it('passes the whole response as third parameter', async () => { - - const stubbedResponse = { - status: 202, - json: stub().resolves({ foo: 'bar' }), - statusText: 'Accepted', - headers: new Map([ ['john', 'doe'] ]) - } + const stubbedResponse = response({ status: 202, statusText: 'Accepted', body: { foo: 'bar' }, headers: new Map([ [ 'john', 'doe' ] ]) }) clientStub.resolves(stubbedResponse) - const response = await api + const received = await api .context({ fetch: clientStub }) .endpoint('some/url') - .get((json, statusCode, response) => { - return response + .get((json, statusCode, fullResponse) => { + return fullResponse }) - expect(response.headers).to.equal(stubbedResponse.headers) + expect(received.headers).to.equal(stubbedResponse.headers) }) it('parses 3xx as successful', async () => { const httpStatusCode = 304 - clientStub.resolves({ - json: stub().resolves({ foo: 'bar' }), - statusText: 'Not Modified', - status: httpStatusCode, - headers: new Map() - }) + clientStub.resolves(response({ status: httpStatusCode, statusText: 'Not Modified', body: { foo: 'bar' } })) const code = await api .context({ fetch: clientStub }) @@ -512,12 +408,7 @@ describe('util/api', () => { const httpStatusCode = 204 const jsonFunction = stub() - clientStub.resolves({ - json: jsonFunction, - statusText: 'No Content', - status: httpStatusCode, - headers: new Map() - }) + clientStub.resolves(response({ status: httpStatusCode, statusText: 'No Content', json: jsonFunction })) await api .context({ fetch: clientStub }) @@ -536,12 +427,7 @@ describe('util/api', () => { const headers = new Map() headers.set('content-length', '0') - clientStub.resolves({ - json: jsonFunction, - headers, - statusText: 'OK', - status: httpStatusCode - }) + clientStub.resolves(response({ status: httpStatusCode, statusText: 'OK', headers, json: jsonFunction })) await api .context({ fetch: clientStub }) @@ -553,17 +439,11 @@ describe('util/api', () => { expect(jsonFunction.callCount).to.equal(0) }) - it('with known status code', async () => { + it('passes the status text to a status-specific handler', async () => { let error const expectedErrorMessage = 'no' - clientStub.resolves({ - text: stub().resolves('foo'), - json: stub().resolves({ foo: 'bar' }), - statusText: expectedErrorMessage, - status: 401, - headers: new Map() - }) + clientStub.resolves(response({ status: 401, statusText: expectedErrorMessage, body: { foo: 'bar' }, text: stub().resolves('foo') })) await api .context({ fetch: clientStub }) @@ -577,7 +457,7 @@ describe('util/api', () => { }) }) - context('parses errors', () => { + context('parses error bodies', () => { let api let clientStub @@ -586,25 +466,17 @@ describe('util/api', () => { } beforeEach(async () => { - global.FormData = Map clientStub = stub() - api = new Api({ - baseUrl: 'https://example.com/api/v1', + api = createApi({ parseErrors: true }) }) - it('with known status code', async () => { + it('passes the parsed body to a status-specific handler', async () => { let payload const expectedErrorMessage = 'no' - clientStub.resolves({ - ok: false, - json: stub().resolves(jsonPayload), - statusText: expectedErrorMessage, - status: 401, - headers: new Map() - }) + clientStub.resolves(response({ status: 401, statusText: expectedErrorMessage, body: jsonPayload })) await api .context({ fetch: clientStub }) @@ -614,319 +486,368 @@ describe('util/api', () => { }) .get() - expect(payload).to.equal(payload) + expect(payload).to.equal(jsonPayload) }) }) - context('retries', () => { - context('succeeds on third try', () => { - let api - let clientStub - let res - - class MySystemError extends Error { - constructor () { - super() - this.code = 'ECONNRESET' - } - } + context('retries', function () { + beforeEach(function () { + this.warn = stub(console, 'warn') + this.clientStub = stub() + this.clientStub.onFirstCall().rejects(fetchFailed('ECONNRESET')) + this.clientStub.onSecondCall().resolves(response({ body: { foo: 'bar' } })) + }) - beforeEach(async () => { - global.FormData = Map - clientStub = stub() - clientStub.onFirstCall().rejects(new MySystemError()) - clientStub.onSecondCall().rejects(new MySystemError()) - clientStub.onThirdCall().resolves({ - status: 200, - json: stub().resolves({ foo: 'bar' }), - headers: new Map() - }) + afterEach(function () { + this.warn.restore() + }) - api = new Api({ - baseUrl: 'https://example.com/api/v1', - retry: { - errors: [ 'ECONNRESET' ], - attempts: 3 - } - }) + context('GET with the default retry settings', function () { + beforeEach(async function () { + const api = createApi() - res = await api - .context({ fetch: clientStub }) + this.res = await api + .context({ fetch: this.clientStub }) .endpoint('some/url') .get(({ foo }) => foo) }) - afterEach(() => { - clientStub.reset() + it('retries a reset connection once', function () { + expect(this.clientStub.callCount).to.equal(2) }) - it('retries three times', () => { - expect( - clientStub.callCount - ).to.equal(3) + it('returns value', function () { + expect(this.res).to.equal('bar') }) - it('returns value', () => { - expect( - res - ).to.equal('bar') + it('logs a warning for the retry', function () { + expect(this.warn.firstCall.args[0]).to.equal('Got ECONNRESET when calling https://example.com/api/v1/some/url. Retrying request (1/1)') }) }) - context('never succeeds', () => { - let api - let clientStub - let successStub - let error + context('GET with retry turned off', function () { + beforeEach(async function () { + const api = createApi({ retry: false }) - class MySystemError extends Error { - constructor () { - super() - this.code = 'ECONNRESET' - } - } + await api + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .default(() => {}) + .get() + }) - beforeEach(async () => { - global.FormData = Map - clientStub = stub() - clientStub.onFirstCall().rejects(new MySystemError()) - clientStub.onSecondCall().rejects(new MySystemError()) - - successStub = stub() - - api = new Api({ - baseUrl: 'https://example.com/api/v1', - retry: { - errors: [ 'ECONNRESET' ], - attempts: 2 - } - }) + it('does not retry', function () { + expect(this.clientStub.callCount).to.equal(1) + }) + }) - await api - .context({ fetch: clientStub }) + context('GET reset every time with one retry', function () { + beforeEach(async function () { + this.clientStub.onSecondCall().rejects(fetchFailed('ECONNRESET')) + this.successHandler = stub() + + const api = createApi({ retry: { attempts: 1 } }) + + this.result = await api + .context({ fetch: this.clientStub }) .endpoint('some/url') - .default(e => { - error = 'stuff happened' + .default((e) => { + this.error = e + return 'default handler ran' }) - .get(successStub) + .get(this.successHandler) }) - afterEach(() => { - clientStub.reset() - successStub.reset() + it('retries once then gives up', function () { + expect(this.clientStub.callCount).to.equal(2) }) - it('retries three times', () => { - expect( - clientStub.callCount - ).to.equal(2) + it('runs the default handler', function () { + expect(this.result).to.equal('default handler ran') }) - it('catches default error', () => { - expect(error).to.equal('stuff happened') + it('does not call the success handler', function () { + expect(this.successHandler.callCount).to.equal(0) }) - it('does not call success method', () => { - expect( - successStub.callCount - ).to.equal(0) + it('attaches the request to the error', function () { + expect(this.error.request).to.equal({ method: 'GET', url: 'https://example.com/api/v1/some/url' }) }) }) - context('fetch network errors', function () { - function fetchFailed (code) { - const cause = new Error(`read ${code}`) - cause.code = code - return new TypeError('fetch failed', { cause }) - } + context('GET reset every time with three retries', function () { + beforeEach(async function () { + this.clientStub.onSecondCall().rejects(fetchFailed('ECONNRESET')) + this.clientStub.onThirdCall().rejects(fetchFailed('ECONNRESET')) + this.clientStub.onCall(3).rejects(fetchFailed('ECONNRESET')) + this.clientStub.onCall(4).resolves(response({ body: { foo: 'bar' } })) - beforeEach(function () { - global.FormData = Map - this.ctx = {} - this.ctx.clientStub = stub() - this.ctx.clientStub.onFirstCall().rejects(fetchFailed('ECONNRESET')) - this.ctx.clientStub.onSecondCall().resolves({ - status: 200, - json: stub().resolves({ foo: 'bar' }), - headers: new Map() - }) + await createApi({ retry: { attempts: 3 } }) + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .default(() => {}) + .get() }) - context('GET', function () { - beforeEach(async function () { - const api = new Api({ - baseUrl: 'https://example.com/api/v1', - retry: { errors: [ 'ECONNRESET' ], attempts: 3 } - }) + it('stops after three retries', function () { + expect(this.clientStub.callCount).to.equal(4) + }) + }) - this.ctx.res = await api - .context({ fetch: this.ctx.clientStub }) - .endpoint('some/url') - .get(({ foo }) => foo) - }) + context('GET with no retries allowed', function () { + beforeEach(async function () { + await createApi({ retry: { attempts: 0 } }) + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .default(() => {}) + .get() + }) - it('retries using the code on the error cause', function () { - expect(this.ctx.clientStub.callCount).to.equal(2) - }) + it('does not retry', function () { + expect(this.clientStub.callCount).to.equal(1) + }) + }) - it('returns value', function () { - expect(this.ctx.res).to.equal('bar') - }) + context('GET that gets an HTTP error response', function () { + beforeEach(async function () { + this.clientStub.onFirstCall().resolves(response({ status: 503, statusText: 'Unavailable' })) + + await createApi() + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .default(() => {}) + .get() }) - context('POST', function () { - beforeEach(async function () { - const api = new Api({ - baseUrl: 'https://example.com/api/v1', - retry: { errors: [ 'ECONNRESET' ], attempts: 3 } - }) + it('does not retry', function () { + expect(this.clientStub.callCount).to.equal(1) + }) + }) - await api - .context({ fetch: this.ctx.clientStub }) + for (const method of [ 'put', 'patch', 'del' ]) { + context(`${method.toUpperCase()} with the default retry settings`, function () { + beforeEach(async function () { + await createApi() + .context({ fetch: this.clientStub }) .endpoint('some/url') - .default(() => {}) - .post() + .default(() => {})[method]() }) it('does not retry', function () { - expect(this.ctx.clientStub.callCount).to.equal(1) + expect(this.clientStub.callCount).to.equal(1) }) }) + } - context('GET with an error code that is not listed', function () { - beforeEach(async function () { - this.ctx.clientStub.onFirstCall().rejects(fetchFailed('ETIMEDOUT')) + context('PUT with only methods configured', function () { + beforeEach(async function () { + await createApi({ retry: { methods: [ 'PUT' ] } }) + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .put() + }) - const api = new Api({ - baseUrl: 'https://example.com/api/v1', - retry: { errors: [ 'ECONNRESET' ], attempts: 3 } - }) + it('retries using the default error codes and attempts', function () { + expect(this.clientStub.callCount).to.equal(2) + }) + }) - await api - .context({ fetch: this.ctx.clientStub }) - .endpoint('some/url') - .default(() => {}) - .get() - }) + context('GET reset twice with two retries', function () { + beforeEach(async function () { + this.clientStub.onSecondCall().rejects(fetchFailed('ECONNRESET')) + this.clientStub.onThirdCall().resolves(response({ body: { foo: 'bar' } })) - it('does not retry', function () { - expect(this.ctx.clientStub.callCount).to.equal(1) - }) + this.res = await createApi({ retry: { attempts: 2 } }) + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .get(({ foo }) => foo) }) - context('POST with methods configured', function () { - beforeEach(async function () { - const api = new Api({ - baseUrl: 'https://example.com/api/v1', - retry: { errors: [ 'ECONNRESET' ], attempts: 3, methods: [ 'POST' ] } - }) - - await api - .context({ fetch: this.ctx.clientStub }) - .endpoint('some/url') - .post() - }) + it('retries twice', function () { + expect(this.clientStub.callCount).to.equal(3) + }) - it('retries', function () { - expect(this.ctx.clientStub.callCount).to.equal(2) - }) + it('returns value', function () { + expect(this.res).to.equal('bar') }) }) - context('passes context to error handlers', () => { - const clientStub = stub() - const defaultErrorHandler = stub() + context('GET with the code on the error itself', function () { + beforeEach(async function () { + this.clientStub.onFirstCall().rejects(errorWithCode('ECONNRESET')) - clientStub.resolves({ - ok: true, - status: 200, - headers: new Map(), - json: stub().resolves() + this.res = await createApi() + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .get(({ foo }) => foo) }) - const ctx = { fetch: clientStub, foo: 'bar' } - - class CustomError extends Error {} + it('retries', function () { + expect(this.clientStub.callCount).to.equal(2) + }) - let api - let error + it('returns value', function () { + expect(this.res).to.equal('bar') + }) + }) - beforeEach(async () => { - api = new Api({ - baseUrl: 'https://example.com/api/v1' + context('GET with custom retry settings', function () { + beforeEach(async function () { + const api = createApi({ + retry: { errors: [ 'ECONNRESET' ], attempts: 3 } }) - error = await expect( - api - .context(ctx) - .endpoint('some/url') - .default(defaultErrorHandler) - .get(() => { - throw new CustomError() - }) - ).to.reject() + this.res = await api + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .get(({ foo }) => foo) }) - it('throws success handler error externally', () => { - expect(error).to.be.an.instanceOf(CustomError) + it('retries using the code on the error cause', function () { + expect(this.clientStub.callCount).to.equal(2) }) - it('does not call default error handler', () => { - expect(defaultErrorHandler.callCount).to.equal(0) + it('returns value', function () { + expect(this.res).to.equal('bar') }) }) - context('error thrown in success handler is not caught', () => { - let api - let clientStub + context('POST', function () { + beforeEach(async function () { + const api = createApi({ + retry: { errors: [ 'ECONNRESET' ], attempts: 3 } + }) - const ctx = { fetch: clientStub, foo: 'bar' } + await api + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .default(() => {}) + .post() + }) - beforeEach(async () => { - clientStub = stub() - clientStub.resolves({ - status: 200, - json: stub().resolves({ foo: 'bar' }), - headers: new Map() - }) + it('does not retry', function () { + expect(this.clientStub.callCount).to.equal(1) + }) + }) - api = new Api({ - baseUrl: 'https://example.com/api/v1' + context('GET with an error code that is not listed', function () { + beforeEach(async function () { + this.clientStub.onFirstCall().rejects(fetchFailed('ETIMEDOUT')) + + const api = createApi({ + retry: { errors: [ 'ECONNRESET' ], attempts: 3 } }) await api - .context({ fetch: clientStub, foo: 'bar' }) + .context({ fetch: this.clientStub }) .endpoint('some/url') - .get(({ foo }) => foo) + .default(() => {}) + .get() }) - it('has cleared endpoint', () => { - expect(api.config.endpoint).to.equal(null) + it('does not retry', function () { + expect(this.clientStub.callCount).to.equal(1) }) + }) - it('has cleared method', () => { - expect(api.config.method).to.equal('GET') - }) + context('POST with methods configured', function () { + beforeEach(async function () { + const api = createApi({ + retry: { errors: [ 'ECONNRESET' ], attempts: 3, methods: [ 'POST' ] } + }) - it('has not cleared client', () => { - expect(api.client).to.equal(clientStub) + await api + .context({ fetch: this.clientStub }) + .endpoint('some/url') + .post() }) - it('has not cleared context', () => { - expect(api.ctx.foo).to.equal(ctx.foo) + it('retries', function () { + expect(this.clientStub.callCount).to.equal(2) }) + }) + }) - it('has cleared query', () => { - expect(api.config.query).to.equal(null) - }) + context('error thrown in success handler is not caught', () => { + const clientStub = stub() + const defaultErrorHandler = stub() - it('has cleared payload', () => { - expect(api.config.payload).to.equal(null) - }) + clientStub.resolves(response()) - it('has cleared overrides', () => { - expect(api.config.overrides).to.equal({}) - }) + const ctx = { fetch: clientStub, foo: 'bar' } + + class CustomError extends Error {} + + let api + let error + + beforeEach(async () => { + api = createApi() + + error = await expect( + api + .context(ctx) + .endpoint('some/url') + .default(defaultErrorHandler) + .get(() => { + throw new CustomError() + }) + ).to.reject() + }) + + it('throws success handler error externally', () => { + expect(error).to.be.an.instanceOf(CustomError) + }) + + it('does not call default error handler', () => { + expect(defaultErrorHandler.callCount).to.equal(0) + }) + }) + + context('resets the request after it completes', () => { + let api + let clientStub + + const ctx = { foo: 'bar' } + + beforeEach(async () => { + clientStub = stub() + clientStub.resolves(response({ body: { foo: 'bar' } })) + + api = createApi() + + await api + .context({ fetch: clientStub, foo: 'bar' }) + .endpoint('some/url') + .get(({ foo }) => foo) + }) + + it('has cleared endpoint', () => { + expect(api.config.endpoint).to.equal(null) + }) + + it('has reset method to GET', () => { + expect(api.config.method).to.equal('GET') + }) + + it('has not cleared client', () => { + expect(api.client).to.equal(clientStub) + }) + + it('has not cleared context', () => { + expect(api.ctx.foo).to.equal(ctx.foo) + }) + + it('has cleared query', () => { + expect(api.config.query).to.equal(null) + }) + + it('has cleared payload', () => { + expect(api.config.payload).to.equal(null) + }) + + it('has cleared overrides', () => { + expect(api.config.overrides).to.equal({}) }) }) @@ -938,9 +859,7 @@ describe('util/api', () => { const ctx = { fetch: clientStub, foo: 'bar' } beforeEach(async () => { - api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + api = createApi() await api .context(ctx) @@ -962,9 +881,7 @@ describe('util/api', () => { const ctx = { foo: 'bar' } beforeEach(async () => { - api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + api = createApi() await api .context(ctx) @@ -985,20 +902,12 @@ describe('util/api', () => { const clientStub = stub() let passed - clientStub.resolves({ - ok: false, - statusText: 'No', - json: stub().resolves({ error: 'no' }), - status: 406, - headers: new Map() - }) + clientStub.resolves(response({ status: 406, statusText: 'No', body: { error: 'no' } })) const ctx = { fetch: clientStub, foo: 'bar' } beforeEach(async () => { - api = new Api({ - baseUrl: 'https://example.com/api/v1' - }) + api = createApi() await api .context(ctx) @@ -1022,17 +931,11 @@ describe('util/api', () => { beforeEach(async () => { clientStub = stub() - api = new Api({ - baseUrl: 'https://example.com/api/v1' - }, clientStub) + api = createApi() }) it('posts data to url', async () => { - clientStub.resolves({ - status: 201, - json: stub(), - headers: new Map() - }) + clientStub.resolves(response({ status: 201 })) const content = { foo: 'bar' } await api @@ -1052,11 +955,7 @@ describe('util/api', () => { }) it('sets an authorization header', async () => { - clientStub.resolves({ - status: 201, - json: stub(), - headers: new Map() - }) + clientStub.resolves(response({ status: 201 })) const content = { foo: 'bar' } await api @@ -1086,17 +985,11 @@ describe('util/api', () => { beforeEach(async () => { clientStub = stub() - api = new Api({ - baseUrl: 'https://example.com/api/v1' - }, clientStub) + api = createApi() }) it('sends data to url', async () => { - clientStub.resolves({ - status: 200, - json: stub(), - headers: new Map() - }) + clientStub.resolves(response()) const content = { foo: 'bar' } await api @@ -1116,11 +1009,7 @@ describe('util/api', () => { }) it('sets an authorization header', async () => { - clientStub.resolves({ - status: 201, - json: stub(), - headers: new Map() - }) + clientStub.resolves(response({ status: 201 })) const content = { foo: 'bar' } await api @@ -1150,17 +1039,11 @@ describe('util/api', () => { beforeEach(async () => { clientStub = stub() - api = new Api({ - baseUrl: 'https://example.com/api/v1' - }, clientStub) + api = createApi() }) - it('fetches data from url', async () => { - clientStub.resolves({ - status: 200, - json: stub(), - headers: new Map() - }) + it('sends data to url', async () => { + clientStub.resolves(response()) const content = { foo: 'bar' } await api.context({ fetch: clientStub }).endpoint('some/url').payload(content).put() @@ -1175,11 +1058,7 @@ describe('util/api', () => { }) it('endpoint returns no data', async () => { - clientStub.resolves({ - status: 200, - json: stub().rejects('Error'), - headers: new Map() - }) + clientStub.resolves(response({ json: stub().rejects('Error') })) expect( await api .context({ fetch: clientStub }) @@ -1189,11 +1068,7 @@ describe('util/api', () => { }) it('sets an authorization header', async () => { - clientStub.resolves({ - status: 200, - json: stub().resolves({ foo: 'bar' }), - headers: new Map() - }) + clientStub.resolves(response({ body: { foo: 'bar' } })) const content = { foo: 'bar' } await api @@ -1223,37 +1098,23 @@ describe('util/api', () => { beforeEach(async () => { clientStub = stub() - api = new Api({ - baseUrl: 'https://example.com/api/v1' - }, clientStub) + api = createApi() }) it('calls url with query params', async () => { - clientStub.resolves({ - status: 200, - json: stub().resolves({}), - headers: new Map() - }) + clientStub.resolves(response({ body: {} })) await api.context({ fetch: clientStub }).endpoint('some/url').query({ foo: 'bar', baz: 'qux' }).del() expect(clientStub.firstCall.args[0]).to.endWith('/some/url?foo=bar&baz=qux') }) it('calls url ignoring undefined query params', async () => { - clientStub.resolves({ - status: 200, - json: stub().resolves({}), - headers: new Map() - }) + clientStub.resolves(response({ body: {} })) await api.context({ fetch: clientStub }).endpoint('some/url').query({ foo: undefined, baz: 'qux' }).del() expect(clientStub.firstCall.args[0]).to.endWith('/some/url?baz=qux') }) it('calls url with delete method', async () => { - clientStub.resolves({ - status: 200, - json: stub().resolves({}), - headers: new Map() - }) + clientStub.resolves(response({ body: {} })) await api.context({ fetch: clientStub }).endpoint('some/url').del() expect(clientStub.firstCall.args[1].method).equals('DELETE') }) diff --git a/lib/types.ts b/lib/types.ts index b82c286..47b24eb 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -10,7 +10,7 @@ export type ApiOptions = { mock?: FetchClient; /** Fetch client for HTTP requests */ fetch?: FetchClient; - /** Whether to retry failed requests */ + /** Retry settings. Omit to use the defaults (1 retry on ECONNRESET for GET, HEAD, OPTIONS), or pass false to never retry */ retry?: RetryOptions | false; /** Whether to parse error responses as JSON */ parseErrors?: boolean; @@ -22,11 +22,11 @@ export type ApiOptions = { * Retry configuration */ export type RetryOptions = { - /** Number of retry attempts */ - attempts: number; - /** Error codes to retry on */ - errors: string[]; - /** HTTP methods to retry, defaults to GET, HEAD and OPTIONS */ + /** Number of retries after the first request. 0 never retries. Defaults to 1 */ + attempts?: number; + /** Node network error codes to retry on, matched on the error or its cause. Defaults to ['ECONNRESET'] */ + errors?: string[]; + /** HTTP methods that may be retried. Defaults to ['GET', 'HEAD', 'OPTIONS'] */ methods?: string[]; };