feat: Add more observability to getPreSignedUrl errors and prevent retries f…

integration-tests/fixtures/generate-bundle-stats/astro/astro-base.config.mjs

@@ -16,8 +16,8 @@ export default defineConfig({
     rollupOptions: {
       output: {
         format: "esm",
-      }
-    }
+      },
+    },
   },
   integrations: [
     react(),

integration-tests/test-api/tsconfig.json

@@ -2,6 +2,6 @@
   "compilerOptions": {
     "strict": true,
     "jsx": "react-jsx",
-    "jsxImportSource": "hono/jsx"
-  }
+    "jsxImportSource": "hono/jsx",
+  },
 }

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

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

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

@@ -1,4 +1,4 @@
-import { type Client, type Scope, startSpan } from "@sentry/core";
+import { type Client, type Scope, startSpan, type Span } from "@sentry/core";
 import {
   type Asset,
   type Chunk,
@@ -8,6 +8,7 @@ import {
   type ProviderUtilInputs,
   type UploadOverrides,
 } from "../types.ts";
+import { AuthenticationError } from "../errors/AuthenticationError.ts";
 import { getPreSignedURL } from "./getPreSignedURL.ts";
 import { type NormalizedOptions } from "./normalizeOptions.ts";
 import { detectProvider } from "./provider.ts";
@@ -249,7 +250,7 @@ class Output {
               scope: this.sentryScope,
               parentSpan: outputWriteSpan,
             },
-            async (getPreSignedURLSpan) => {
+            async (getPreSignedURLSpan: Span | undefined) => {
               let url = "";
               try {
                 url = await getPreSignedURL({
@@ -264,19 +265,23 @@ class Output {
                 });
               } catch (error) {
                 if (this.sentryClient && this.sentryScope) {
-                  this.sentryScope.addBreadcrumb({
-                    category: "output.write.getPreSignedURL",
-                    level: "error",
-                    data: { error },
-                  });
-                  // only setting this as info because this could be caused by user error
-                  this.sentryClient.captureMessage(
-                    "Error in getPreSignedURL",
-                    "info",
-                    undefined,
-                    this.sentryScope,
-                  );
-                  await safeFlushTelemetry(this.sentryClient);
+                  // if the user gets an authentication error, something is wrong with their setup
+                  // so we don't want to log it to sentry
+                  if (!(error instanceof AuthenticationError)) {
+                    this.sentryScope.addBreadcrumb({
+                      category: "output.write.getPreSignedURL",
+                      level: "error",
+                      data: { error },
+                    });
+                    // only setting this as info because this could be caused by user error
+                    this.sentryClient.captureMessage(
+                      "Error in getPreSignedURL",
+                      "info",
+                      undefined,
+                      this.sentryScope,
+                    );
+                    await safeFlushTelemetry(this.sentryClient);
+                  }
                 }
 
                 if (emitError) {

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

@@ -35,6 +35,7 @@ interface SetupArgs {
   retryCount?: number;
   sendError?: boolean;
   failFetch?: boolean;
+  retryFailureStatus?: number;
 }
 
 describe("fetchWithRetry", () => {
@@ -50,6 +51,7 @@ describe("fetchWithRetry", () => {
     sendError = false,
     failFetch = false,
     retryCount = 0,
+    retryFailureStatus = 503,
   }: SetupArgs) {
     consoleSpy = vi.spyOn(console, "log").mockImplementation(() => null);
 
@@ -64,7 +66,9 @@ describe("fetchWithRetry", () => {
         }
 
         retryCount -= 1;
-        return new HttpResponse("not found", { status: 404 });
+        return new HttpResponse("service unavailable", {
+          status: retryFailureStatus,
+        });
       }),
     );
   }
@@ -92,6 +96,7 @@ describe("fetchWithRetry", () => {
       setup({
         data: { url: "http://example.com" },
         retryCount: 2,
+        retryFailureStatus: 503,
       });
 
       const urlPromise = await fetchWithRetry({
@@ -105,12 +110,43 @@ describe("fetchWithRetry", () => {
     });
   });
 
+  describe("when the initial response is a 4xx error", () => {
+    it("returns the response without retrying", async () => {
+      let requestCount = 0;
+      consoleSpy = vi.spyOn(console, "log").mockImplementation(() => null);
+
+      server.use(
+        http.all("http://localhost", () => {
+          requestCount += 1;
+          return HttpResponse.json(
+            { message: "Bad Request", detail: "example error body" },
+            { status: 400 },
+          );
+        }),
+      );
+
+      const response = await fetchWithRetry({
+        url: "http://localhost",
+        requestData: {},
+        retryCount: 5,
+      });
+
+      expect(requestCount).toBe(1);
+      expect(response.status).toBe(400);
+      await expect(response.json()).resolves.toEqual({
+        message: "Bad Request",
+        detail: "example error body",
+      });
+    });
+  });
+
   describe("retry count exceeds limit", () => {
     it("returns the response", async () => {
       setup({
         data: { url: "http://example.com" },
         retryCount: 2,
         failFetch: true,
+        retryFailureStatus: 503,
       });
 
       const response = await fetchWithRetry({
@@ -119,7 +155,7 @@ describe("fetchWithRetry", () => {
         retryCount: 1,
       });
 
-      expect(response.status).toBe(404);
+      expect(response.status).toBe(503);
     });
   });
 

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

@@ -302,30 +302,85 @@ describe("getPreSignedURL", () => {
     });
 
     describe('http response is not "ok"', () => {
-      it("throws an error", async () => {
-        const { consoleSpy } = setup({
-          data: { url: "http://example.com" },
-          status: 400,
+      describe("4xx error", () => {
+        it("throws an error", async () => {
+          let requestCount = 0;
+          const consoleSpy = vi
+            .spyOn(console, "log")
+            .mockImplementation(() => null);
+
+          server.use(
+            http.post(
+              "http://localhost/upload/bundle_analysis/v1",
+              async ({ request }) => {
+                requestCount += 1;
+                const requestBody = await request.json();
+                requestBodyMock(requestBody);
+
+                if (request.headers.get("Authorization")) {
+                  requestTokenMock(request.headers.get("Authorization"));
+                }
+
+                return HttpResponse.json(
+                  { detail: "Bad Request" },
+                  { status: 400 },
+                );
+              },
+            ),
+          );
+
+          let error;
+          try {
+            await getPreSignedURL({
+              apiUrl: "http://localhost",
+              uploadToken: "cool-upload-token",
+              retryCount: 3,
+              serviceParams: {
+                commit: "123",
+              },
+            });
+          } catch (e) {
+            error = e;
+          }
+
+          expect(requestCount).toBe(1);
+          expect(error).toBeInstanceOf(FailedFetchError);
+          expect(consoleSpy).toHaveBeenCalledWith(
+            `[codecov] ${Chalk.red(
+              "Failed to get pre-signed URL, bad response: Bad Request",
+            )}`,
+          );
         });
+      });
 
-        let error;
-        try {
-          await getPreSignedURL({
-            apiUrl: "http://localhost",
-            uploadToken: "cool-upload-token",
-            retryCount: 0,
-            serviceParams: {
-              commit: "123",
-            },
+      describe("non-4xx error", () => {
+        it("throws an error", async () => {
+          const { consoleSpy } = setup({
+            data: { url: "http://example.com" },
+            status: 503,
           });
-        } catch (e) {
-          error = e;
-        }
 
-        expect(error).toBeInstanceOf(FailedFetchError);
-        expect(consoleSpy).toHaveBeenCalledWith(
-          `[codecov] ${Chalk.red('Failed to get pre-signed URL, bad response: "400 - Bad Request"')}`,
-        );
+          let error;
+          try {
+            await getPreSignedURL({
+              apiUrl: "http://localhost",
+              uploadToken: "cool-upload-token",
+              retryCount: 0,
+              serviceParams: {
+                commit: "123",
+              },
+            });
+          } catch (e) {
+            error = e;
+          }
+
+          expect(error).toBeInstanceOf(FailedFetchError);
+          expect(consoleSpy).toHaveBeenCalledWith(
+            `[codecov] ${Chalk.red(
+              "Failed to get pre-signed URL (server), bad response: 503 - Service Unavailable",
+            )}`,
+          );
+        });
       });
     });
 

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

@@ -26,6 +26,13 @@ export const fetchWithRetry = async ({
       response = await fetch(url, requestData);
 
       if (!response.ok) {
+        // 4xx errors are permanent client errors - no need to retry
+        if (response.status >= 400 && response.status < 500) {
+          debug(
+            `${name} received ${response.status} client error, not retrying`,
+          );
+          return response;
+        }
         throw new BadResponseError("Response not ok");
       }
       break;

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

@@ -7,6 +7,7 @@ import {
   type Span,
 } from "@sentry/core";
 import { z } from "zod";
+import { AuthenticationError } from "../errors/AuthenticationError.ts";
 import { FailedFetchError } from "../errors/FailedFetchError.ts";
 import { UploadLimitReachedError } from "../errors/UploadLimitReachedError.ts";
 import { type ProviderServiceParams } from "../types.ts";
@@ -39,6 +40,10 @@ const PreSignedURLSchema = z.object({
   url: z.string(),
 });
 
+const ApiErrorBodySchema = z.object({
+  detail: z.string().optional(),
+});
+
 const API_ENDPOINT = "/upload/bundle_analysis/v1";
 
 export const getPreSignedURL = async ({
@@ -119,7 +124,7 @@ export const getPreSignedURL = async ({
         scope: sentryScope,
         parentSpan: sentrySpan,
       },
-      async (getPreSignedURLSpan) => {
+      async (getPreSignedURLSpan: Span | undefined) => {
         let wrappedResponse: Response;
         const HTTP_METHOD = "POST";
         const URL = `${apiUrl}${API_ENDPOINT}`;
@@ -182,11 +187,40 @@ export const getPreSignedURL = async ({
     throw new UploadLimitReachedError("Upload limit reached");
   }
 
-  if (!response.ok) {
-    red(
-      `Failed to get pre-signed URL, bad response: "${response.status} - ${response.statusText}"`,
+  if (response.status >= 400 && response.status < 500) {
+    // Attempt to parse a server-provided error message to give users actionable feedback
+    let serverMessage = "";
+    try {
+      const raw: unknown = await response.clone().json();
+      const parsed = ApiErrorBodySchema.safeParse(raw);
+      serverMessage = parsed.success ? parsed.data.detail ?? "" : "";
+    } catch {
+      // ignore parse failures
+    }
+    const responseStatusWithText = `${response.status} - ${response.statusText}`;
+    const msgDetail = serverMessage || responseStatusWithText;
+    red(`Failed to get pre-signed URL, bad response: ${msgDetail}`);
+    if (
+      serverMessage.toLowerCase().includes("token") ||
+      response.status === 401 ||
+      response.status === 403
+    ) {
+      red(
+        "Set uploadToken in your Codecov plugin config or enable tokenless uploads for your public repository.",
+      );
+      throw new AuthenticationError(
+        serverMessage || `Authentication failed: ${responseStatusWithText}`,
+      );
+    }
+    throw new FailedFetchError(
+      `Failed to get pre-signed URL (client), bad response: ${responseStatusWithText}`,
     );
-    throw new FailedFetchError("Failed to get pre-signed URL");
+  }
+
+  if (!response.ok) {
+    const msg = `Failed to get pre-signed URL (server), bad response: ${response.status} - ${response.statusText}`;
+    red(msg);
+    throw new FailedFetchError(msg);
   }
 
   let data;