From f9a2d69343a821397cfe3d55ea190c7ff80bbafc Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 25 Oct 2024 13:22:14 -0700 Subject: [PATCH 1/7] feat: Support `timeout` for `fetch` and `node-fetch` v3 --- README.md | 4 ++-- src/common.ts | 3 +++ src/gaxios.ts | 10 ++++++++++ test/test.getch.ts | 38 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0114e2f8..2cbe6a26 100644 --- a/README.md +++ b/README.md @@ -86,8 +86,8 @@ interface GaxiosOptions = { querystring: 'parameters' }, - // The timeout for the HTTP request in milliseconds. Defaults to 0. - timeout: 1000, + // The timeout for the HTTP request in milliseconds. No timeout by default. + timeout: 60000, // Optional method to override making the actual HTTP request. Useful // for writing tests and instrumentation diff --git a/src/common.ts b/src/common.ts index 911a0505..47665355 100644 --- a/src/common.ts +++ b/src/common.ts @@ -234,6 +234,9 @@ export interface GaxiosOptions extends RequestInit { * @deprecated Use {@link URLSearchParams} instead and pass this directly to {@link GaxiosOptions.data `data`}. */ paramsSerializer?: (params: {[index: string]: string | number}) => string; + /** + * A timeout for the request, in milliseconds. No timeout by default. + */ timeout?: number; /** * @deprecated ignored diff --git a/src/gaxios.ts b/src/gaxios.ts index 8e2a2d32..3dbfcd70 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -468,6 +468,16 @@ export class Gaxios { (opts as {duplex: string}).duplex = 'half'; } + if (opts.timeout) { + const timeoutSignal = AbortSignal.timeout(opts.timeout); + + if (opts.signal) { + opts.signal = AbortSignal.any([opts.signal, timeoutSignal]); + } else { + opts.signal = timeoutSignal; + } + } + return Object.assign(opts, { headers: preparedHeaders, url: opts.url instanceof URL ? opts.url : new URL(opts.url), diff --git a/test/test.getch.ts b/test/test.getch.ts index 93426f29..03da7e8b 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -690,6 +690,44 @@ describe('🥁 configuration options', () => { assert.equal(instance.defaults.errorRedactor, errorRedactor); }); + + describe('timeout', () => { + it('should accept and use a `timeout`', async () => { + nock(url).get('/').delay(2000).reply(204); + const gaxios = new Gaxios(); + const timeout = 10; + + await assert.rejects(() => gaxios.request({url, timeout}), /abort/); + }); + + it('should a `timeout`, an existing `signal`, and be triggered by timeout', async () => { + nock(url).get('/').delay(2000).reply(204); + const gaxios = new Gaxios(); + const signal = new AbortController().signal; + const timeout = 10; + + await assert.rejects( + () => gaxios.request({url, timeout, signal}), + /abort/ + ); + }); + + it('should use a `timeout`, a `signal`, and be triggered by signal', async () => { + nock(url).get('/').delay(2000).reply(204); + const gaxios = new Gaxios(); + const ac = new AbortController(); + const signal = ac.signal; + const timeout = Number.MAX_SAFE_INTEGER; + const message = 'Changed my mind - no request please'; + + setTimeout(() => ac.abort(message), 10); + + await assert.rejects( + () => gaxios.request({url, timeout, signal}), + message + ); + }); + }); }); describe('🎏 data handling', () => { From 59a465563b9de51d29e228d8ac24e3238db733fd Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 25 Oct 2024 14:37:33 -0700 Subject: [PATCH 2/7] test: fix --- test/test.getch.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test.getch.ts b/test/test.getch.ts index 03da7e8b..8578e392 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -720,11 +720,11 @@ describe('🥁 configuration options', () => { const timeout = Number.MAX_SAFE_INTEGER; const message = 'Changed my mind - no request please'; - setTimeout(() => ac.abort(message), 10); + setTimeout(() => ac.abort(new Error(message)), 10); await assert.rejects( () => gaxios.request({url, timeout, signal}), - message + new RegExp(message) ); }); }); From d5bda53233fe26565c9d476f8333cf397481385b Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 25 Oct 2024 14:41:19 -0700 Subject: [PATCH 3/7] test: fix --- test/test.getch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.getch.ts b/test/test.getch.ts index 8578e392..b097777a 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -717,7 +717,7 @@ describe('🥁 configuration options', () => { const gaxios = new Gaxios(); const ac = new AbortController(); const signal = ac.signal; - const timeout = Number.MAX_SAFE_INTEGER; + const timeout = 4000; // after network delay, so this shouldn't trigger const message = 'Changed my mind - no request please'; setTimeout(() => ac.abort(new Error(message)), 10); From 0190a1dada809ce7b2bc1022d6765b6f5725bb82 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 25 Oct 2024 16:10:47 -0700 Subject: [PATCH 4/7] feat: `timeout`s should be retryable --- src/retry.ts | 2 +- test/test.getch.ts | 9 +++++++-- test/test.retry.ts | 27 +++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/retry.ts b/src/retry.ts index 6f696676..3e76bf14 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -103,7 +103,7 @@ function shouldRetryRequest(err: GaxiosError) { const config = getConfig(err); if ( - err.config.signal?.aborted || + (err.config.signal?.aborted && err.error?.name !== 'TimeoutError') || err.name === 'AbortError' || err.error?.name === 'AbortError' ) { diff --git a/test/test.getch.ts b/test/test.getch.ts index b097777a..5f1a6dec 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -720,11 +720,16 @@ describe('🥁 configuration options', () => { const timeout = 4000; // after network delay, so this shouldn't trigger const message = 'Changed my mind - no request please'; - setTimeout(() => ac.abort(new Error(message)), 10); + setTimeout(() => ac.abort(message), 10); + + // await gaxios.request({url, timeout, signal}); await assert.rejects( () => gaxios.request({url, timeout, signal}), - new RegExp(message) + // `node-fetch` always rejects with the generic 'abort' error: + /abort/ + // native `fetch` matches the error properly: + // new RegExp(message) ); }); }); diff --git a/test/test.retry.ts b/test/test.retry.ts index 00ecbe16..559c4312 100644 --- a/test/test.retry.ts +++ b/test/test.retry.ts @@ -367,4 +367,31 @@ describe('🛸 retry & exponential backoff', () => { assert.ok(delay > 4000 && delay < 4999); scope.done(); }); + + it('should retry on `timeout`', async () => { + let scope = nock(url).get('/').delay(2000).reply(400); + + const gaxios = new Gaxios(); + const timeout = 10; + + function onRetryAttempt() { + // prepare nock for next request + scope = nock(url).get('/').reply(204); + } + + const res = await gaxios.request({ + url, + timeout, + // NOTE: `node-fetch` does not yet support `TimeoutError` - testing with native `fetch` for now. + fetchImplementation: fetch, + retryConfig: { + onRetryAttempt, + }, + }); + + assert.equal(res.status, 204); + assert.equal(res.config?.retryConfig?.currentRetryAttempt, 1); + + scope.done(); + }); }); From 85a81c6830009557b3376e3b573145c425b66dd3 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 25 Oct 2024 16:37:42 -0700 Subject: [PATCH 5/7] chore: lint fixes --- test/test.getch.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test.getch.ts b/test/test.getch.ts index 284df58a..9a6c4e30 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -708,7 +708,7 @@ describe('🥁 configuration options', () => { await assert.rejects( () => gaxios.request({url, timeout, signal}), - /abort/ + /abort/, ); }); @@ -727,7 +727,7 @@ describe('🥁 configuration options', () => { await assert.rejects( () => gaxios.request({url, timeout, signal}), // `node-fetch` always rejects with the generic 'abort' error: - /abort/ + /abort/, // native `fetch` matches the error properly: // new RegExp(message) ); From 86c29db25e9f4caf0d8b25044920ea9c9ff8ce94 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 25 Oct 2024 16:41:15 -0700 Subject: [PATCH 6/7] feat!: Use `Error#cause` in `GaxiosError` --- src/common.ts | 14 ++++++++++---- src/gaxios.ts | 13 +++++++++---- src/retry.ts | 6 +----- tsconfig.json | 2 +- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/common.ts b/src/common.ts index 1d840e80..df00e15a 100644 --- a/src/common.ts +++ b/src/common.ts @@ -93,9 +93,9 @@ export class GaxiosError extends Error { message: string, public config: GaxiosOptionsPrepared, public response?: GaxiosResponse, - public error?: Error | NodeJS.ErrnoException, + cause?: unknown, ) { - super(message); + super(message, {cause}); // deep-copy config as we do not want to mutate // the existing config for future retries/use @@ -120,8 +120,14 @@ export class GaxiosError extends Error { this.status = this.response.status; } - if (error && 'code' in error && error.code) { - this.code = error.code; + if (this.cause instanceof Error) { + if (this.cause instanceof DOMException) { + // 'code' is legacy for DOMExceptions, use the `name` instead + // https://developer.mozilla.org/en-US/docs/Web/API/DOMException#error_names + this.code = this.cause.name; + } else if ('code' in this.cause && typeof this.cause.code === 'string') { + this.code = this.cause.code; + } } if (config.errorRedactor) { diff --git a/src/gaxios.ts b/src/gaxios.ts index 33918109..c2164ac4 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -144,10 +144,15 @@ export class Gaxios { } return translatedResponse; } catch (e) { - const err = - e instanceof GaxiosError - ? e - : new GaxiosError((e as Error).message, opts, undefined, e as Error); + let err: GaxiosError; + + if (e instanceof GaxiosError) { + err = e; + } else if (e instanceof Error) { + err = new GaxiosError(e.message, opts, undefined, e); + } else { + err = new GaxiosError(new Error(e as '').message, opts, undefined, e); + } const {shouldRetry, config} = await getRetryConfig(err); if (shouldRetry && config) { diff --git a/src/retry.ts b/src/retry.ts index ba792c59..c9ab9ca2 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -102,11 +102,7 @@ export async function getRetryConfig(err: GaxiosError) { function shouldRetryRequest(err: GaxiosError) { const config = getConfig(err); - if ( - err.config.signal?.aborted || - err.name === 'AbortError' || - err.error?.name === 'AbortError' - ) { + if (err.config.signal?.aborted || err.code === 'AbortError') { return false; } diff --git a/tsconfig.json b/tsconfig.json index 3538ab11..ed82b9de 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "./node_modules/gts/tsconfig-google.json", "compilerOptions": { - "lib": ["es2023", "dom"], + "lib": ["ES2023", "dom"], "rootDir": ".", "outDir": "build", "esModuleInterop": true, From ad63f648c901729ff4b4b234027697385dbce6d1 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 31 Oct 2024 21:10:49 -0700 Subject: [PATCH 7/7] fix: merge tsconfig --- tsconfig.base.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tsconfig.base.json b/tsconfig.base.json index 31a5d2ff..075a0bc9 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -1,7 +1,7 @@ { "extends": "gts/tsconfig-google.json", "compilerOptions": { - "lib": ["es2020", "dom"], + "lib": ["ES2023", "dom"], "esModuleInterop": true, "rootDir": "." },