diff --git a/integration-tests/fixtures/generate-bundle-stats/astro/astro-base.config.mjs b/integration-tests/fixtures/generate-bundle-stats/astro/astro-base.config.mjs index d9e32b23..39164f25 100644 --- a/integration-tests/fixtures/generate-bundle-stats/astro/astro-base.config.mjs +++ b/integration-tests/fixtures/generate-bundle-stats/astro/astro-base.config.mjs @@ -16,8 +16,8 @@ export default defineConfig({ rollupOptions: { output: { format: "esm", - } - } + }, + }, }, integrations: [ react(), diff --git a/integration-tests/test-api/tsconfig.json b/integration-tests/test-api/tsconfig.json index 9136c143..c829ef88 100644 --- a/integration-tests/test-api/tsconfig.json +++ b/integration-tests/test-api/tsconfig.json @@ -2,6 +2,6 @@ "compilerOptions": { "strict": true, "jsx": "react-jsx", - "jsxImportSource": "hono/jsx" - } + "jsxImportSource": "hono/jsx", + }, } diff --git a/packages/bundler-plugin-core/src/errors/AuthenticationError.ts b/packages/bundler-plugin-core/src/errors/AuthenticationError.ts new file mode 100644 index 00000000..0f9b8a2c --- /dev/null +++ b/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); + } +} diff --git a/packages/bundler-plugin-core/src/utils/Output.ts b/packages/bundler-plugin-core/src/utils/Output.ts index 0bc53d78..e8bce793 100644 --- a/packages/bundler-plugin-core/src/utils/Output.ts +++ b/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) { diff --git a/packages/bundler-plugin-core/src/utils/__tests__/fetchWithRetry.test.ts b/packages/bundler-plugin-core/src/utils/__tests__/fetchWithRetry.test.ts index ef1f28a9..72da5b02 100644 --- a/packages/bundler-plugin-core/src/utils/__tests__/fetchWithRetry.test.ts +++ b/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); }); }); diff --git a/packages/bundler-plugin-core/src/utils/__tests__/getPreSignedURL.test.ts b/packages/bundler-plugin-core/src/utils/__tests__/getPreSignedURL.test.ts index 5fcc49f1..cc272dfb 100644 --- a/packages/bundler-plugin-core/src/utils/__tests__/getPreSignedURL.test.ts +++ b/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", + )}`, + ); + }); }); }); diff --git a/packages/bundler-plugin-core/src/utils/fetchWithRetry.ts b/packages/bundler-plugin-core/src/utils/fetchWithRetry.ts index 6ae81f19..1c874d49 100644 --- a/packages/bundler-plugin-core/src/utils/fetchWithRetry.ts +++ b/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; diff --git a/packages/bundler-plugin-core/src/utils/getPreSignedURL.ts b/packages/bundler-plugin-core/src/utils/getPreSignedURL.ts index 4c74228e..45b3e731 100644 --- a/packages/bundler-plugin-core/src/utils/getPreSignedURL.ts +++ b/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;