From 3df52c0f2e4a2bcfaa07135fdb6dad7a8ec73242 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 30 Sep 2025 21:59:36 +0100 Subject: [PATCH 1/2] retry subdomain requests to be more resilient to flakes --- .changeset/tangy-donuts-jog.md | 5 + .../wrangler/src/__tests__/deploy.test.ts | 21 ++++ .../helpers/mock-workers-subdomain.ts | 26 ++++- .../src/__tests__/utils/retry.test.ts | 103 ++++++++---------- .../versions/versions.upload.test.ts | 4 +- packages/wrangler/src/cfetch/index.ts | 41 +++---- packages/wrangler/src/cfetch/internal.ts | 16 ++- packages/wrangler/src/triggers/deploy.ts | 26 +++-- packages/wrangler/src/utils/retry.ts | 2 +- 9 files changed, 141 insertions(+), 103 deletions(-) create mode 100644 .changeset/tangy-donuts-jog.md diff --git a/.changeset/tangy-donuts-jog.md b/.changeset/tangy-donuts-jog.md new file mode 100644 index 00000000000..30688063b4d --- /dev/null +++ b/.changeset/tangy-donuts-jog.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +retry subdomain requests to be more resilient to flakes diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index 13b558172b0..9fa9ef4a4e9 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/packages/wrangler/src/__tests__/deploy.test.ts @@ -6081,6 +6081,27 @@ addEventListener('fetch', event => {});` expect(std.err).toMatchInlineSnapshot(`""`); }); + it("should deploy successfully if the /subdomain POST request is flaky", async () => { + writeWranglerConfig(); + writeWorkerSource(); + mockUploadWorkerRequest(); + mockGetWorkerSubdomain({ enabled: false }); + mockSubDomainRequest(); + mockUpdateWorkerSubdomain({ enabled: true, flakes: 1 }); + + await runWrangler("deploy ./index"); + + expect(std.out).toMatchInlineSnapshot(` + "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: 100 ms + Uploaded test-name (TIMINGS) + Deployed test-name triggers (TIMINGS) + https://test-name.test-sub-domain.workers.dev + Current Version ID: Galaxy-Class" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + }); + it("should deploy to the workers.dev domain if workers_dev is `true`", async () => { writeWranglerConfig({ workers_dev: true, diff --git a/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts b/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts index 06d7b157b73..d8f7a4f67cb 100644 --- a/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts +++ b/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts @@ -79,18 +79,21 @@ export function mockUpdateWorkerSubdomain({ env, legacyEnv = false, expectedScriptName = "test-name", + flakes = 0, }: { enabled: boolean; previews_enabled?: boolean; env?: string | undefined; legacyEnv?: boolean | undefined; expectedScriptName?: string; + flakes?: number; }) { const url = env && !legacyEnv ? `*/accounts/:accountId/workers/services/:scriptName/environments/:envName/subdomain` : `*/accounts/:accountId/workers/scripts/:scriptName/subdomain`; - msw.use( + + const handlers = [ http.post( url, async ({ request, params }) => { @@ -108,6 +111,23 @@ export function mockUpdateWorkerSubdomain({ ); }, { once: true } - ) - ); + ), + ]; + while (flakes > 0) { + flakes--; + handlers.unshift( + http.post( + url, + () => + HttpResponse.json( + createFetchResult(null, false, [ + { code: 10013, message: "An unknown error has occurred." }, + ]), + { status: 500 } + ), + { once: true } + ) + ); + } + msw.use(...handlers); } diff --git a/packages/wrangler/src/__tests__/utils/retry.test.ts b/packages/wrangler/src/__tests__/utils/retry.test.ts index 5881e8010a7..bfb63cffad0 100644 --- a/packages/wrangler/src/__tests__/utils/retry.test.ts +++ b/packages/wrangler/src/__tests__/utils/retry.test.ts @@ -1,3 +1,4 @@ +import { logger } from "../../logger"; import { APIError } from "../../parse"; import { retryOnAPIFailure } from "../../utils/retry"; import { mockConsoleMethods } from "../helpers/mock-console"; @@ -5,6 +6,12 @@ import { mockConsoleMethods } from "../helpers/mock-console"; describe("retryOnAPIFailure", () => { const std = mockConsoleMethods(); + beforeEach(() => { + const level = logger.loggerLevel; + logger.loggerLevel = "debug"; + return () => (logger.loggerLevel = level); + }); + it("should retry 5xx errors and succeed if the 3rd try succeeds", async () => { let attempts = 0; @@ -15,15 +22,13 @@ describe("retryOnAPIFailure", () => { } }); expect(attempts).toBe(3); - expect(std).toMatchInlineSnapshot(` - Object { - "debug": "", - "err": "", - "info": "Retrying API call after error... - Retrying API call after error...", - "out": "", - "warn": "", - } + expect(getRetryAndErrorLogs(std.debug)).toMatchInlineSnapshot(` + Array [ + "Retrying API call after error...", + "APIError: 500 error", + "Retrying API call after error...", + "APIError: 500 error", + ] `); }); @@ -37,16 +42,15 @@ describe("retryOnAPIFailure", () => { }) ).rejects.toMatchInlineSnapshot(`[APIError: 500 error]`); expect(attempts).toBe(3); - expect(std).toMatchInlineSnapshot(` - Object { - "debug": "", - "err": "", - "info": "Retrying API call after error... - Retrying API call after error... - Retrying API call after error...", - "out": "", - "warn": "", - } + expect(getRetryAndErrorLogs(std.debug)).toMatchInlineSnapshot(` + Array [ + "Retrying API call after error...", + "APIError: 500 error", + "Retrying API call after error...", + "APIError: 500 error", + "Retrying API call after error...", + "APIError: 500 error", + ] `); }); @@ -60,15 +64,7 @@ describe("retryOnAPIFailure", () => { }) ).rejects.toMatchInlineSnapshot(`[APIError: 401 error]`); expect(attempts).toBe(1); - expect(std).toMatchInlineSnapshot(` - Object { - "debug": "", - "err": "", - "info": "", - "out": "", - "warn": "", - } - `); + expect(getRetryAndErrorLogs(std.debug)).toMatchInlineSnapshot(`Array []`); }); it("should retry TypeError", async () => { @@ -81,16 +77,12 @@ describe("retryOnAPIFailure", () => { }) ).rejects.toMatchInlineSnapshot(`[TypeError: type error]`); expect(attempts).toBe(3); - expect(std).toMatchInlineSnapshot(` - Object { - "debug": "", - "err": "", - "info": "Retrying API call after error... - Retrying API call after error... - Retrying API call after error...", - "out": "", - "warn": "", - } + expect(getRetryAndErrorLogs(std.debug)).toMatchInlineSnapshot(` + Array [ + "Retrying API call after error...", + "Retrying API call after error...", + "Retrying API call after error...", + ] `); }); @@ -104,15 +96,7 @@ describe("retryOnAPIFailure", () => { }) ).rejects.toMatchInlineSnapshot(`[Error: some error]`); expect(attempts).toBe(1); - expect(std).toMatchInlineSnapshot(` - Object { - "debug": "", - "err": "", - "info": "", - "out": "", - "warn": "", - } - `); + expect(getRetryAndErrorLogs(std.debug)).toMatchInlineSnapshot(`Array []`); }); it("should retry custom APIError implementation with non-5xx error", async () => { @@ -134,16 +118,21 @@ describe("retryOnAPIFailure", () => { ).rejects.toMatchInlineSnapshot(`[CustomAPIError: 401 error]`); expect(attempts).toBe(3); expect(checkedCustomIsRetryable).toBe(true); - expect(std).toMatchInlineSnapshot(` - Object { - "debug": "", - "err": "", - "info": "Retrying API call after error... - Retrying API call after error... - Retrying API call after error...", - "out": "", - "warn": "", - } + expect(getRetryAndErrorLogs(std.debug)).toMatchInlineSnapshot(` + Array [ + "Retrying API call after error...", + "CustomAPIError: 401 error", + "Retrying API call after error...", + "CustomAPIError: 401 error", + "Retrying API call after error...", + "CustomAPIError: 401 error", + ] `); }); }); + +function getRetryAndErrorLogs(debugOutput: string): string[] { + return debugOutput + .split("\n") + .filter((line) => line.includes("Retrying") || line.includes("APIError")); +} diff --git a/packages/wrangler/src/__tests__/versions/versions.upload.test.ts b/packages/wrangler/src/__tests__/versions/versions.upload.test.ts index 0c4109d5baa..afacc6987f5 100644 --- a/packages/wrangler/src/__tests__/versions/versions.upload.test.ts +++ b/packages/wrangler/src/__tests__/versions/versions.upload.test.ts @@ -250,7 +250,7 @@ describe("versions upload", () => { writeWorkerSource(); setIsTTY(false); - const result = runWrangler("versions upload"); + const result = runWrangler("versions upload", { WRANGLER_LOG: "debug" }); await expect(result).resolves.toBeUndefined(); @@ -265,7 +265,7 @@ describe("versions upload", () => { Worker Version ID: 51e4886e-2db7-4900-8d38-fbfecfeab993" `); - expect(std.info).toContain("Retrying API call after error..."); + expect(std.debug).toContain("Retrying API call after error..."); }); test("correctly detects python workers", async () => { diff --git a/packages/wrangler/src/cfetch/index.ts b/packages/wrangler/src/cfetch/index.ts index 09b3634a562..e11d5de9ded 100644 --- a/packages/wrangler/src/cfetch/index.ts +++ b/packages/wrangler/src/cfetch/index.ts @@ -31,18 +31,13 @@ export async function fetchResult( abortSignal?: AbortSignal, apiToken?: ApiCredentials ): Promise { - const json = await fetchInternal>( - complianceConfig, - resource, - init, - queryParams, - abortSignal, - apiToken - ); + const { response: json, status } = await fetchInternal< + FetchResult + >(complianceConfig, resource, init, queryParams, abortSignal, apiToken); if (json.success) { return json.result; } else { - throwFetchError(resource, json); + throwFetchError(resource, json, status); } } @@ -54,7 +49,7 @@ export async function fetchGraphqlResult( init: RequestInit = {}, abortSignal?: AbortSignal ): Promise { - const json = await fetchInternal( + const { response: json } = await fetchInternal( complianceConfig, "/graphql", { ...init, method: "POST" }, //Cloudflare API v4 doesn't allow GETs to /graphql @@ -87,12 +82,9 @@ export async function fetchListResult( queryParams = new URLSearchParams(queryParams); queryParams.set("cursor", cursor); } - const json = await fetchInternal>( - complianceConfig, - resource, - init, - queryParams - ); + const { response: json, status } = await fetchInternal< + FetchResult + >(complianceConfig, resource, init, queryParams); if (json.success) { results.push(...json.result); if (hasCursor(json.result_info)) { @@ -101,7 +93,7 @@ export async function fetchListResult( getMoreResults = false; } } else { - throwFetchError(resource, json); + throwFetchError(resource, json, status); } } return results; @@ -127,12 +119,9 @@ export async function fetchPagedListResult( queryParams = new URLSearchParams(queryParams); queryParams.set("page", String(page)); - const json = await fetchInternal>( - complianceConfig, - resource, - init, - queryParams - ); + const { response: json, status } = await fetchInternal< + FetchResult + >(complianceConfig, resource, init, queryParams); if (json.success) { results.push(...json.result); if (hasMorePages(json.result_info)) { @@ -141,7 +130,7 @@ export async function fetchPagedListResult( getMoreResults = false; } } else { - throwFetchError(resource, json); + throwFetchError(resource, json, status); } } return results; @@ -171,7 +160,8 @@ export function hasMorePages( function throwFetchError( resource: string, - response: FetchResult + response: FetchResult, + status: number ): never { // This is an error from within an MSW handler if (typeof vitest !== "undefined" && !("errors" in response)) { @@ -187,6 +177,7 @@ function throwFetchError( ...response.errors.map((err) => ({ text: renderError(err) })), ...(response.messages?.map((text) => ({ text })) ?? []), ], + status, }); // add the first error code directly to this error // so consumers can use it for specific behaviour diff --git a/packages/wrangler/src/cfetch/internal.ts b/packages/wrangler/src/cfetch/internal.ts index 33270433570..acd948bd0dc 100644 --- a/packages/wrangler/src/cfetch/internal.ts +++ b/packages/wrangler/src/cfetch/internal.ts @@ -158,7 +158,7 @@ export async function fetchInternal( queryParams?: URLSearchParams, abortSignal?: AbortSignal, apiToken?: ApiCredentials -): Promise { +): Promise<{ response: ResponseType; status: number }> { const method = init.method ?? "GET"; const response = await performApiFetch( complianceConfig, @@ -181,12 +181,20 @@ export async function fetchInternal( // HTTP 204 and HTTP 205 responses do not return a body. We need to special-case this // as otherwise parseJSON will throw an error back to the user. if (!jsonText && (response.status === 204 || response.status === 205)) { - const emptyBody = `{"result": {}, "success": true, "errors": [], "messages": []}`; - return parseJSON(emptyBody) as ResponseType; + return { + response: { + result: {}, + success: true, + errors: [], + messages: [], + } as ResponseType, + status: response.status, + }; } try { - return parseJSON(jsonText) as ResponseType; + const json = parseJSON(jsonText) as ResponseType; + return { response: json, status: response.status }; } catch { throw new APIError({ text: "Received a malformed response from the API", diff --git a/packages/wrangler/src/triggers/deploy.ts b/packages/wrangler/src/triggers/deploy.ts index 4c9304537cb..de43317521e 100644 --- a/packages/wrangler/src/triggers/deploy.ts +++ b/packages/wrangler/src/triggers/deploy.ts @@ -379,17 +379,21 @@ async function subdomainDeploy( // Update subdomain enablement status if needed. if (!allInSync) { - await fetchResult(config, `${workerUrl}/subdomain`, { - method: "POST", - body: JSON.stringify({ - enabled: wantWorkersDev, - previews_enabled: wantPreviews, - }), - headers: { - "Content-Type": "application/json", - "Cloudflare-Workers-Script-Api-Date": "2025-08-01", - }, - }); + // Occasionally this update to the subdomain endpoint fails due to some internal API error. + // We retry this request a few times to mitigate that. + await retryOnAPIFailure(async () => + fetchResult(config, `${workerUrl}/subdomain`, { + method: "POST", + body: JSON.stringify({ + enabled: wantWorkersDev, + previews_enabled: wantPreviews, + }), + headers: { + "Content-Type": "application/json", + "Cloudflare-Workers-Script-Api-Date": "2025-08-01", + }, + }) + ); } if (workersDevURL) { diff --git a/packages/wrangler/src/utils/retry.ts b/packages/wrangler/src/utils/retry.ts index aa3ad65a579..9ace9c5c60e 100644 --- a/packages/wrangler/src/utils/retry.ts +++ b/packages/wrangler/src/utils/retry.ts @@ -29,7 +29,7 @@ export async function retryOnAPIFailure( throw err; } - logger.info(chalk.dim(`Retrying API call after error...`)); + logger.debug(chalk.dim(`Retrying API call after error...`)); logger.debug(err); if (attempts <= 1) { From c95a0d7146563d2aeb72a29e84de4550175bf8a1 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Wed, 1 Oct 2025 10:32:06 +0100 Subject: [PATCH 2/2] fixup! retry subdomain requests to be more resilient to flakes --- packages/wrangler/src/__tests__/deploy.test.ts | 2 +- .../src/__tests__/helpers/mock-workers-subdomain.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index 9fa9ef4a4e9..dabd204d1f9 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/packages/wrangler/src/__tests__/deploy.test.ts @@ -6087,7 +6087,7 @@ addEventListener('fetch', event => {});` mockUploadWorkerRequest(); mockGetWorkerSubdomain({ enabled: false }); mockSubDomainRequest(); - mockUpdateWorkerSubdomain({ enabled: true, flakes: 1 }); + mockUpdateWorkerSubdomain({ enabled: true, flakeCount: 1 }); await runWrangler("deploy ./index"); diff --git a/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts b/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts index d8f7a4f67cb..f05f934f38d 100644 --- a/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts +++ b/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts @@ -79,14 +79,14 @@ export function mockUpdateWorkerSubdomain({ env, legacyEnv = false, expectedScriptName = "test-name", - flakes = 0, + flakeCount = 0, }: { enabled: boolean; previews_enabled?: boolean; env?: string | undefined; legacyEnv?: boolean | undefined; expectedScriptName?: string; - flakes?: number; + flakeCount?: number; // The first `flakeCount` requests will fail with a 500 error }) { const url = env && !legacyEnv @@ -113,8 +113,8 @@ export function mockUpdateWorkerSubdomain({ { once: true } ), ]; - while (flakes > 0) { - flakes--; + while (flakeCount > 0) { + flakeCount--; handlers.unshift( http.post( url,