feat: Support timeout for fetch and node-fetch v3 (#660)

* feat: Support `timeout` for `fetch` and `node-fetch` v3

* test: fix

* test: fix

* feat: `timeout`s should be retryable

* chore: lint fixes

* chore: test clean-up

README.md

@@ -87,8 +87,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

src/common.ts

@@ -245,6 +245,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

src/gaxios.ts

@@ -484,6 +484,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),

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'
   ) {

test/test.getch.ts

@@ -693,6 +693,47 @@ 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 = 4000; // after network delay, so this shouldn't trigger
+      const message = 'Changed my mind - no request please';
+
+      setTimeout(() => ac.abort(message), 10);
+
+      await assert.rejects(
+        () => gaxios.request({url, timeout, signal}),
+        // `node-fetch` always rejects with the generic 'abort' error:
+        /abort/,
+        // native `fetch` matches the error properly:
+        // new RegExp(message)
+      );
+    });
+  });
 });
 
 describe('🎏 data handling', () => {

test/test.retry.ts

@@ -365,4 +365,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();
+  });
 });