diff --git a/.changeset/calm-pens-reply.md b/.changeset/calm-pens-reply.md new file mode 100644 index 0000000000..768acfd59f --- /dev/null +++ b/.changeset/calm-pens-reply.md @@ -0,0 +1,7 @@ +--- +"wrangler": patch +--- + +fix: Add retry and timeout protection to remote preview API calls + +Remote preview sessions (`wrangler dev --remote`) now automatically retry transient 5xx API errors (up to 3 attempts with linear backoff) and enforce a 30-second per-request timeout. Previously, a single hung or failed API response during session creation or worker upload could block the dev session reload indefinitely. diff --git a/fixtures/get-platform-proxy-remote-bindings/tests/index.test.ts b/fixtures/get-platform-proxy-remote-bindings/tests/index.test.ts index dc34395e99..cc5b7e0ab5 100644 --- a/fixtures/get-platform-proxy-remote-bindings/tests/index.test.ts +++ b/fixtures/get-platform-proxy-remote-bindings/tests/index.test.ts @@ -19,6 +19,9 @@ import type { DispatchFetch, Response } from "miniflare"; type Fetcher = { fetch: DispatchFetch }; +const workersDomain = + process.env.E2E_ACCOUNT_WORKERS_DEV_DOMAIN ?? + "devprod-testing7928.workers.dev"; const auth = getAuthenticatedEnv(); const execOptions = { encoding: "utf8", @@ -35,8 +38,7 @@ if (auth) { let remoteKvId: string; beforeAll(async () => { - const deployedUrl = - "https://preserve-e2e-get-platform-proxy-remote.devprod-testing7928.workers.dev/"; + const deployedUrl = `https://preserve-e2e-get-platform-proxy-remote.${workersDomain}/`; try { assert((await fetch(deployedUrl)).status !== 404); @@ -54,8 +56,7 @@ if (auth) { ); } - const stagingDeployedUrl = - "https://preserve-e2e-get-platform-proxy-remote-staging.devprod-testing7928.workers.dev/"; + const stagingDeployedUrl = `https://preserve-e2e-get-platform-proxy-remote-staging.${workersDomain}/`; try { assert((await fetch(stagingDeployedUrl)).status !== 404); } catch { diff --git a/packages/create-cloudflare/e2e/tests/cli/cli.test.ts b/packages/create-cloudflare/e2e/tests/cli/cli.test.ts index 2ecd2136ca..ffe9e34086 100644 --- a/packages/create-cloudflare/e2e/tests/cli/cli.test.ts +++ b/packages/create-cloudflare/e2e/tests/cli/cli.test.ts @@ -15,6 +15,9 @@ import { recreateLogFolder } from "../../helpers/log-stream"; import { runC3 } from "../../helpers/run-c3"; const { name: pm } = detectPackageManager(); +const workersDomain = + process.env.E2E_ACCOUNT_WORKERS_DEV_DOMAIN ?? + "devprod-testing7928.workers.dev"; describe("Create Cloudflare CLI", () => { beforeAll((ctx) => { @@ -497,7 +500,7 @@ describe("Create Cloudflare CLI", () => { if ( ( await fetch( - "https://existing-script-test-do-not-delete.devprod-testing7928.workers.dev/" + `https://existing-script-test-do-not-delete.${workersDomain}/` ) ).status === 404 ) { diff --git a/packages/vite-plugin-cloudflare/e2e/helpers.ts b/packages/vite-plugin-cloudflare/e2e/helpers.ts index 9cf6ca9736..83c44ba423 100644 --- a/packages/vite-plugin-cloudflare/e2e/helpers.ts +++ b/packages/vite-plugin-cloudflare/e2e/helpers.ts @@ -307,7 +307,9 @@ export async function fetchJson(url: string, info?: RequestInit) { export async function waitForReady(proc: Process) { const match = await vi.waitUntil( () => proc.stdout.match(/Local:\s+(http:\/\/localhost:\d+)/), - { interval: 100, timeout: 20_000 } + // buildAndPreview does a full build before starting the server, + // so allow more time than the default. + { interval: 100, timeout: 30_000 } ); return match[1]; } diff --git a/packages/vite-plugin-cloudflare/e2e/remote-bindings.test.ts b/packages/vite-plugin-cloudflare/e2e/remote-bindings.test.ts index 1e29fde216..c6f88b7e8a 100644 --- a/packages/vite-plugin-cloudflare/e2e/remote-bindings.test.ts +++ b/packages/vite-plugin-cloudflare/e2e/remote-bindings.test.ts @@ -12,6 +12,9 @@ import { const commands = ["dev", "buildAndPreview"] as const; const isWindows = process.platform === "win32"; +const workersDomain = + process.env.E2E_ACCOUNT_WORKERS_DEV_DOMAIN ?? + "devprod-testing7928.workers.dev"; // Remote bindings tests are skipped on Windows due to slow/unreliable remote proxy // session initialization times in CI, which causes intermittent timeout failures. @@ -38,11 +41,8 @@ if (isWindows) { beforeAll(async () => { try { assert( - ( - await fetch( - "https://preserve-e2e-vite-remote.devprod-testing7928.workers.dev/" - ) - ).status !== 404 + (await fetch(`https://preserve-e2e-vite-remote.${workersDomain}/`)) + .status !== 404 ); } catch { runCommand(`npx wrangler deploy`, `${projectPath}/remote-worker`); @@ -51,7 +51,7 @@ if (isWindows) { assert( ( await fetch( - "https://preserve-e2e-vite-remote-alt.devprod-testing7928.workers.dev/" + `https://preserve-e2e-vite-remote-alt.${workersDomain}/` ) ).status !== 404 ); diff --git a/packages/wrangler/e2e/README.md b/packages/wrangler/e2e/README.md index 4debd017b6..2ab23a77a0 100644 --- a/packages/wrangler/e2e/README.md +++ b/packages/wrangler/e2e/README.md @@ -39,16 +39,33 @@ pnpm test:e2e:wrangler -- -u --bail=1 ### Cloudflare Credentials Cloudflare credentials are provided to the tests by setting `CLOUDFLARE_ACCOUNT_ID` and `CLOUDFLARE_API_TOKEN`. +If you don't provide these then only the local e2e tests are executed. -- If you don't provide these then only the local e2e tests are executed. -- If you don't provide the "DevProd Testing" Cloudflare account (as `CLOUDFLARE_ACCOUNT_ID=8d783f274e1f82dc46744c297b015a2f`), tests that require that specific account are not executed. +#### Running against the CI account -To fully run the tests you should generate an API token for the "DevProd Testing" account: +The default configuration targets the "DevProd Testing" account. To fully run the tests, generate an API token for that account: ```zsh CLOUDFLARE_ACCOUNT_ID=8d783f274e1f82dc46744c297b015a2f CLOUDFLARE_API_TOKEN= pnpm test:e2e:wrangler ``` +#### Running against your own account + +You can run the e2e tests against any Cloudflare account. Some tests rely on +pre-deployed `preserve-e2e-*` workers whose URL includes the account's +`workers.dev` subdomain. Set `E2E_ACCOUNT_WORKERS_DEV_DOMAIN` so these tests +can locate (and, on first run, deploy) those workers on your account: + +```zsh +CLOUDFLARE_ACCOUNT_ID= \ +CLOUDFLARE_API_TOKEN= \ +E2E_ACCOUNT_WORKERS_DEV_DOMAIN=.workers.dev \ +pnpm test:e2e:wrangler +``` + +> You can find your subdomain in the Cloudflare dashboard under **Workers & Pages**. +> It defaults to `devprod-testing7928.workers.dev` (the CI account). + ### Focusing on a single e2e test file If you want to run a subset of tests (e.g. just one) while retaining the turborepo cache for the builds of the dependencies, you can provide the list of test files via the `WRANGLER_E2E_TEST_FILE` environment variable. diff --git a/packages/wrangler/e2e/assets-multiworker.test.ts b/packages/wrangler/e2e/assets-multiworker.test.ts index e1560052fc..14f0dbd946 100644 --- a/packages/wrangler/e2e/assets-multiworker.test.ts +++ b/packages/wrangler/e2e/assets-multiworker.test.ts @@ -1,30 +1,12 @@ import dedent from "ts-dedent"; import { fetch } from "undici"; -import { beforeEach, describe, it, vi } from "vitest"; +import { beforeEach, describe, it } from "vitest"; import { WranglerE2ETestHelper } from "./helpers/e2e-wrangler-test"; +import { fetchJson } from "./helpers/fetch-json"; import { fetchText } from "./helpers/fetch-text"; import { generateResourceName } from "./helpers/generate-resource-name"; import { seed as baseSeed, makeRoot } from "./helpers/setup"; -import type { RequestInit } from "undici"; - -async function fetchJson(url: string, info?: RequestInit): Promise { - return vi.waitFor( - async () => { - const text: string = await fetch(url, { - headers: { "MF-Disable-Pretty-Error": "true" }, - ...info, - }).then((r) => r.text()); - try { - return JSON.parse(text) as T; - } catch (cause) { - const err = new Error(`Failed to parse JSON from:\n${text}`); - err.cause = cause; - throw err; - } - }, - { timeout: 5_000, interval: 250 } - ); -} +import { waitForLong } from "./helpers/wait-for"; async function startWorkersDevRegistry( wranglerDev: string, @@ -251,18 +233,14 @@ describe.each( false ); - await vi.waitFor( - async () => - await expect( - fetch(`${url}/hello-from-dee`).then((r) => r.text()) - ).resolves.toBe("hello world from dee"), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect( + fetch(`${url}/hello-from-dee`).then((r) => r.text()) + ).resolves.toBe("hello world from dee") ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/count`)).resolves.toBe("6"), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(`${url}/count`)).resolves.toBe("6") ); }); @@ -277,24 +255,18 @@ describe.each( false ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/do-rpc`)).resolves.toBe( - "Hello through DO RPC" - ), - { interval: 1000, timeout: 10_000 } - ); - await vi.waitFor( - async () => - await expect( - fetchJson(`${url}/do`, { - headers: { - "X-Reset-Count": "true", - }, - }) - ).resolves.toMatchObject({ count: 1 }), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(`${url}/do-rpc`)).resolves.toBe( + "Hello through DO RPC" + ) ); + await expect( + fetchJson(`${url}/do`, { + headers: { + "X-Reset-Count": "true", + }, + }) + ).resolves.toMatchObject({ count: 1 }); } ); }); @@ -339,12 +311,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/asset`)).resolves.toBe( - "

have an asset directly

" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/asset`)).resolves.toBe( + "

have an asset directly

" + ) ); }); @@ -355,12 +325,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetch(`${url}/not-an-asset`)).resolves.toMatchObject({ - status: 404, - }), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetch(`${url}/not-an-asset`)).resolves.toMatchObject({ + status: 404, + }) ); }); @@ -371,13 +339,11 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/rpc`)).resolves.toContain( - // Cannot call RPC methods on assets-only workers - "The RPC receiver does not implement the method" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/rpc`)).resolves.toContain( + // Cannot call RPC methods on assets-only workers + "The RPC receiver does not implement the method" + ) ); }); }); @@ -469,12 +435,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/asset`)).resolves.toBe( - "

have an asset directly

" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/asset`)).resolves.toBe( + "

have an asset directly

" + ) ); }); @@ -485,12 +449,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/not-an-asset`)).resolves.toBe( - "hello world from a worker with assets" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/not-an-asset`)).resolves.toBe( + "hello world from a worker with assets" + ) ); }); @@ -501,12 +463,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/asset-via-binding`)).resolves.toBe( - "

have an asset via a binding

" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/asset-via-binding`)).resolves.toBe( + "

have an asset via a binding

" + ) ); }); @@ -517,10 +477,8 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/rpc`)).resolves.toBe("2"), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/rpc`)).resolves.toBe("2") ); }); }); @@ -551,12 +509,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/asset`)).resolves.toBe( - "

have an asset directly

" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/asset`)).resolves.toBe( + "

have an asset directly

" + ) ); }); @@ -567,12 +523,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/not-an-asset`)).resolves.toBe( - "hello world from a worker with assets" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/not-an-asset`)).resolves.toBe( + "hello world from a worker with assets" + ) ); }); @@ -583,12 +537,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/asset-via-binding`)).resolves.toBe( - "

have an asset via a binding

" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/asset-via-binding`)).resolves.toBe( + "

have an asset via a binding

" + ) ); }); @@ -599,10 +551,8 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/rpc`)).resolves.toBe("2"), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/rpc`)).resolves.toBe("2") ); }); }); @@ -647,12 +597,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/asset`)).resolves.toBe( - "hello world from a worker with assets" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/asset`)).resolves.toBe( + "hello world from a worker with assets" + ) ); }); @@ -663,12 +611,10 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/asset-via-binding`)).resolves.toBe( - "

have an asset via a binding

" - ), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/asset-via-binding`)).resolves.toBe( + "

have an asset via a binding

" + ) ); }); @@ -679,10 +625,8 @@ describe.each( assetWorker, regularWorker ); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/rpc`)).resolves.toBe("2"), - { interval: 1000, timeout: 5_000 } + await waitForLong(() => + expect(fetchText(`${url}/rpc`)).resolves.toBe("2") ); }); }); diff --git a/packages/wrangler/e2e/containers.dev.test.ts b/packages/wrangler/e2e/containers.dev.test.ts index 53d8672927..c5fd0853ed 100644 --- a/packages/wrangler/e2e/containers.dev.test.ts +++ b/packages/wrangler/e2e/containers.dev.test.ts @@ -10,6 +10,7 @@ import { dedent } from "../src/utils/dedent"; import { CLOUDFLARE_ACCOUNT_ID } from "./helpers/account-id"; import { WranglerE2ETestHelper } from "./helpers/e2e-wrangler-test"; import { generateResourceName } from "./helpers/generate-resource-name"; +import { waitFor, waitForLong } from "./helpers/wait-for"; const imageSource = ["pull", "build"] as const; @@ -244,7 +245,7 @@ for (const source of imageSource) { const worker = helper.runLongLived("wrangler dev"); const ready = await worker.waitForReady(); - await vi.waitFor(async () => { + await waitFor(async () => { const response = await fetch(`${ready.url}/status`); expect(response.status).toBe(200); const status = await response.json(); @@ -256,24 +257,21 @@ for (const source of imageSource) { expect(response.status).toBe(200); expect(text).toBe("Container create request sent..."); - await vi.waitFor(async () => { + await waitFor(async () => { response = await fetch(`${ready.url}/status`); expect(response.status).toBe(200); const status = await response.json(); expect(status).toBe(true); }); - await vi.waitFor( - async () => { - response = await fetch(`${ready.url}/fetch`, { - signal: AbortSignal.timeout(3_000), - headers: { "MF-Disable-Pretty-Error": "true" }, - }); - text = await response.text(); - expect(text).toBe("Hello World! Have an env var! I'm an env var!"); - }, - { timeout: 5_000 } - ); + await waitFor(async () => { + response = await fetch(`${ready.url}/fetch`, { + signal: AbortSignal.timeout(3_000), + headers: { "MF-Disable-Pretty-Error": "true" }, + }); + text = await response.text(); + expect(text).toBe("Hello World! Have an env var! I'm an env var!"); + }); // Set up egress HTTP interception so the container can call back to the worker response = await fetch(`${ready.url}/setup-intercept`, { @@ -285,18 +283,15 @@ for (const source of imageSource) { expect(text).toBe("Intercept setup done"); // Fetch through the container's /intercept route which curls back to the worker - await vi.waitFor( - async () => { - response = await fetch(`${ready.url}/fetch-intercept`, { - signal: AbortSignal.timeout(5_000), - headers: { "MF-Disable-Pretty-Error": "true" }, - }); - text = await response.text(); - expect(response.status).toBe(200); - expect(text).toBe("hello from worker"); - }, - { timeout: 10_000 } - ); + await waitForLong(async () => { + response = await fetch(`${ready.url}/fetch-intercept`, { + signal: AbortSignal.timeout(5_000), + headers: { "MF-Disable-Pretty-Error": "true" }, + }); + text = await response.text(); + expect(response.status).toBe(200); + expect(text).toBe("hello from worker"); + }); // Check that a container is running using `docker ps` const ids = getContainerIds("e2econtainer"); @@ -334,7 +329,7 @@ for (const source of imageSource) { const ready = await worker.waitForReady(); // check that the container can still start - await vi.waitFor(async () => { + await waitFor(async () => { const response = await fetch(`${ready.url}/status`); expect(response.status).toBe(200); const status = await response.json(); diff --git a/packages/wrangler/e2e/deployments.test.ts b/packages/wrangler/e2e/deployments.test.ts index c2edc88e5f..c02930a620 100644 --- a/packages/wrangler/e2e/deployments.test.ts +++ b/packages/wrangler/e2e/deployments.test.ts @@ -11,7 +11,6 @@ import { describe, expect, it, - vi, } from "vitest"; /* eslint-enable no-restricted-imports */ import { CLOUDFLARE_ACCOUNT_ID } from "./helpers/account-id"; @@ -19,6 +18,7 @@ import { WranglerE2ETestHelper } from "./helpers/e2e-wrangler-test"; import { generateResourceName } from "./helpers/generate-resource-name"; import { normalizeOutput, validateAssetUploadLogs } from "./helpers/normalize"; import { retry } from "./helpers/retry"; +import { waitForLong } from "./helpers/wait-for"; const TIMEOUT = 50_000; @@ -223,7 +223,7 @@ function generateInitialAssets(workerName: string) { async function checkAssets(testCases: AssetTestCase[], deployedUrl: string) { for (const testCase of testCases) { - await vi.waitFor( + await waitForLong( async () => { const r = await fetch(new URL(testCase.path, deployedUrl)); const text = await r.text(); @@ -247,10 +247,7 @@ async function checkAssets(testCases: AssetTestCase[], deployedUrl: string) { ).toEqual(new URL(testCase.path, deployedUrl).pathname); } }, - { - interval: 1_000, - timeout: 40_000, - } + { timeout: 40_000 } ); } } @@ -987,7 +984,7 @@ describe.skipIf(skipContainersTest)("containers", () => { "can fetch DO container", { timeout: 60 * 2 * 1000 }, async ({ expect }) => { - await vi.waitFor( + await waitForLong( async () => { const response = await fetch(`${deployedUrl}/do`, { signal: AbortSignal.timeout(5_000), @@ -1000,9 +997,7 @@ describe.skipIf(skipContainersTest)("containers", () => { expect(await response.text()).toEqual("hello from container"); }, - - // big timeout for containers - // (3m) + // big timeout for containers (2m) { timeout: 60 * 2 * 1000, interval: 1000 } ); } diff --git a/packages/wrangler/e2e/dev-registry.test.ts b/packages/wrangler/e2e/dev-registry.test.ts index 3542463f5a..143cf2a771 100644 --- a/packages/wrangler/e2e/dev-registry.test.ts +++ b/packages/wrangler/e2e/dev-registry.test.ts @@ -1,36 +1,13 @@ -import getPort from "get-port"; import dedent from "ts-dedent"; -import { fetch, Request } from "undici"; -import { beforeEach, describe, it, vi } from "vitest"; +import { fetch } from "undici"; +import { beforeEach, describe, it } from "vitest"; import { WranglerE2ETestHelper } from "./helpers/e2e-wrangler-test"; +import { fetchJson } from "./helpers/fetch-json"; import { fetchText } from "./helpers/fetch-text"; import { generateResourceName } from "./helpers/generate-resource-name"; import { normalizeOutput } from "./helpers/normalize"; import { seed as baseSeed, makeRoot } from "./helpers/setup"; -import type { RequestInit } from "undici"; - -async function fetchJson(url: string, info?: RequestInit): Promise { - const request = new Request(url, info); - const headers = new Headers(request.headers); - - headers.set("MF-Disable-Pretty-Error", "true"); - - return vi.waitFor( - async () => { - const text: string = await fetch(request, { - headers, - }).then((r) => r.text()); - try { - return JSON.parse(text) as T; - } catch (cause) { - const err = new Error(`Failed to parse JSON from:\n${text}`); - err.cause = cause; - throw err; - } - }, - { timeout: 10_000, interval: 250 } - ); -} +import { waitFor, waitForLong } from "./helpers/wait-for"; describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { let workerName: string; @@ -179,13 +156,14 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { const workerA = helper.runLongLived(cmd, { cwd: a }); const { url } = await workerA.waitForReady(5_000); - await vi.waitFor( - async () => await expect(fetchText(url)).resolves.toBe("hello world"), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(url)).resolves.toBe("hello world") ); - expect(normalizeOutput(workerA.currentOutput)).toContain( - "connect to other Wrangler or Vite dev processes running locally" + await waitFor(async () => + expect(normalizeOutput(workerA.currentOutput)).toContain( + "connect to other Wrangler or Vite dev processes running locally" + ) ); }); @@ -196,9 +174,8 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { const workerB = helper.runLongLived(cmd, { cwd: b }); await workerB.waitForReady(5_000); - await vi.waitFor( - async () => await expect(fetchText(url)).resolves.toBe("hello world"), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(url)).resolves.toBe("hello world") ); }); }); @@ -229,12 +206,10 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { const { url } = await workerA.waitForReady(5_000); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/service`)).resolves.toBe( - "Hello from service worker" - ), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(`${url}/service`)).resolves.toBe( + "Hello from service worker" + ) ); }); @@ -249,12 +224,10 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { await workerC.waitForReady(5_000); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/service`)).resolves.toBe( - "Hello from service worker" - ), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(`${url}/service`)).resolves.toBe( + "Hello from service worker" + ) ); } ); @@ -315,13 +288,10 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { await expect(fetchText(`${url}`)).resolves.toBe("hello from a"); - await vi.waitFor( - async () => { - await fetchText(`${url}`); - expect(workerB.currentOutput).includes("received tail event"); - }, - { interval: 1000, timeout: 10_000 } - ); + await waitForLong(async () => { + await fetchText(`${url}`); + expect(workerB.currentOutput).includes("received tail event"); + }); }); }); @@ -372,17 +342,13 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { await workerA.waitForReady(5_000); - await vi.waitFor( - async () => - await expect( - fetchJson(`${url}/do`, { - headers: { - "X-Reset-Count": "true", - }, - }) - ).resolves.toMatchObject({ count: 1 }), - { interval: 1000, timeout: 10_000 } - ); + await expect( + fetchJson(`${url}/do`, { + headers: { + "X-Reset-Count": "true", + }, + }) + ).resolves.toMatchObject({ count: 1 }); } ); @@ -396,16 +362,14 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { const { url } = await workerB.waitForReady(5_000); - await vi.waitFor( - async () => - await expect( - fetch(`${url}/do`, { - headers: { - "X-Reset-Count": "true", - }, - }).then((r) => r.json()) - ).resolves.toMatchObject({ count: 1 }), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect( + fetch(`${url}/do`, { + headers: { + "X-Reset-Count": "true", + }, + }).then((r) => r.json()) + ).resolves.toMatchObject({ count: 1 }) ); }); }); @@ -444,9 +408,8 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { }); it("can fetch a (pages project)", async ({ expect }) => { - const port = await getPort(); const worker = helper.runLongLived( - `${cmd.replace("wrangler dev", "wrangler pages dev")} --port ${port}`, + `${cmd.replace("wrangler dev", "wrangler pages dev")}`, { cwd: a } ); @@ -462,30 +425,26 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { // We don't need b's URL, but ensure that b starts up before a await workerB.waitForReady(5_000); - const port = await getPort(); const workerA = helper.runLongLived( - `${cmd.replace("wrangler dev", "wrangler pages dev")} --port ${port}`, + `${cmd.replace("wrangler dev", "wrangler pages dev")}`, { cwd: a } ); const { url } = await workerA.waitForReady(5_000); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/service`)).resolves.toBe( - "hello world" - ), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(`${url}/service`)).resolves.toBe("hello world") ); - expect(normalizeOutput(workerA.currentOutput)).toContain( - "connect to other Wrangler or Vite dev processes running locally" + await waitFor(async () => + expect(normalizeOutput(workerA.currentOutput)).toContain( + "connect to other Wrangler or Vite dev processes running locally" + ) ); }); it("can fetch b through a (start a, start b)", async ({ expect }) => { - const port = await getPort(); const workerA = helper.runLongLived( - `${cmd.replace("wrangler dev", "wrangler pages dev")} --port ${port}`, + `${cmd.replace("wrangler dev", "wrangler pages dev")}`, { cwd: a } ); const { url } = await workerA.waitForReady(5_000); @@ -493,12 +452,8 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { const workerB = helper.runLongLived(cmd, { cwd: b }); await workerB.waitForReady(5_000); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/service`)).resolves.toBe( - "hello world" - ), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(`${url}/service`)).resolves.toBe("hello world") ); }); @@ -509,9 +464,8 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { "wrangler.toml": dedent` `, }); - const port = await getPort(); const workerA = helper.runLongLived( - `${cmd.replace("wrangler dev", "wrangler pages dev")} dist --service BEE=${workerName2} --port ${port}`, + `${cmd.replace("wrangler dev", "wrangler pages dev")} dist --service BEE=${workerName2}`, { cwd: a } ); const { url } = await workerA.waitForReady(5_000); @@ -519,12 +473,8 @@ describe.each([{ cmd: "wrangler dev" }])("dev registry $cmd", ({ cmd }) => { const workerB = helper.runLongLived(cmd, { cwd: b }); await workerB.waitForReady(5_000); - await vi.waitFor( - async () => - await expect(fetchText(`${url}/service`)).resolves.toBe( - "hello world" - ), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(`${url}/service`)).resolves.toBe("hello world") ); }); }); diff --git a/packages/wrangler/e2e/dev.test.ts b/packages/wrangler/e2e/dev.test.ts index c081d405bd..de93375493 100644 --- a/packages/wrangler/e2e/dev.test.ts +++ b/packages/wrangler/e2e/dev.test.ts @@ -1,12 +1,15 @@ -import assert, { fail } from "node:assert"; +import assert from "node:assert"; import { existsSync } from "node:fs"; import { readFile } from "node:fs/promises"; import * as nodeNet from "node:net"; -import { scheduler, setTimeout } from "node:timers/promises"; +import { setTimeout } from "node:timers/promises"; import dedent from "ts-dedent"; import { fetch } from "undici"; -import { afterEach, beforeEach, describe, it } from "vitest"; -import { CLOUDFLARE_ACCOUNT_ID } from "./helpers/account-id"; +import { afterEach, beforeEach, describe, it, vi } from "vitest"; +import { + CLOUDFLARE_ACCOUNT_ID, + E2E_ACCOUNT_WORKERS_DEV_DOMAIN, +} from "./helpers/account-id"; import { WranglerE2ETestHelper } from "./helpers/e2e-wrangler-test"; import { fetchText } from "./helpers/fetch-text"; import { fetchWithETag } from "./helpers/fetch-with-etag"; @@ -20,6 +23,7 @@ import { POSTGRES_SSL_REQUEST_PACKET, } from "./helpers/postgres-echo-handler"; import { retry } from "./helpers/retry"; +import { waitFor, waitForLong } from "./helpers/wait-for"; import { getStartedWorkerdProcesses } from "./helpers/workerd-processes"; const HYPERDRIVE_DATABASES = [ @@ -97,7 +101,7 @@ describe.each([ // Regression test for issue where multiple request logs were being logged per request expect([...worker.currentOutput.matchAll(/GET /g)].length).toBe(1); - await expect(fetchText(url)).resolves.toMatchSnapshot(); + await waitForLong(() => expect(fetchText(url)).resolves.toMatchSnapshot()); }); it("works with basic service worker", async ({ expect }) => { @@ -176,9 +180,11 @@ describe.each([ const { url } = await worker.waitForReady(); - await expect(fetch(url).then((r) => r.text())).resolves.toMatchSnapshot(); + await waitForLong(() => + expect(fetch(url).then((r) => r.text())).resolves.toMatchSnapshot() + ); - await expect(worker.currentOutput).not.toContain("[b] open a browser"); + expect(worker.currentOutput).not.toContain("[b] open a browser"); }); describe(`--test-scheduled works with ${cmd}`, async () => { @@ -288,13 +294,15 @@ describe.each([ const { hostname, port } = new URL(url); // The warning should contain the actual port, not "undefined" - expect(worker.currentOutput).toContain( - "Scheduled Workers are not automatically triggered" - ); - expect(worker.currentOutput).toContain( - `curl "http://${hostname}:${port}/cdn-cgi/handler/scheduled"` - ); - expect(worker.currentOutput).not.toContain("undefined"); + await waitFor(() => { + expect(worker.currentOutput).toContain( + "Scheduled Workers are not automatically triggered" + ); + expect(worker.currentOutput).toContain( + `curl "http://${hostname}:${port}/cdn-cgi/handler/scheduled"` + ); + expect(worker.currentOutput).not.toContain("undefined"); + }); }); it("does not show warning when --test-scheduled is enabled", async ({ @@ -369,7 +377,9 @@ describe.each([ }); const worker = helper.runLongLived(cmd); - const { url } = await worker.waitForReady(); + // Remote mode with assets involves session creation + asset upload + + // bundle upload to edge-preview, which can be slow on Windows CI. + const { url } = await worker.waitForReady(30_000); await expect(fetch(url).then((r) => r.text())).resolves.toMatchSnapshot(); @@ -382,7 +392,7 @@ describe.each([ }`, }); - await worker.waitForReload(); + await worker.waitForReload(30_000); await expect(fetchText(url)).resolves.toMatchSnapshot(); }); @@ -417,7 +427,7 @@ describe.each([ }); const worker = helper.runLongLived(cmd); - const { url } = await worker.waitForReady(); + const { url } = await worker.waitForReady(30_000); await expect(fetch(url).then((r) => r.text())).resolves.toMatchSnapshot(); @@ -426,7 +436,7 @@ describe.each([ Welcome to updated Workers + Assets readme!`, }); - await worker.waitForReload(); + await worker.waitForReload(30_000); await expect(fetchText(url)).resolves.toMatchSnapshot(); }); @@ -1231,7 +1241,7 @@ describe.skipIf(CLOUDFLARE_ACCOUNT_ID !== "8d783f274e1f82dc46744c297b015a2f")( const text = await fetchText(url); - expect(text).toContain(`devprod-testing7928.workers.dev`); + expect(text).toContain(E2E_ACCOUNT_WORKERS_DEV_DOMAIN); }); it("respects dev.host setting", async ({ expect }) => { @@ -2385,19 +2395,20 @@ This is a random email body. expect(response.status).toBe(200); - expect(worker.currentOutput).includes( - "Email handler replied to sender with the following message:" - ); + await waitFor(() => { + expect(worker.currentOutput).toContain( + "Email handler replied to sender with the following message:" + ); + }); const pathRegexp = new RegExp( "Email handler replied to sender with the following message:\\s*(\\S*)" ); - const maybeReplyPath = pathRegexp.exec(worker.currentOutput)?.[1]; - - if (maybeReplyPath === undefined) { - fail("Reply message does not contain path"); - } + const maybeReplyPath = await vi.waitUntil( + () => pathRegexp.exec(worker.currentOutput)?.[1], + { interval: 100, timeout: 5000 } + ); expect(await readFile(maybeReplyPath, "utf-8")).toMatchInlineSnapshot(` "References: @@ -2496,10 +2507,12 @@ This is a random email body. expect(response.status).toBe(200); - expect(worker.currentOutput).includes( - `Email handler forwarded message with` - ); - expect(worker.currentOutput).includes(`rcptTo: mark.s@example.com`); + await waitFor(() => { + expect(worker.currentOutput).toContain( + `Email handler forwarded message with` + ); + expect(worker.currentOutput).toContain(`rcptTo: mark.s@example.com`); + }); }); it("should save file on send_email", async ({ expect }) => { @@ -2550,21 +2563,20 @@ This is a random email body. expect(response.status).toBe(200); - await scheduler.wait(1000); - - expect(worker.currentOutput).includes( - "send_email binding called with the following message" + await waitFor(() => + expect(worker.currentOutput).toContain( + "send_email binding called with the following message" + ) ); const pathRegexp = new RegExp( "send_email binding called with the following message:\\s*(\\S*)" ); - const maybeReplyPath = pathRegexp.exec(worker.currentOutput)?.[1]; - - if (maybeReplyPath === undefined || maybeReplyPath === null) { - fail("send_email message does not contain path"); - } + const maybeReplyPath = await vi.waitUntil( + () => pathRegexp.exec(worker.currentOutput)?.[1], + { interval: 100, timeout: 5000 } + ); expect(await readFile(maybeReplyPath, "utf-8")).toMatchInlineSnapshot(` "From: someone diff --git a/packages/wrangler/e2e/helpers/account-id.ts b/packages/wrangler/e2e/helpers/account-id.ts index c7f313d581..f7146aa6de 100644 --- a/packages/wrangler/e2e/helpers/account-id.ts +++ b/packages/wrangler/e2e/helpers/account-id.ts @@ -1,2 +1,12 @@ export const CLOUDFLARE_ACCOUNT_ID = process.env .CLOUDFLARE_ACCOUNT_ID as string; + +/** + * The workers.dev subdomain for the account used by e2e tests. + * + * Set the `E2E_ACCOUNT_WORKERS_DEV_DOMAIN` environment variable to run the + * tests against a different account (e.g. your personal account's subdomain). + */ +export const E2E_ACCOUNT_WORKERS_DEV_DOMAIN = + process.env.E2E_ACCOUNT_WORKERS_DEV_DOMAIN ?? + "devprod-testing7928.workers.dev"; diff --git a/packages/wrangler/e2e/helpers/command.ts b/packages/wrangler/e2e/helpers/command.ts index 959b60133c..966767ae85 100644 --- a/packages/wrangler/e2e/helpers/command.ts +++ b/packages/wrangler/e2e/helpers/command.ts @@ -150,7 +150,7 @@ export class LongLivedCommand { } async stop() { - return new Promise((resolve) => { + await new Promise((resolve) => { assert( this.commandProcess.pid, `Command "${this.command}" had no process id` @@ -169,6 +169,10 @@ export class LongLivedCommand { resolve(); }); }); + // Wait for the process to actually exit so that ports are fully released + // before the next test starts. Without this, the next test may encounter + // EADDRINUSE because the dying process still holds the port. + await this.exitPromise.catch(() => {}); } async signal(signal: NodeJS.Signals) { diff --git a/packages/wrangler/e2e/helpers/e2e-wrangler-test.ts b/packages/wrangler/e2e/helpers/e2e-wrangler-test.ts index 5830a7dc77..6dac7a99d4 100644 --- a/packages/wrangler/e2e/helpers/e2e-wrangler-test.ts +++ b/packages/wrangler/e2e/helpers/e2e-wrangler-test.ts @@ -3,8 +3,8 @@ import crypto from "node:crypto"; import { cp } from "node:fs/promises"; import { setTimeout } from "node:timers/promises"; import { fetch } from "undici"; -// eslint-disable-next-line no-restricted-imports -import { expect, onTestFinished, vi } from "vitest"; +import { onTestFinished } from "vitest"; +import { E2E_ACCOUNT_WORKERS_DEV_DOMAIN } from "./account-id"; import { generateLeafCertificate, generateMtlsCertName, @@ -12,6 +12,7 @@ import { } from "./cert"; import { generateResourceName } from "./generate-resource-name"; import { makeRoot, removeFiles, seed } from "./setup"; +import { waitForLong } from "./wait-for"; import { MINIFLARE_IMPORT, runWrangler, @@ -266,6 +267,49 @@ export class WranglerE2ETestHelper { return certificateId; } + /** + * Ensure a worker with a well-known `preserve-e2e-*` name is deployed. + * + * Checks whether the worker is already live by fetching its workers.dev + * URL (controlled by `E2E_ACCOUNT_WORKERS_DEV_DOMAIN`). If it responds + * with a non-404 status the deploy is skipped; otherwise `wrangler deploy` + * is run and the helper waits for the worker to become available. + * + * No cleanup is registered — the worker is expected to persist across + * test runs and is excluded from the periodic e2e cleanup job by its + * `preserve-e2e-` prefix. + */ + async ensureWorkerDeployed({ + workerName, + entryPoint = "", + configPath, + }: { + workerName: string; + entryPoint?: string; + configPath?: string; + }): Promise { + const deployedUrl = `https://${workerName}.${E2E_ACCOUNT_WORKERS_DEV_DOMAIN}/`; + try { + const response = await fetch(deployedUrl); + if (response.status !== 404) { + return; // Worker already exists + } + } catch { + // Worker doesn't exist or is not reachable — fall through to deploy + } + const configOption = configPath ? `-c ${configPath}` : ""; + await this.run( + `wrangler deploy ${entryPoint} --name ${workerName} ${configOption} --compatibility-date 2025-01-01` + ); + await waitForLong(async () => { + const response = await fetch(deployedUrl); + assert( + response.status === 200, + `Expected status 200 but got ${response.status}` + ); + }); + } + /** * Create a worker for the test and attempt to delete it after the test has finished. * @@ -325,13 +369,13 @@ export class WranglerE2ETestHelper { await setTimeout(2_000); // Wait for the worker to become available - await vi.waitFor( - async () => { - const response = await fetch(deployedUrl); - expect(response.status).toBe(200); - }, - { timeout: 10_000, interval: 500 } - ); + await waitForLong(async () => { + const response = await fetch(deployedUrl); + assert( + response.status === 200, + `Expected status 200 but got ${response.status}` + ); + }); const cleanup = async () => { await this.bestEffortRun(`wrangler delete --name ${workerName} --force`); diff --git a/packages/wrangler/e2e/helpers/fetch-json.ts b/packages/wrangler/e2e/helpers/fetch-json.ts new file mode 100644 index 0000000000..941c6ec918 --- /dev/null +++ b/packages/wrangler/e2e/helpers/fetch-json.ts @@ -0,0 +1,29 @@ +import { fetch, Request } from "undici"; +import { waitForLong } from "./wait-for"; +import type { RequestInit } from "undici"; + +/** + * Fetch a URL, parse the response as JSON, and retry until it succeeds. + * Uses `waitForLong` internally so it will keep retrying on network errors + * or non-JSON responses until the timeout is reached. + */ +export async function fetchJson( + url: string, + info?: RequestInit +): Promise { + return waitForLong(async () => { + const request = new Request(url, info); + const headers = new Headers(request.headers); + headers.set("MF-Disable-Pretty-Error", "true"); + const text: string = await fetch(request, { + headers, + }).then((r) => r.text()); + try { + return JSON.parse(text) as T; + } catch (cause) { + const err = new Error(`Failed to parse JSON from:\n${text}`); + err.cause = cause; + throw err; + } + }); +} diff --git a/packages/wrangler/e2e/helpers/wait-for.ts b/packages/wrangler/e2e/helpers/wait-for.ts new file mode 100644 index 0000000000..5b4fb36e32 --- /dev/null +++ b/packages/wrangler/e2e/helpers/wait-for.ts @@ -0,0 +1,34 @@ +import { vi } from "vitest"; + +type WaitForOptions = { interval?: number; timeout?: number }; + +/** + * Wrapper around `vi.waitFor()` for polling synchronous state (e.g. `currentOutput`). + * Defaults to `{ interval: 100, timeout: 5_000 }`. + */ +export function waitFor( + callback: () => T | Promise, + options?: WaitForOptions +): Promise { + return vi.waitFor(callback, { + interval: 100, + timeout: 5_000, + ...options, + }); +} + +/** + * Wrapper around `vi.waitFor()` with a slower poll interval and longer timeout than `waitFor`. + * Useful for polling HTTP endpoints (e.g. `fetch`/`fetchText`). + * Defaults to `{ interval: 500, timeout: 10_000 }`. + */ +export function waitForLong( + callback: () => T | Promise, + options?: WaitForOptions +): Promise { + return vi.waitFor(callback, { + interval: 500, + timeout: 10_000, + ...options, + }); +} diff --git a/packages/wrangler/e2e/helpers/wrangler.ts b/packages/wrangler/e2e/helpers/wrangler.ts index 700ad0360d..ead4be84a2 100644 --- a/packages/wrangler/e2e/helpers/wrangler.ts +++ b/packages/wrangler/e2e/helpers/wrangler.ts @@ -51,6 +51,13 @@ export class WranglerLongLivedCommand extends LongLivedCommand { } } +function isDevCommand(command: string) { + return ( + command.startsWith("wrangler dev") || + command.startsWith("wrangler pages dev") + ); +} + function getWranglerCommand(command: string) { // Enforce a `wrangler` prefix to make commands clearer to read assert( @@ -61,15 +68,13 @@ function getWranglerCommand(command: string) { // If the user hasn't specifically set an inspector port, set it to 0 to reduce port conflicts const inspectorPort = - command.includes(`--inspector-port`) || !command.startsWith("wrangler dev") + command.includes(`--inspector-port`) || !isDevCommand(command) ? "" : " --inspector-port 0"; // If the user hasn't specifically set a Worker port, set it to 0 to reduce port conflicts const workerPort = - command.includes(`--port`) || !command.startsWith("wrangler dev") - ? "" - : " --port 0"; + command.includes(`--port`) || !isDevCommand(command) ? "" : " --port 0"; return `${WRANGLER} ${command.slice("wrangler ".length)}${inspectorPort}${workerPort}`; } diff --git a/packages/wrangler/e2e/multiworker-dev.test.ts b/packages/wrangler/e2e/multiworker-dev.test.ts index 4a4966f486..015e1c936f 100644 --- a/packages/wrangler/e2e/multiworker-dev.test.ts +++ b/packages/wrangler/e2e/multiworker-dev.test.ts @@ -1,31 +1,13 @@ import { randomUUID } from "node:crypto"; import dedent from "ts-dedent"; import { fetch } from "undici"; -import { beforeEach, describe, it, vi } from "vitest"; +import { beforeEach, describe, it } from "vitest"; import { WranglerE2ETestHelper } from "./helpers/e2e-wrangler-test"; +import { fetchJson } from "./helpers/fetch-json"; import { fetchText } from "./helpers/fetch-text"; import { generateResourceName } from "./helpers/generate-resource-name"; import { seed as baseSeed, makeRoot } from "./helpers/setup"; -import type { RequestInit } from "undici"; - -async function fetchJson(url: string, info?: RequestInit): Promise { - return vi.waitFor( - async () => { - const text: string = await fetch(url, { - headers: { "MF-Disable-Pretty-Error": "true" }, - ...info, - }).then((r) => r.text()); - try { - return JSON.parse(text) as T; - } catch (cause) { - const err = new Error(`Failed to parse JSON from:\n${text}`); - err.cause = cause; - throw err; - } - }, - { timeout: 10_000, interval: 250 } - ); -} +import { waitFor, waitForLong } from "./helpers/wait-for"; describe("multiworker", () => { let workerName: string; @@ -221,9 +203,8 @@ describe("multiworker", () => { ); const { url } = await workerA.waitForReady(5_000); - await vi.waitFor( - async () => await expect(fetchText(url)).resolves.toBe("hello world"), - { interval: 1000, timeout: 10_000 } + await waitForLong( + async () => await expect(fetchText(url)).resolves.toBe("hello world") ); }); @@ -236,9 +217,8 @@ describe("multiworker", () => { ); const { url } = await workerA.waitForReady(5_000); - await vi.waitFor( - async () => await expect(fetchText(`${url}/count`)).resolves.toBe("6"), - { interval: 1000, timeout: 10_000 } + await waitForLong( + async () => await expect(fetchText(`${url}/count`)).resolves.toBe("6") ); }); @@ -249,17 +229,14 @@ describe("multiworker", () => { ); const { url } = await workerA.waitForReady(5_000); - await vi.waitFor( - async () => { - const response = await fetch(`${url}/props`); - const props = await response.json(); - expect(props).toEqual({ - foo: 123, - bar: { baz: "hello from props" }, - }); - }, - { interval: 1000, timeout: 10_000 } - ); + await waitForLong(async () => { + const response = await fetch(`${url}/props`); + const props = await response.json(); + expect(props).toEqual({ + foo: 123, + bar: { baz: "hello from props" }, + }); + }); }); it("shows runtime error when fetching non-existent service", async ({ @@ -285,12 +262,11 @@ describe("multiworker", () => { const { url } = await workerA.waitForReady(5_000); - await vi.waitFor( + await waitForLong( async () => await expect(fetchText(url)).resolves.toBe( `Couldn't find a local dev session for the "default" entrypoint of service "${service}" to proxy to` - ), - { interval: 1000, timeout: 10_000 } + ) ); }); }); @@ -317,12 +293,11 @@ describe("multiworker", () => { ); const { url } = await workerA.waitForReady(5_000); - await vi.waitFor( + await waitForLong( async () => await expect(fetchText(`${url}/service`)).resolves.toBe( "Hello from service worker" - ), - { interval: 1000, timeout: 10_000 } + ) ); }); }); @@ -371,17 +346,13 @@ describe("multiworker", () => { ); const { url } = await workerB.waitForReady(5_000); - await vi.waitFor( - async () => - await expect( - fetchJson(`${url}/do`, { - headers: { - "X-Reset-Count": "true", - }, - }) - ).resolves.toMatchObject({ count: 1 }), - { interval: 1000, timeout: 10_000 } - ); + await expect( + fetchJson(`${url}/do`, { + headers: { + "X-Reset-Count": "true", + }, + }) + ).resolves.toMatchObject({ count: 1 }); }); it("can fetch remote DO attached to a through b with RPC", async ({ @@ -393,12 +364,11 @@ describe("multiworker", () => { ); const { url } = await workerB.waitForReady(5_000); - await vi.waitFor( + await waitForLong( async () => await expect(fetchText(`${url}/do-rpc`)).resolves.toBe( "Hello through DO RPC" - ), - { interval: 1000, timeout: 10_000 } + ) ); }); }); @@ -458,10 +428,8 @@ describe("multiworker", () => { await expect(fetchText(`${url}`)).resolves.toBe("hello from a"); - await vi.waitFor( - async () => - expect(worker.currentOutput).includes("received tail event"), - { interval: 1000, timeout: 10_000 } + await waitFor(() => + expect(worker.currentOutput).includes("received tail event") ); }); }); @@ -518,12 +486,11 @@ describe("multiworker", () => { ); const { url } = await worker.waitForReady(5_000); - await expect(fetchText(`${url}`)).resolves.toBe("hello from a"); - - await vi.waitFor( - async () => - expect(worker.currentOutput).includes("received tail stream event"), - { interval: 1000, timeout: 10_000 } + await waitForLong(() => + expect(fetchText(`${url}`)).resolves.toBe("hello from a") + ); + await waitFor(() => + expect(worker.currentOutput).includes("received tail stream event") ); }); }); @@ -563,12 +530,11 @@ describe("multiworker", () => { ); const { url } = await pages.waitForReady(5_000); - await vi.waitFor( + await waitForLong( async () => await expect(fetchText(`${url}`)).resolves.toBe( "

hello pages assets

" - ), - { interval: 1000, timeout: 10_000 } + ) ); }); @@ -579,12 +545,11 @@ describe("multiworker", () => { ); const { url } = await pages.waitForReady(5_000); - await vi.waitFor( + await waitForLong( async () => await expect(fetchText(`${url}/cee`)).resolves.toBe( "Hello from service worker" - ), - { interval: 1000, timeout: 10_000 } + ) ); }); @@ -595,10 +560,9 @@ describe("multiworker", () => { ); const { url } = await pages.waitForReady(5_000); - await vi.waitFor( + await waitForLong( async () => - await expect(fetchText(`${url}/bee`)).resolves.toBe("hello world"), - { interval: 1000, timeout: 10_000 } + await expect(fetchText(`${url}/bee`)).resolves.toBe("hello world") ); }); @@ -675,14 +639,16 @@ describe("multiworker", () => { const { url } = await worker.waitForReady(5_000); const { hostname, port } = new URL(url); - // The warning should contain the actual port, not "undefined" - expect(worker.currentOutput).toContain( - "Scheduled Workers are not automatically triggered" - ); - expect(worker.currentOutput).toContain( - `curl "http://${hostname}:${port}/cdn-cgi/handler/scheduled"` - ); - expect(worker.currentOutput).not.toContain("undefined"); + await waitFor(() => { + // The warning should contain the actual port, not "undefined" + expect(worker.currentOutput).toContain( + "Scheduled Workers are not automatically triggered" + ); + expect(worker.currentOutput).toContain( + `curl "http://${hostname}:${port}/cdn-cgi/handler/scheduled"` + ); + expect(worker.currentOutput).not.toContain("undefined"); + }); }); }); }); diff --git a/packages/wrangler/e2e/pages-dev.test.ts b/packages/wrangler/e2e/pages-dev.test.ts index ddf693c311..1d5989edbc 100644 --- a/packages/wrangler/e2e/pages-dev.test.ts +++ b/packages/wrangler/e2e/pages-dev.test.ts @@ -8,6 +8,7 @@ import { describe, it } from "vitest"; import { WranglerE2ETestHelper } from "./helpers/e2e-wrangler-test"; import { fetchText } from "./helpers/fetch-text"; import { normalizeOutput } from "./helpers/normalize"; +import { waitFor } from "./helpers/wait-for"; const port = await getPort(); const inspectorPort = await getPort(); @@ -67,8 +68,10 @@ describe.sequential("wrangler pages dev", () => { const { url } = await worker.waitForReady(); const text = await fetchText(url); expect(text).toBe("Testing [--experimental-local]"); - expect(await worker.currentOutput).toContain( - `--experimental-local is no longer required and will be removed in a future version` + await waitFor(() => + expect(worker.currentOutput).toContain( + `--experimental-local is no longer required and will be removed in a future version` + ) ); }); @@ -93,21 +96,17 @@ describe.sequential("wrangler pages dev", () => { `${cmd} --inspector-port ${inspectorPort} . --port ${port} --service test --kv = --do test --d1 = --r2 =` ); await worker.waitForReady(); - expect(await worker.currentOutput).toContain( - `Could not parse Service binding: test` - ); - expect(await worker.currentOutput).toContain( - `Could not parse KV binding: =` - ); - expect(await worker.currentOutput).toContain( - `Could not parse Durable Object binding: test` - ); - expect(await worker.currentOutput).toContain( - `Could not parse R2 binding: =` - ); - expect(await worker.currentOutput).toContain( - `Could not parse D1 binding: =` - ); + await waitFor(() => { + expect(worker.currentOutput).toContain( + `Could not parse Service binding: test` + ); + expect(worker.currentOutput).toContain(`Could not parse KV binding: =`); + expect(worker.currentOutput).toContain( + `Could not parse Durable Object binding: test` + ); + expect(worker.currentOutput).toContain(`Could not parse R2 binding: =`); + expect(worker.currentOutput).toContain(`Could not parse D1 binding: =`); + }); }); it("should use bindings specified as args in the command line", async ({ @@ -126,6 +125,13 @@ describe.sequential("wrangler pages dev", () => { `${cmd} --inspector-port ${inspectorPort} . --port ${port} --service TEST_SERVICE=test-worker --kv TEST_KV --do TEST_DO=TestDurableObject@a --d1 TEST_D1 --r2 TEST_R2 --compatibility-date=2025-05-21` ); await worker.waitForReady(); + + await waitFor(() => + expect(worker.currentOutput).toContain( + "Your Worker has access to the following bindings:" + ) + ); + const bindingMessages = worker.currentOutput.split( "Your Worker has access to the following bindings:" ); @@ -354,6 +360,12 @@ describe.sequential("wrangler pages dev", () => { await worker.readUntil(/GET \/ 200 OK/); + await waitFor(() => + expect(worker.currentOutput).toContain( + "Your Worker has access to the following bindings:" + ) + ); + expect(normalizeOutput(worker.currentOutput)).toMatchInlineSnapshot(` "✨ Compiled Worker successfully Your Worker has access to the following bindings: @@ -459,6 +471,12 @@ describe.sequential("wrangler pages dev", () => { }); await worker.waitForReady(); + await waitFor(() => + expect(worker.currentOutput).toContain( + "Your Worker has access to the following bindings:" + ) + ); + // We only care about the list of bindings and warnings, so strip other output const [prestartOutput] = normalizeOutput(worker.currentOutput).split( "⎔ Starting local server..." diff --git a/packages/wrangler/e2e/remote-binding/dev-remote-bindings.test.ts b/packages/wrangler/e2e/remote-binding/dev-remote-bindings.test.ts index 5b5b8f4786..8c3912455a 100644 --- a/packages/wrangler/e2e/remote-binding/dev-remote-bindings.test.ts +++ b/packages/wrangler/e2e/remote-binding/dev-remote-bindings.test.ts @@ -1,42 +1,35 @@ import { readFile, writeFile } from "node:fs/promises"; import { resolve } from "node:path"; import { setTimeout } from "node:timers/promises"; -import getPort from "get-port"; import dedent from "ts-dedent"; -import { beforeAll, describe, it, vi } from "vitest"; +import { beforeAll, describe, it } from "vitest"; import { CLOUDFLARE_ACCOUNT_ID } from "../helpers/account-id"; import { WranglerE2ETestHelper } from "../helpers/e2e-wrangler-test"; import { fetchText } from "../helpers/fetch-text"; -import { generateResourceName } from "../helpers/generate-resource-name"; import { normalizeOutput } from "../helpers/normalize"; import { makeRoot, seed } from "../helpers/setup"; +import { waitFor, waitForLong } from "../helpers/wait-for"; describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)( "wrangler dev - remote bindings", () => { - const remoteWorkerName = generateResourceName(); - const alternativeRemoteWorkerName = generateResourceName(); + const remoteWorkerName = "preserve-e2e-wrangler-remote-worker"; + const alternativeRemoteWorkerName = + "preserve-e2e-wrangler-remote-worker-alt"; const helper = new WranglerE2ETestHelper(); beforeAll(async () => { await helper.seed(resolve(__dirname, "./workers")); - const cleanups = await Promise.all([ - helper - .worker({ - entryPoint: "remote-worker.js", - workerName: remoteWorkerName, - cleanOnTestFinished: false, - }) - .then(({ cleanup }) => cleanup), - helper - .worker({ - entryPoint: "alt-remote-worker.js", - workerName: alternativeRemoteWorkerName, - cleanOnTestFinished: false, - }) - .then(({ cleanup }) => cleanup), + await Promise.all([ + helper.ensureWorkerDeployed({ + entryPoint: "remote-worker.js", + workerName: remoteWorkerName, + }), + helper.ensureWorkerDeployed({ + entryPoint: "alt-remote-worker.js", + workerName: alternativeRemoteWorkerName, + }), ]); - return () => Promise.allSettled(cleanups.map((cleanup) => cleanup())); }, 35_000); it("handles both remote and local service bindings at the same time", async ({ @@ -180,17 +173,17 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)( ); // This should only include logs from the user Wrangler session (i.e. a single list of attached bindings, and only one ready message) - const normalizedOutput = normalizeOutput(worker.currentOutput); - - expect(normalizedOutput).toMatchInlineSnapshot(` - "Your Worker has access to the following bindings: - Binding Resource Mode - env.AI AI remote - ▲ [WARNING] AI bindings always access remote resources, and so may incur usage charges even in local dev. To suppress this warning, set \`remote: true\` for the binding definition in your configuration file. - ⎔ Starting local server... - [wrangler:info] Ready on http://: - [wrangler:info] GET / 200 OK (TIMINGS)" - `); + await waitFor(() => + expect(normalizeOutput(worker.currentOutput)).toEqual(dedent` + Your Worker has access to the following bindings: + Binding Resource Mode + env.AI AI remote + ▲ [WARNING] AI bindings always access remote resources, and so may incur usage charges even in local dev. To suppress this warning, set \`remote: true\` for the binding definition in your configuration file. + ⎔ Starting local server... + [wrangler:info] Ready on http://: + [wrangler:info] GET / 200 OK (TIMINGS) + `) + ); }); describe("shows helpful error logs", () => { @@ -214,12 +207,12 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)( const worker = helper.runLongLived("wrangler dev"); - await vi.waitFor( - () => - expect(worker.currentOutput).toContain( - "Service binding 'REMOTE_WORKER' references Worker 'non-existent-service-binding' which was not found." - ), - 7_000 + // The error appears only after the remote proxy session validates + // the binding against the Cloudflare API, so use the longer timeout. + await waitForLong(() => + expect(worker.currentOutput).toContain( + "Service binding 'REMOTE_WORKER' references Worker 'non-existent-service-binding' which was not found." + ) ); }); @@ -243,12 +236,12 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)( const worker = helper.runLongLived("wrangler dev"); - await vi.waitFor( - () => - expect(worker.currentOutput).toContain( - "KV namespace 'non-existent-kv' is not valid." - ), - 7_000 + // The error appears only after the remote proxy session validates + // the binding against the Cloudflare API, so use the longer timeout. + await waitForLong(() => + expect(worker.currentOutput).toContain( + "KV namespace 'non-existent-kv' is not valid." + ) ); }); }); @@ -304,7 +297,10 @@ describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)( `wrangler dev -c wrangler.json -c ${localTest}/wrangler.json` ); - const { url } = await worker.waitForReady(); + // Multi-worker with remote bindings on both workers requires two + // serialised remote proxy sessions (API calls) before "Ready on" + // can appear, so give it more time than the default 15s. + const { url } = await worker.waitForReady(30_000); await expect(fetchText(url)).resolves.toMatchInlineSnapshot(` "LOCAL: [local-test-worker]REMOTE: Hello from an alternative remote worker @@ -331,11 +327,6 @@ async function spawnLocalWorker(helper: WranglerE2ETestHelper): Promise { } }`, }); - const localWorker = helper.runLongLived( - // Note: we use a random port here otherwise for some reason in CI windows - // allows the default port to be overridden by other processes - `wrangler dev --port ${await getPort()}`, - { cwd: local } - ); + const localWorker = helper.runLongLived("wrangler dev", { cwd: local }); await localWorker.waitForReady(); } diff --git a/packages/wrangler/e2e/remote-binding/remote-bindings-api.test.ts b/packages/wrangler/e2e/remote-binding/remote-bindings-api.test.ts index 4871b499fd..42d74aa331 100644 --- a/packages/wrangler/e2e/remote-binding/remote-bindings-api.test.ts +++ b/packages/wrangler/e2e/remote-binding/remote-bindings-api.test.ts @@ -6,7 +6,6 @@ import { importWrangler, WranglerE2ETestHelper, } from "../helpers/e2e-wrangler-test"; -import { generateResourceName } from "../helpers/generate-resource-name"; import type { MiniflareOptions, Miniflare as MiniflareType, @@ -25,17 +24,15 @@ const { startRemoteProxySession, maybeStartOrUpdateRemoteProxySession } = describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)( "wrangler dev - remote bindings - programmatic API", async () => { - const remoteWorkerName = generateResourceName(); + const remoteWorkerName = "preserve-e2e-wrangler-remote-worker"; const helper = new WranglerE2ETestHelper(); beforeAll(async () => { await helper.seed(resolve(__dirname, "./workers")); - const { cleanup } = await helper.worker({ + await helper.ensureWorkerDeployed({ workerName: remoteWorkerName, entryPoint: "remote-worker.js", - cleanOnTestFinished: false, }); - return cleanup; }, 35_000); function getMfOptions( diff --git a/packages/wrangler/e2e/remote-binding/start-worker-remote-bindings.test.ts b/packages/wrangler/e2e/remote-binding/start-worker-remote-bindings.test.ts index 9439850c3c..dcc554fb4b 100644 --- a/packages/wrangler/e2e/remote-binding/start-worker-remote-bindings.test.ts +++ b/packages/wrangler/e2e/remote-binding/start-worker-remote-bindings.test.ts @@ -7,23 +7,20 @@ import { importWrangler, WranglerE2ETestHelper, } from "../helpers/e2e-wrangler-test"; -import { generateResourceName } from "../helpers/generate-resource-name"; const { unstable_startWorker: startWorker } = await importWrangler(); describe.skipIf(!CLOUDFLARE_ACCOUNT_ID)("startWorker - remote bindings", () => { - const remoteWorkerName = generateResourceName(); + const remoteWorkerName = "preserve-e2e-wrangler-remote-worker"; const helper = new WranglerE2ETestHelper(); beforeAll(async () => { await helper.seed(resolve(__dirname, "./workers")); - const { cleanup } = await helper.worker({ + await helper.ensureWorkerDeployed({ entryPoint: "remote-worker.js", workerName: remoteWorkerName, - cleanOnTestFinished: false, }); - return cleanup; - }, 35_000); + }, 60_000); it("allows connecting to a remote worker", async ({ expect }) => { await helper.seed({ diff --git a/packages/wrangler/e2e/start-worker-auth-opts.test.ts b/packages/wrangler/e2e/start-worker-auth-opts.test.ts index 504a173a73..73f0592aba 100644 --- a/packages/wrangler/e2e/start-worker-auth-opts.test.ts +++ b/packages/wrangler/e2e/start-worker-auth-opts.test.ts @@ -16,6 +16,7 @@ import { importWrangler, WranglerE2ETestHelper, } from "./helpers/e2e-wrangler-test"; +import { waitForLong } from "./helpers/wait-for"; import type { Worker } from "../src/api/startDevWorker"; import type { MockInstance } from "vitest"; @@ -267,7 +268,7 @@ async function fetchTimedTextFromWorker( try { assert(worker, "Worker is not defined"); - await vi.waitFor( + await waitForLong( async () => { responseText = await ( await worker.fetch("http://example.com", { @@ -275,7 +276,7 @@ async function fetchTimedTextFromWorker( }) ).text(); }, - { timeout: 20_000, interval: 700 } + { timeout: 20_000 } ); } catch { return null; diff --git a/packages/wrangler/e2e/startWorker.test.ts b/packages/wrangler/e2e/startWorker.test.ts index f4c2097afc..2b5264acda 100644 --- a/packages/wrangler/e2e/startWorker.test.ts +++ b/packages/wrangler/e2e/startWorker.test.ts @@ -10,6 +10,7 @@ import { importWrangler, WranglerE2ETestHelper, } from "./helpers/e2e-wrangler-test"; +import { waitFor, waitForLong } from "./helpers/wait-for"; import type { DevToolsEvent } from "../src/api"; const OPTIONS = [ @@ -30,9 +31,6 @@ function waitForMessageContaining(ws: WebSocket, value: string): Promise { }); } -const waitFor: typeof vi.waitFor = (cb) => - vi.waitFor(cb, { interval: 200, timeout: 5000 }); - function collectMessagesContaining( ws: WebSocket, value: string, @@ -89,7 +87,7 @@ describe("DevEnv", { sequential: true }, () => { "src/index.ts": script.replace("body:1", "body:2"), }); - await waitFor(async () => { + await waitForLong(async () => { res = await worker.fetch("http://dummy"); expect(await res.text()).toBe("body:2"); }); diff --git a/packages/wrangler/e2e/unenv-preset/preset.test.ts b/packages/wrangler/e2e/unenv-preset/preset.test.ts index deef4d4eb4..eb6719a065 100644 --- a/packages/wrangler/e2e/unenv-preset/preset.test.ts +++ b/packages/wrangler/e2e/unenv-preset/preset.test.ts @@ -1,11 +1,12 @@ +import assert from "node:assert"; import { join } from "node:path"; import { fetch } from "undici"; -// eslint-disable-next-line no-restricted-imports -import { beforeAll, describe, expect, test, vi } from "vitest"; +import { beforeAll, describe, test } from "vitest"; import { CLOUDFLARE_ACCOUNT_ID } from "../helpers/account-id"; import { WranglerE2ETestHelper } from "../helpers/e2e-wrangler-test"; import { generateResourceName } from "../helpers/generate-resource-name"; import { retry } from "../helpers/retry"; +import { waitForLong } from "../helpers/wait-for"; import { WorkerdTests } from "./worker/index"; type TestConfig = { @@ -870,13 +871,21 @@ describe.each(groupedLocalConfigs)( (resp) => !resp.ok, async () => await fetch(`${url}/ping`) ); - await expect(readyResp.text()).resolves.toEqual("pong"); + const responseText = await readyResp.text(); + assert( + responseText === "pong", + `Expected "pong" but got "${responseText}"` + ); // Assert runtime flag values for await (const [flag, value] of Object.entries(expectRuntimeFlags)) { const flagResp = await fetch(`${url}/flag?name=${flag}`); - expect(flagResp.ok).toEqual(true); - await expect(flagResp.json(), `flag "${flag}"`).resolves.toEqual(value); + assert(flagResp.ok, `Expected flag response to be ok for "${flag}"`); + const flagValue = await flagResp.json(); + assert( + flagValue === value, + `Expected flag "${flag}" to be ${value} but got ${flagValue}` + ); } return async () => await wrangler.stop(); @@ -888,13 +897,13 @@ describe.each(groupedLocalConfigs)( async (testName, { expect }) => { // Retries the callback until it succeeds or times out. // Useful for the i.e. DNS tests where underlying requests might error/timeout. - await vi.waitFor( + await waitForLong( async () => { const response = await fetch(`${url}/${testName}`); const body = await response.text(); expect(body).toMatch("passed"); }, - { timeout: 19_000, interval: 200 } + { timeout: 19_000 } ); } ); @@ -928,13 +937,21 @@ describe.runIf(Boolean(CLOUDFLARE_ACCOUNT_ID))( (resp) => !resp.ok, async () => await fetch(`${url}/ping`) ); - await expect(readyResp.text()).resolves.toEqual("pong"); + const responseText = await readyResp.text(); + assert( + responseText === "pong", + `Expected "pong" but got "${responseText}"` + ); // Assert runtime flag values for await (const flag of collectEnabledFlags(localTestConfigs)) { const flagResp = await fetch(`${url}/flag?name=${flag}`); - expect(flagResp.ok).toEqual(true); - await expect(flagResp.json(), `flag "${flag}"`).resolves.toEqual(false); + assert(flagResp.ok, `Expected flag response to be ok for "${flag}"`); + const flagValue = await flagResp.json(); + assert( + flagValue === false, + `Expected flag "${flag}" to be false but got ${flagValue}` + ); } return async () => await wrangler.stop(); @@ -946,13 +963,13 @@ describe.runIf(Boolean(CLOUDFLARE_ACCOUNT_ID))( async (testName, { expect }) => { // Retries the callback until it succeeds or times out. // Useful for the i.e. DNS tests where underlying requests might error/timeout. - await vi.waitFor( + await waitForLong( async () => { const response = await fetch(`${url}/${testName}`); const body = await response.text(); expect(body).toMatch("passed"); }, - { timeout: 19_000, interval: 200 } + { timeout: 19_000 } ); } ); @@ -987,13 +1004,21 @@ describe.runIf(Boolean(CLOUDFLARE_ACCOUNT_ID))( (resp) => !resp.ok, async () => await fetch(`${url}/ping`) ); - await expect(readyResp.text()).resolves.toEqual("pong"); + const responseText = await readyResp.text(); + assert( + responseText === "pong", + `Expected "pong" but got "${responseText}"` + ); // Assert runtime flag values for await (const flag of flags) { const flagResp = await fetch(`${url}/flag?name=${flag}`); - expect(flagResp.ok).toEqual(true); - await expect(flagResp.json(), `flag "${flag}"`).resolves.toEqual(true); + assert(flagResp.ok, `Expected flag response to be ok for "${flag}"`); + const flagValue = await flagResp.json(); + assert( + flagValue === true, + `Expected flag "${flag}" to be true but got ${flagValue}` + ); } return async () => await wrangler.stop(); @@ -1005,13 +1030,13 @@ describe.runIf(Boolean(CLOUDFLARE_ACCOUNT_ID))( async (testName, { expect }) => { // Retries the callback until it succeeds or times out. // Useful for the i.e. DNS tests where underlying requests might error/timeout. - await vi.waitFor( + await waitForLong( async () => { const response = await fetch(`${url}/${testName}`); const body = await response.text(); expect(body).toMatch("passed"); }, - { timeout: 19_000, interval: 200 } + { timeout: 19_000 } ); } ); diff --git a/packages/wrangler/package.json b/packages/wrangler/package.json index 980a107ab9..c4bb8fbc9d 100644 --- a/packages/wrangler/package.json +++ b/packages/wrangler/package.json @@ -181,7 +181,7 @@ "fsevents": "~2.3.2" }, "engines": { - "node": ">=20.0.0" + "node": ">=20.3.0" }, "volta": { "extends": "../../package.json" diff --git a/packages/wrangler/src/__tests__/utils/retry.test.ts b/packages/wrangler/src/__tests__/utils/retry.test.ts index 6e7394af63..2f29ed0714 100644 --- a/packages/wrangler/src/__tests__/utils/retry.test.ts +++ b/packages/wrangler/src/__tests__/utils/retry.test.ts @@ -102,6 +102,76 @@ describe("retryOnAPIFailure", () => { expect(getRetryAndErrorLogs(std.debug)).toMatchInlineSnapshot(`[]`); }); + it("should cancel retry backoff when abort signal fires", async ({ + expect, + }) => { + const controller = new AbortController(); + let attempts = 0; + + const promise = retryOnAPIFailure( + () => { + attempts++; + if (attempts === 1) { + // After the first failure, abort before the backoff completes + queueMicrotask(() => controller.abort()); + } + throw new APIError({ status: 500, text: "500 error" }); + }, + // Use a very long backoff so the test would hang without abort + 10_000, + 3, + controller.signal + ); + + await expect(promise).rejects.toThrow(); + // Only the first attempt should have been made before the abort + // cancelled the backoff delay + expect(attempts).toBe(1); + }); + + it("should propagate abort error from action without retrying", async ({ + expect, + }) => { + const controller = new AbortController(); + controller.abort(); + let attempts = 0; + + await expect(() => + retryOnAPIFailure( + () => { + attempts++; + throw controller.signal.reason; + }, + undefined, + undefined, + controller.signal + ) + ).rejects.toThrow(); + // AbortError is not an APIError or TypeError, so it should not be retried + expect(attempts).toBe(1); + }); + + it("should retry TimeoutError from AbortSignal.timeout()", async ({ + expect, + }) => { + let attempts = 0; + + await expect(() => + retryOnAPIFailure(() => { + attempts++; + throw new DOMException("The operation was aborted.", "TimeoutError"); + }) + ).rejects.toThrow("The operation was aborted."); + expect(attempts).toBe(3); + expect(getRetryAndErrorLogs(std.debug)).toMatchInlineSnapshot(` + [ + "Retrying API call after error...", + "Retrying API call after error...", + "Retrying API call after error...", + ] + `); + }); + it("should retry custom APIError implementation with non-5xx error", async ({ expect, }) => { diff --git a/packages/wrangler/src/api/remoteBindings/start-remote-proxy-session.ts b/packages/wrangler/src/api/remoteBindings/start-remote-proxy-session.ts index ebc580bb96..9fa1dd4f51 100644 --- a/packages/wrangler/src/api/remoteBindings/start-remote-proxy-session.ts +++ b/packages/wrangler/src/api/remoteBindings/start-remote-proxy-session.ts @@ -1,5 +1,4 @@ import path from "node:path"; -import getPort from "get-port"; import { DeferredPromise } from "miniflare"; import remoteBindingsWorkerPath from "worker:remoteBindings/ProxyServerWorker"; import { logger } from "../../logger"; @@ -43,7 +42,7 @@ export async function startRemoteProxySession( remote: "minimal", auth: options?.auth, server: { - port: await getPort(), + port: 0, }, inspector: false, logLevel: getStartWorkerLogLevel(logger.loggerLevel), diff --git a/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts b/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts index 11ce2d03df..585106aa60 100644 --- a/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts +++ b/packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts @@ -17,6 +17,7 @@ import { logger } from "../../logger"; import { TRACE_VERSION } from "../../tail/createTail"; import { realishPrintLogs } from "../../tail/printing"; import { getAccessToken } from "../../user/access"; +import { retryOnAPIFailure } from "../../utils/retry"; import { RuntimeController } from "./BaseController"; import { castErrorCause } from "./events"; import { unwrapHook } from "./utils"; @@ -60,12 +61,18 @@ export class RemoteRuntimeController extends RuntimeController { const { workerAccount, workerContext } = await getWorkerAccountAndContext(props); - return await createPreviewSession( - props.complianceConfig, - workerAccount, - workerContext, - this.#abortController.signal, - props.name + return await retryOnAPIFailure( + () => + createPreviewSession( + props.complianceConfig, + workerAccount, + workerContext, + this.#abortController.signal, + props.name + ), + undefined, + undefined, + this.#abortController.signal ); } catch (err: unknown) { if (err instanceof Error && err.name == "AbortError") { @@ -94,6 +101,9 @@ export class RemoteRuntimeController extends RuntimeController { if (!this.#session) { return; } + // Capture session in a local variable so TypeScript can narrow + // the type inside the retryOnAPIFailure closure below. + const session = this.#session; try { /* @@ -156,14 +166,20 @@ export class RemoteRuntimeController extends RuntimeController { if (props.bundleId !== this.#currentBundleId) { return; } - const workerPreviewToken = await createWorkerPreview( - props.complianceConfig, - init, - workerAccount, - workerContext, - this.#session, - this.#abortController.signal, - props.minimal_mode + const workerPreviewToken = await retryOnAPIFailure( + () => + createWorkerPreview( + props.complianceConfig, + init, + workerAccount, + workerContext, + session, + this.#abortController.signal, + props.minimal_mode + ), + undefined, + undefined, + this.#abortController.signal ); if (workerPreviewToken.tailUrl) { diff --git a/packages/wrangler/src/dev/create-worker-preview.ts b/packages/wrangler/src/dev/create-worker-preview.ts index 0698a5c1df..d566cc0072 100644 --- a/packages/wrangler/src/dev/create-worker-preview.ts +++ b/packages/wrangler/src/dev/create-worker-preview.ts @@ -15,6 +15,21 @@ import type { } from "@cloudflare/workers-utils"; import type { HeadersInit } from "undici"; +/** + * Maximum time (ms) to wait for an individual preview API request before + * treating it as a timeout. Without this, a hung API response blocks the + * entire dev-session reload indefinitely. + */ +const PREVIEW_API_TIMEOUT_MS = 30_000; + +/** + * Combine the caller's abort signal with a per-request timeout so that a + * hung Cloudflare API response doesn't block forever. + */ +function withTimeout(signal: AbortSignal): AbortSignal { + return AbortSignal.any([signal, AbortSignal.timeout(PREVIEW_API_TIMEOUT_MS)]); +} + /** * A Cloudflare account. */ @@ -187,10 +202,18 @@ export async function createPreviewSession( const { token, exchange_url } = await fetchResult<{ token: string; exchange_url?: string; - }>(complianceConfig, initUrl, undefined, undefined, abortSignal, apiToken); + }>( + complianceConfig, + initUrl, + undefined, + undefined, + withTimeout(abortSignal), + apiToken + ); const previewSessionToken = exchange_url - ? ((await tryExpandToken(exchange_url, ctx, abortSignal)) ?? token) + ? ((await tryExpandToken(exchange_url, ctx, withTimeout(abortSignal))) ?? + token) : token; try { @@ -200,7 +223,8 @@ export async function createPreviewSession( complianceConfig, account.accountId, undefined, - apiToken + apiToken, + withTimeout(abortSignal) ); host = `${name ?? crypto.randomUUID()}.${subdomain}`; } @@ -280,7 +304,7 @@ async function createPreviewToken( }, }, undefined, - abortSignal + withTimeout(abortSignal) ); return { diff --git a/packages/wrangler/src/routes.ts b/packages/wrangler/src/routes.ts index a09ff0189c..3c77022c20 100644 --- a/packages/wrangler/src/routes.ts +++ b/packages/wrangler/src/routes.ts @@ -17,7 +17,8 @@ export async function getWorkersDevSubdomain( complianceConfig: ComplianceConfig, accountId: string, configPath: string | undefined, - apiToken?: ApiCredentials + apiToken?: ApiCredentials, + abortSignal?: AbortSignal ): Promise { try { // note: API docs say that this field is "name", but they're lying. @@ -26,7 +27,7 @@ export async function getWorkersDevSubdomain( `/accounts/${accountId}/workers/subdomain`, undefined, undefined, - undefined, + abortSignal, apiToken ); return `${subdomain}${getComplianceRegionSubdomain(complianceConfig)}.workers.dev`; diff --git a/packages/wrangler/src/utils/retry.ts b/packages/wrangler/src/utils/retry.ts index a035ac8f8f..dfa2e1637a 100644 --- a/packages/wrangler/src/utils/retry.ts +++ b/packages/wrangler/src/utils/retry.ts @@ -16,7 +16,8 @@ const MAX_ATTEMPTS = 3; export async function retryOnAPIFailure( action: () => T | Promise, backoff = 0, - attempts = MAX_ATTEMPTS + attempts = MAX_ATTEMPTS, + abortSignal?: AbortSignal ): Promise { try { return await action(); @@ -25,6 +26,10 @@ export async function retryOnAPIFailure( if (!err.isRetryable()) { throw err; } + } else if (err instanceof DOMException && err.name === "TimeoutError") { + // Per-request timeouts (from AbortSignal.timeout()) are transient + // and should be retried, but user-initiated aborts (AbortError) + // should not. } else if (!(err instanceof TypeError)) { throw err; } @@ -36,11 +41,7 @@ export async function retryOnAPIFailure( throw err; } - await setTimeout(backoff); - return retryOnAPIFailure( - action, - backoff + (MAX_ATTEMPTS - attempts) * 1000, - attempts - 1 - ); + await setTimeout(backoff, undefined, { signal: abortSignal }); + return retryOnAPIFailure(action, backoff + 1000, attempts - 1, abortSignal); } } diff --git a/turbo.json b/turbo.json index c3b8bab6fb..5c8953fefa 100644 --- a/turbo.json +++ b/turbo.json @@ -54,7 +54,12 @@ "test:e2e": { "dependsOn": ["build"], "outputLogs": "new-only", - "env": ["CLOUDFLARE_ACCOUNT_ID", "CLOUDFLARE_API_TOKEN", "NODE_DEBUG"] + "env": [ + "CLOUDFLARE_ACCOUNT_ID", + "CLOUDFLARE_API_TOKEN", + "E2E_ACCOUNT_WORKERS_DEV_DOMAIN", + "NODE_DEBUG" + ] }, "//#check:format": { "cache": true