Implement retries (#972)

* Implemented retries for fetch

* changed nock test setup

* Fixed nock setup order

* Ensure the retry is not negative

Author: odehamer

.changeset/loud-pigs-tell.md

@@ -0,0 +1,5 @@
+---
+'tuf-js': patch
+---
+
+Implement retry logic on fetch

package-lock.json

@@ -34,7 +34,8 @@
   },
   "dependencies": {
     "@tufjs/models": "4.1.0",
-    "debug": "^4.4.3"
+    "debug": "^4.4.3",
+    "@gar/promise-retry": "^1.0.3"
   },
   "engines": {
     "node": "^20.17.0 || >=22.9.0"

packages/client/package.json

@@ -4,8 +4,9 @@ import util from 'util';
 
 import { DownloadHTTPError, DownloadLengthMismatchError } from './error';
 import { withTempFile } from './utils/tmpfile';
+import { promiseRetry } from '@gar/promise-retry';
 
-import type { TimeoutsOptions } from 'retry';
+import type { OperationOptions, TimeoutsOptions } from 'retry';
 
 const log = debug('tuf:fetch');
 
@@ -95,31 +96,69 @@ interface FetcherOptions {
 export class DefaultFetcher extends BaseFetcher {
   private userAgent?: string;
   private timeout?: number;
-  private retry?: Retry;
+  private retry?: OperationOptions;
 
   constructor(options: FetcherOptions = {}) {
     super();
     this.userAgent = options.userAgent;
     this.timeout = options.timeout;
-    this.retry = options.retry;
+    // Map retry to OperationOptions
+    if (options.retry === true) {
+      this.retry = { forever: true };
+    } else if (options.retry === false || options.retry === undefined) {
+      this.retry = undefined;
+    } else if (typeof options.retry === 'number') {
+      if (options.retry < 0) {
+        throw new Error('Retry count must be non-negative number');
+      }
+      this.retry = { retries: options.retry };
+    } else {
+      this.retry = options.retry;
+    }
   }
 
   public override async fetch(
     url: string
   ): Promise<ReadableStream<Uint8Array<ArrayBuffer>>> {
-    log('GET %s', url);
-    const response = await fetch(url, {
-      headers: {
-        [USER_AGENT_HEADER]: this.userAgent || '',
-      },
-      signal: this.timeout ? AbortSignal.timeout(this.timeout) : undefined,
-    });
+    const shouldRetry = this.retry !== undefined;
+
+    return promiseRetry(
+      async (retry: (err: Error) => never, number: number) => {
+        log('GET %s (attempt %d)', url, number);
+
+        let response: Response;
+        try {
+          response = await fetch(url, {
+            headers: {
+              [USER_AGENT_HEADER]: this.userAgent || '',
+            },
+            signal: this.timeout
+              ? AbortSignal.timeout(this.timeout)
+              : undefined,
+          });
+        } catch (error) {
+          const err = error instanceof Error ? error : new Error(String(error));
+          if (shouldRetry) {
+            return retry(err);
+          }
+          throw err;
+        }
 
-    if (!response.ok || !response?.body) {
-      throw new DownloadHTTPError('Failed to download', response.status);
-    }
+        if (!response.ok || !response.body) {
+          const err = new DownloadHTTPError(
+            'Failed to download',
+            response.status
+          );
+          if (shouldRetry && response.status >= 500 && response.status < 600) {
+            return retry(err);
+          }
+          throw err;
+        }
 
-    return response.body;
+        return response.body;
+      },
+      this.retry
+    );
   }
 }
 

packages/client/src/fetcher.ts

@@ -47,13 +47,68 @@ describe('Fetcher Test', () => {
   });
 
   describe('Request Fetcher Fetch Test', () => {
-    beforeAll(() => {
+    it('fetch with bad status code', async () => {
+      const fetcher = new DefaultFetcher();
       nock(baseURL).get('/').reply(404, 'error');
+      await expect(fetcher.fetch(baseURL)).rejects.toThrow(DownloadHTTPError);
     });
 
-    it('fetch with bad status code', async () => {
+    it('fetch with 500 status code', async () => {
       const fetcher = new DefaultFetcher();
-      await expect(fetcher.fetch(baseURL)).rejects.toThrow(DownloadHTTPError);
+      nock(baseURL).get('/internal-error').reply(500, 'error');
+
+      await expect(
+        fetcher.fetch(`${baseURL}/internal-error`)
+      ).rejects.toMatchObject({
+        statusCode: 500,
+      });
+    });
+
+    it('retries on 500 when retry is configured', async () => {
+      let attempts = 0;
+      nock(baseURL)
+        .get('/retry-500')
+        .times(3)
+        .reply(() => {
+          attempts += 1;
+          return [500, 'error'];
+        });
+
+      const fetcher = new DefaultFetcher({
+        retry: {
+          retries: 2,
+          minTimeout: 1,
+          maxTimeout: 1,
+          randomize: false,
+        },
+      });
+
+      await expect(fetcher.fetch(`${baseURL}/retry-500`)).rejects.toMatchObject(
+        {
+          statusCode: 500,
+        }
+      );
+      expect(attempts).toBe(3);
+    });
+
+    it('succeeds after retrying on 500', async () => {
+      const fetcher = new DefaultFetcher({
+        retry: {
+          retries: 1,
+          minTimeout: 1,
+          maxTimeout: 1,
+          randomize: false,
+        },
+      });
+      nock(baseURL)
+        .get('/suceed-after-retry')
+        .reply(500, 'error')
+        .get('/suceed-after-retry')
+        .reply(200, response);
+
+      const result = await fetcher.fetch(`${baseURL}/suceed-after-retry`);
+      const text = await new Response(result).text();
+      expect(text).toBe(response);
     });
   });
 });