fix: Fetch with retry not actually retrying (#59)

Fix fetchWithRetry not actually retrying.

Author: nsdeschenes

.changeset/fluffy-candles-switch.md

@@ -0,0 +1,8 @@
+---
+"@codecov/bundler-plugin-core": patch
+"@codecov/rollup-plugin": patch
+"@codecov/vite-plugin": patch
+"@codecov/webpack-plugin": patch
+---
+
+Fix retry when fetching by throwing custom error if response is not okay

packages/bundler-plugin-core/src/errors/BadResponseError.ts

@@ -0,0 +1,5 @@
+export class BadResponseError extends Error {
+  constructor(msg: string) {
+    super(msg);
+  }
+}

packages/bundler-plugin-core/src/utils/__tests__/fetchWithRetry.test.ts

@@ -24,6 +24,7 @@ interface SetupArgs {
   data?: object;
   retryCount?: number;
   sendError?: boolean;
+  failFetch?: boolean;
 }
 
 describe("fetchWithRetry", () => {
@@ -37,17 +38,23 @@ describe("fetchWithRetry", () => {
     status = 200,
     data = {},
     sendError = false,
+    failFetch = false,
     retryCount = 0,
   }: SetupArgs) {
     consoleSpy = jest.spyOn(console, "log").mockImplementation(() => null);
 
     server.use(
       http.all("http://localhost", ({}) => {
-        if (retryCount === 0 && !sendError) {
+        if (retryCount === 0 && !sendError && !failFetch) {
           return HttpResponse.json(data, { status });
         }
+
+        if (sendError && !failFetch) {
+          return HttpResponse.error();
+        }
+
         retryCount -= 1;
-        return HttpResponse.error();
+        return new HttpResponse("not found", { status: 404 });
       }),
     );
   }
@@ -89,6 +96,24 @@ describe("fetchWithRetry", () => {
   });
 
   describe("retry count exceeds limit", () => {
+    it("returns the response", async () => {
+      setup({
+        data: { url: "http://example.com" },
+        retryCount: 2,
+        failFetch: true,
+      });
+
+      const response = await fetchWithRetry({
+        url: "http://localhost",
+        requestData: {},
+        retryCount: 1,
+      });
+
+      expect(response.status).toBe(404);
+    });
+  });
+
+  describe("when the fetch throws an error", () => {
     it("throws an error", async () => {
       setup({
         data: { url: "http://example.com" },

packages/bundler-plugin-core/src/utils/__tests__/getPreSignedURL.test.ts

@@ -65,6 +65,7 @@ describe("getPreSignedURL", () => {
             serviceParams: {
               commit: "123",
             },
+            retryCount: 0,
           });
 
           expect(url).toEqual("http://example.com");
@@ -87,6 +88,7 @@ describe("getPreSignedURL", () => {
             serviceParams: {
               commit: "123",
             },
+            retryCount: 0,
           });
         } catch (e) {
           error = e;
@@ -116,6 +118,7 @@ describe("getPreSignedURL", () => {
             serviceParams: {
               commit: "123",
             },
+            retryCount: 0,
           });
         } catch (e) {
           error = e;
@@ -141,6 +144,7 @@ describe("getPreSignedURL", () => {
             serviceParams: {
               commit: "123",
             },
+            retryCount: 0,
           });
         } catch (e) {
           error = e;
@@ -176,6 +180,7 @@ describe("getPreSignedURL", () => {
             serviceParams: {
               commit: "123",
             },
+            retryCount: 0,
           });
         } catch (e) {
           error = e;
@@ -201,6 +206,7 @@ describe("getPreSignedURL", () => {
             serviceParams: {
               commit: "123",
             },
+            retryCount: 0,
           });
         } catch (e) {
           error = e;
@@ -236,6 +242,7 @@ describe("getPreSignedURL", () => {
             serviceParams: {
               commit: "123",
             },
+            retryCount: 0,
           });
         } catch (e) {
           error = e;

packages/bundler-plugin-core/src/utils/__tests__/uploadStats.test.ts

@@ -62,6 +62,7 @@ describe("uploadStats", () => {
       const data = await uploadStats({
         message: "cool-message",
         preSignedUrl: "http://localhost/upload/stats/",
+        retryCount: 0,
       });
 
       expect(data).toBeTruthy();
@@ -75,7 +76,11 @@ describe("uploadStats", () => {
 
         let error;
         try {
-          await uploadStats({ message: "cool-message", preSignedUrl: "" });
+          await uploadStats({
+            message: "cool-message",
+            preSignedUrl: "",
+            retryCount: 1,
+          });
         } catch (e) {
           error = e;
         }
@@ -93,6 +98,7 @@ describe("uploadStats", () => {
           await uploadStats({
             message: "cool-message",
             preSignedUrl: "http://localhost/upload/stats/",
+            retryCount: 0,
           });
         } catch (e) {
           error = e;
@@ -111,6 +117,7 @@ describe("uploadStats", () => {
           await uploadStats({
             message: "cool-message",
             preSignedUrl: "http://localhost/upload/stats/",
+            retryCount: 0,
           });
         } catch (e) {
           error = e;

packages/bundler-plugin-core/src/utils/fetchWithRetry.ts

@@ -1,3 +1,4 @@
+import { BadResponseError } from "../errors/BadResponseError";
 import { DEFAULT_RETRY_DELAY } from "./constants";
 import { delay } from "./delay";
 import { debug, red } from "./logging";
@@ -22,13 +23,22 @@ export const fetchWithRetry = async ({
       debug(`Attempting to fetch ${name}, attempt: ${i}`);
       await delay(DEFAULT_RETRY_DELAY * i);
       response = await fetch(url, requestData);
-      break;
+
+      if (!response.ok) {
+        throw new BadResponseError("Response not ok");
+      }
     } catch (err) {
       debug(`${name} fetch attempt ${i} failed`);
       const isLastAttempt = i + 1 === retryCount;
+
       if (isLastAttempt) {
         red(`${name} failed after ${i} attempts`);
-        throw err;
+
+        if (!(err instanceof BadResponseError)) {
+          console.debug("ahh");
+          throw err;
+        }
+        return response;
       }
     }
   }