diff --git a/fixtures/dev-registry/tests/dev-registry.test.ts b/fixtures/dev-registry/tests/dev-registry.test.ts index c60c9e7ac41..bd12900743b 100644 --- a/fixtures/dev-registry/tests/dev-registry.test.ts +++ b/fixtures/dev-registry/tests/dev-registry.test.ts @@ -4,7 +4,7 @@ import * as path from "node:path"; import { resolve } from "node:path"; /* eslint-disable workers-sdk/no-vitest-import-expect -- uses expect in module-scope helper functions */ import { - describe as baseDescribe, + describe, expect, onTestFailed, onTestFinished, @@ -18,11 +18,6 @@ import { } from "../../../packages/vite-plugin-cloudflare/e2e/helpers"; import { runWranglerDev as baseRunWranglerDev } from "../../shared/src/run-wrangler-long-lived"; -// TODO: These tests are consistently failing on Windows in CI and are blocking -// other work. Skipping them there as a temporary measure until the underlying -// issue is fixed. There's still value in running them on macOS and Linux. -const describe = baseDescribe.skipIf(process.platform === "win32"); - const waitForTimeout = 20_000; const cwd = resolve(__dirname, ".."); const tmpPathBase = path.join(os.tmpdir(), "wrangler-tests"); @@ -32,8 +27,18 @@ const it = test.extend<{ // Fixture for creating a temporary directory async devRegistryPath({}, use) { const tmpPath = await fs.realpath(await fs.mkdtemp(tmpPathBase)); + + // Fixture teardown runs *before* `onTestFinished` callbacks, so removing + // the directory here would pull the registry out from under dev sessions + // that are still running. Registering the cleanup as an + // `onTestFinished` callback during fixture setup instead makes it the + // first one registered, and therefore the last one to run under Vitest's + // LIFO ordering — after every dev session has exited. + onTestFinished(async () => { + await fs.rm(tmpPath, { recursive: true, maxRetries: 10 }); + }); + await use(tmpPath); - await fs.rm(tmpPath, { recursive: true, maxRetries: 10 }); }, }); @@ -94,6 +99,27 @@ async function runWranglerDev( return url; } +/** + * Starts a tail consumer, then its producer, returning both URLs. + * + * Vitest runs `onTestFinished` callbacks in LIFO order, so the session started + * last is torn down first. Starting the producer last therefore guarantees it + * is killed while its consumer is still running. + * + * The reverse order is not safe: killing a dev session that another running + * session is forwarding tail events to aborts workerd on the surviving side on + * Windows, which restarts the dev server mid-teardown and times the test out. + * The `tail_consumers` in this fixture are one-directional for the same reason + * — with a cycle there is no order that keeps every producer shorter-lived than + * its consumer. + */ +async function startTailPair( + startConsumer: () => Promise, + startProducer: () => Promise +): Promise<[consumer: string, producer: string]> { + return [await startConsumer(), await startProducer()]; +} + async function setupPlatformProxy(config: string, devRegistryPath?: string) { vi.stubEnv("WRANGLER_REGISTRY_PATH", devRegistryPath); @@ -400,17 +426,25 @@ describe("Dev Registry: wrangler dev <-> wrangler dev", () => { }, waitForTimeout); }); - it("supports tail handler", async ({ devRegistryPath }) => { - const exportedHandlerWithAssets = await runWranglerDev( - "wrangler.exported-handler-with-assets.jsonc", - devRegistryPath - ); - const workerEntrypoint = await runWranglerDev( - [ - "wrangler.worker-entrypoint.jsonc", - "wrangler.internal-durable-object.jsonc", - ], - devRegistryPath + it("supports tail handler when the consumer has assets", async ({ + devRegistryPath, + }) => { + // The producer runs alongside a second worker so that its logs are + // prefixed with the worker name, exercising multi-worker sessions too + const [exportedHandlerWithAssets, workerEntrypoint] = await startTailPair( + () => + runWranglerDev( + "wrangler.exported-handler-with-assets.jsonc", + devRegistryPath + ), + () => + runWranglerDev( + [ + "wrangler.worker-entrypoint.jsonc", + "wrangler.internal-durable-object.jsonc", + ], + devRegistryPath + ) ); const searchParams = new URLSearchParams({ @@ -418,29 +452,7 @@ describe("Dev Registry: wrangler dev <-> wrangler dev", () => { }); await vi.waitFor(async () => { - // Trigger tail handler of worker-entrypoint via exported handler - await fetch(`${exportedHandlerWithAssets}?${searchParams}`, { - method: "POST", - body: JSON.stringify(["hello world", "this is the 2nd log"]), - }); - await fetch(`${exportedHandlerWithAssets}?${searchParams}`, { - method: "POST", - body: JSON.stringify(["some other log"]), - }); - - const response = await fetch(`${workerEntrypoint}?${searchParams}`); - - expect(await response.json()).toEqual({ - worker: "Worker Entrypoint", - tailEvents: expect.arrayContaining([ - [["[exported-handler]"], ["hello world", "this is the 2nd log"]], - [["[exported-handler]"], ["some other log"]], - ]), - }); - }, waitForTimeout); - - await vi.waitFor(async () => { - // Trigger tail handler of exported-handler via worker-entrypoint + // Trigger tail handler of exported-handler-with-assets via worker-entrypoint await fetch(`${workerEntrypoint}?${searchParams}`, { method: "POST", body: JSON.stringify(["hello from test"]), @@ -469,6 +481,45 @@ describe("Dev Registry: wrangler dev <-> wrangler dev", () => { }, waitForTimeout); }); + it("supports tail handler when the producer has assets", async ({ + devRegistryPath, + }) => { + const [exportedHandler, exportedHandlerWithAssets] = await startTailPair( + () => runWranglerDev("wrangler.exported-handler.jsonc", devRegistryPath), + () => + runWranglerDev( + "wrangler.exported-handler-with-assets.jsonc", + devRegistryPath + ) + ); + + const searchParams = new URLSearchParams({ + "test-method": "tail", + }); + + await vi.waitFor(async () => { + // Trigger tail handler of exported-handler via exported-handler-with-assets + await fetch(`${exportedHandlerWithAssets}?${searchParams}`, { + method: "POST", + body: JSON.stringify(["hello world", "this is the 2nd log"]), + }); + await fetch(`${exportedHandlerWithAssets}?${searchParams}`, { + method: "POST", + body: JSON.stringify(["some other log"]), + }); + + const response = await fetch(`${exportedHandler}?${searchParams}`); + + expect(await response.json()).toEqual({ + worker: "exported-handler", + tailEvents: expect.arrayContaining([ + [["[exported-handler]"], ["hello world", "this is the 2nd log"]], + [["[exported-handler]"], ["some other log"]], + ]), + }); + }, waitForTimeout); + }); + it("supports queues across dev sessions", async ({ devRegistryPath }) => { const exportedHandler = await runWranglerDev( "wrangler.exported-handler.jsonc", @@ -715,14 +766,16 @@ describe("Dev Registry: vite dev <-> vite dev", () => { }, waitForTimeout); }); - it("supports tail handler", async ({ devRegistryPath }) => { - const exportedHandler = await runViteDev( - "vite.exported-handler.config.ts", - devRegistryPath - ); - const workerEntrypointWithAssets = await runViteDev( - "vite.worker-entrypoint-with-assets.config.ts", - devRegistryPath + it("supports tail handler when the consumer has assets", async ({ + devRegistryPath, + }) => { + const [exportedHandlerWithAssets, workerEntrypoint] = await startTailPair( + () => + runViteDev( + "vite.exported-handler-with-assets.config.ts", + devRegistryPath + ), + () => runViteDev("vite.worker-entrypoint.config.ts", devRegistryPath) ); const searchParams = new URLSearchParams({ @@ -730,38 +783,55 @@ describe("Dev Registry: vite dev <-> vite dev", () => { }); await vi.waitFor(async () => { - // Trigger tail handler of worker-entrypoint via exported-handler - await fetch(`${exportedHandler}?${searchParams}`, { + // Trigger tail handler of exported-handler-with-assets via worker-entrypoint + await fetch(`${workerEntrypoint}?${searchParams}`, { method: "POST", - body: JSON.stringify(["hello world", "this is the 2nd log"]), + body: JSON.stringify(["hello from test"]), }); - await fetch(`${exportedHandler}?${searchParams}`, { + await fetch(`${workerEntrypoint}?${searchParams}`, { method: "POST", - body: JSON.stringify(["some other log"]), + body: JSON.stringify(["yet another log", "and another one"]), }); const response = await fetch( - `${workerEntrypointWithAssets}?${searchParams}` + `${exportedHandlerWithAssets}?${searchParams}` ); expect(await response.json()).toEqual({ - worker: "Worker Entrypoint", + worker: "exported-handler", tailEvents: expect.arrayContaining([ - [["[exported-handler]"], ["hello world", "this is the 2nd log"]], - [["[exported-handler]"], ["some other log"]], + [["[Worker Entrypoint]"], ["hello from test"]], + [["[Worker Entrypoint]"], ["yet another log", "and another one"]], ]), }); }, waitForTimeout); + }); + + it("supports tail handler when the producer has assets", async ({ + devRegistryPath, + }) => { + const [exportedHandler, exportedHandlerWithAssets] = await startTailPair( + () => runViteDev("vite.exported-handler.config.ts", devRegistryPath), + () => + runViteDev( + "vite.exported-handler-with-assets.config.ts", + devRegistryPath + ) + ); + + const searchParams = new URLSearchParams({ + "test-method": "tail", + }); await vi.waitFor(async () => { - // Trigger tail handler of exported-handler via worker-entrypoint - await fetch(`${workerEntrypointWithAssets}?${searchParams}`, { + // Trigger tail handler of exported-handler via exported-handler-with-assets + await fetch(`${exportedHandlerWithAssets}?${searchParams}`, { method: "POST", - body: JSON.stringify(["hello from test"]), + body: JSON.stringify(["hello world", "this is the 2nd log"]), }); - await fetch(`${workerEntrypointWithAssets}?${searchParams}`, { + await fetch(`${exportedHandlerWithAssets}?${searchParams}`, { method: "POST", - body: JSON.stringify(["yet another log", "and another one"]), + body: JSON.stringify(["some other log"]), }); const response = await fetch(`${exportedHandler}?${searchParams}`); @@ -769,8 +839,8 @@ describe("Dev Registry: vite dev <-> vite dev", () => { expect(await response.json()).toEqual({ worker: "exported-handler", tailEvents: expect.arrayContaining([ - [["[Worker Entrypoint]"], ["hello from test"]], - [["[Worker Entrypoint]"], ["yet another log", "and another one"]], + [["[exported-handler]"], ["hello world", "this is the 2nd log"]], + [["[exported-handler]"], ["some other log"]], ]), }); }, waitForTimeout); @@ -977,14 +1047,16 @@ describe("Dev Registry: vite dev <-> wrangler dev", () => { }, waitForTimeout); }); - it("supports tail handler", async ({ devRegistryPath }) => { - const exportedHandlerWithStaticAssets = await runViteDev( - "vite.exported-handler-with-assets.config.ts", - devRegistryPath - ); - const workerEntrypoint = await runWranglerDev( - "wrangler.worker-entrypoint.jsonc", - devRegistryPath + it("supports tail handler from wrangler dev to vite dev", async ({ + devRegistryPath, + }) => { + const [exportedHandlerWithAssets, workerEntrypoint] = await startTailPair( + () => + runViteDev( + "vite.exported-handler-with-assets.config.ts", + devRegistryPath + ), + () => runWranglerDev("wrangler.worker-entrypoint.jsonc", devRegistryPath) ); const searchParams = new URLSearchParams({ @@ -992,47 +1064,64 @@ describe("Dev Registry: vite dev <-> wrangler dev", () => { }); await vi.waitFor(async () => { - // Trigger tail handler of worker-entrypoint via exported-handler - await fetch(`${exportedHandlerWithStaticAssets}?${searchParams}`, { + // Trigger tail handler of exported-handler-with-assets via worker-entrypoint + await fetch(`${workerEntrypoint}?${searchParams}`, { method: "POST", - body: JSON.stringify(["hello world", "this is the 2nd log"]), + body: JSON.stringify(["hello from test"]), }); - await fetch(`${exportedHandlerWithStaticAssets}?${searchParams}`, { + await fetch(`${workerEntrypoint}?${searchParams}`, { method: "POST", - body: JSON.stringify(["some other log"]), + body: JSON.stringify(["yet another log", "and another one"]), }); - const response = await fetch(`${workerEntrypoint}?${searchParams}`); + const response = await fetch( + `${exportedHandlerWithAssets}?${searchParams}` + ); expect(await response.json()).toEqual({ - worker: "Worker Entrypoint", + worker: "exported-handler", tailEvents: expect.arrayContaining([ - [["[exported-handler]"], ["hello world", "this is the 2nd log"]], - [["[exported-handler]"], ["some other log"]], + [["[Worker Entrypoint]"], ["hello from test"]], + [["[Worker Entrypoint]"], ["yet another log", "and another one"]], ]), }); }, waitForTimeout); + }); + + it("supports tail handler from vite dev to wrangler dev", async ({ + devRegistryPath, + }) => { + const [exportedHandler, exportedHandlerWithAssets] = await startTailPair( + () => runWranglerDev("wrangler.exported-handler.jsonc", devRegistryPath), + () => + runViteDev( + "vite.exported-handler-with-assets.config.ts", + devRegistryPath + ) + ); + + const searchParams = new URLSearchParams({ + "test-method": "tail", + }); await vi.waitFor(async () => { - // Trigger tail handler of exported-handler via worker-entrypoint - await fetch(`${workerEntrypoint}?${searchParams}`, { + // Trigger tail handler of exported-handler via exported-handler-with-assets + await fetch(`${exportedHandlerWithAssets}?${searchParams}`, { method: "POST", - body: JSON.stringify(["hello from test"]), + body: JSON.stringify(["hello world", "this is the 2nd log"]), }); - await fetch(`${workerEntrypoint}?${searchParams}`, { + await fetch(`${exportedHandlerWithAssets}?${searchParams}`, { method: "POST", - body: JSON.stringify(["yet another log", "and another one"]), + body: JSON.stringify(["some other log"]), }); - const response = await fetch( - `${exportedHandlerWithStaticAssets}?${searchParams}` - ); + const response = await fetch(`${exportedHandler}?${searchParams}`); expect(await response.json()).toEqual({ worker: "exported-handler", tailEvents: expect.arrayContaining([ - [["[Worker Entrypoint]"], ["hello from test"]], - [["[Worker Entrypoint]"], ["yet another log", "and another one"]], + [["[exported-handler]"], ["hello world", "this is the 2nd log"]], + [["[exported-handler]"], ["some other log"]], ]), }); }, waitForTimeout); diff --git a/fixtures/dev-registry/wrangler.exported-handler-with-assets.jsonc b/fixtures/dev-registry/wrangler.exported-handler-with-assets.jsonc index 322ca0243e2..d41abdc6e15 100644 --- a/fixtures/dev-registry/wrangler.exported-handler-with-assets.jsonc +++ b/fixtures/dev-registry/wrangler.exported-handler-with-assets.jsonc @@ -34,9 +34,12 @@ "entrypoint": "NamedEntrypoint", }, ], + // Middle link of the one-directional tail chain described in + // wrangler.worker-entrypoint.jsonc. Pointing this back at worker-entrypoint + // would close the cycle. "tail_consumers": [ { - "service": "worker-entrypoint", + "service": "exported-handler", }, ], } diff --git a/fixtures/dev-registry/wrangler.exported-handler.jsonc b/fixtures/dev-registry/wrangler.exported-handler.jsonc index f8af4671b3e..1cf153dfbe1 100644 --- a/fixtures/dev-registry/wrangler.exported-handler.jsonc +++ b/fixtures/dev-registry/wrangler.exported-handler.jsonc @@ -27,11 +27,6 @@ "entrypoint": "NamedEntrypoint", }, ], - "tail_consumers": [ - { - "service": "worker-entrypoint-with-assets", - }, - ], "queues": { "producers": [ { diff --git a/fixtures/dev-registry/wrangler.external-durable-object.jsonc b/fixtures/dev-registry/wrangler.external-durable-object.jsonc index c74e28cbb7a..43dd46d96c4 100644 --- a/fixtures/dev-registry/wrangler.external-durable-object.jsonc +++ b/fixtures/dev-registry/wrangler.external-durable-object.jsonc @@ -12,9 +12,4 @@ }, ], }, - "tail_consumers": [ - { - "service": "exported-handler", - }, - ], } diff --git a/fixtures/dev-registry/wrangler.worker-entrypoint-with-assets.jsonc b/fixtures/dev-registry/wrangler.worker-entrypoint-with-assets.jsonc index 77b873a3924..d51c286fd06 100644 --- a/fixtures/dev-registry/wrangler.worker-entrypoint-with-assets.jsonc +++ b/fixtures/dev-registry/wrangler.worker-entrypoint-with-assets.jsonc @@ -29,9 +29,4 @@ "entrypoint": "NamedEntrypoint", }, ], - "tail_consumers": [ - { - "service": "exported-handler", - }, - ], } diff --git a/fixtures/dev-registry/wrangler.worker-entrypoint.jsonc b/fixtures/dev-registry/wrangler.worker-entrypoint.jsonc index 61ddb2eeb05..1a94c73f2ef 100644 --- a/fixtures/dev-registry/wrangler.worker-entrypoint.jsonc +++ b/fixtures/dev-registry/wrangler.worker-entrypoint.jsonc @@ -26,6 +26,11 @@ "entrypoint": "NamedEntrypoint", }, ], + // Tail relationships across these configs form a one-directional chain: + // worker-entrypoint -> exported-handler-with-assets -> exported-handler. + // Keep it acyclic and confined to the tail tests. A dev session that + // outlives a tail consumer it has already connected to aborts workerd on + // Windows, and a cycle makes a safe shutdown order impossible to pick. "tail_consumers": [ { "service": "exported-handler-with-assets", diff --git a/fixtures/shared/src/run-wrangler-long-lived.ts b/fixtures/shared/src/run-wrangler-long-lived.ts index 8e4c8e190b6..cf342ae4d9f 100644 --- a/fixtures/shared/src/run-wrangler-long-lived.ts +++ b/fixtures/shared/src/run-wrangler-long-lived.ts @@ -189,23 +189,17 @@ async function runLongLivedWrangler( async function stop() { stopping = true; - return new Promise((resolve) => { - if (processExited) { - // Already dead — nothing to kill. Avoid noisy Windows taskkill errors. - resolve(); - return; - } - assert( - wranglerProcess.pid, - `Command "${command.join(" ")}" had no process id` - ); - treeKill(wranglerProcess.pid, (e) => { + if (processExited) { + // Already dead — nothing to kill. Avoid noisy Windows taskkill errors. + return; + } + const pid = wranglerProcess.pid; + assert(pid, `Command "${command.join(" ")}" had no process id`); + + await new Promise((resolve) => { + treeKill(pid, (e) => { if (e) { - console.error( - "Failed to kill command: " + command.join(" "), - wranglerProcess.pid, - e - ); + console.error("Failed to kill command: " + command.join(" "), pid, e); } // fallthrough to resolve() because either the process is already dead // or don't have permission to kill it or some other reason? @@ -213,6 +207,22 @@ async function runLongLivedWrangler( resolve(); }); }); + + // The kill callback only tells us the signal was delivered (on Windows it + // is the exit of `taskkill`), not that the process is gone. Tests that + // stop several sessions in sequence rely on each one being fully dead + // before the next is stopped, so wait for the actual exit — with a bound, + // since failing to reap a child should not fail the test. + if (processExited) { + return; + } + await new Promise((resolve) => { + const timeoutHandle = setTimeout(resolve, 10_000); + wranglerProcess.once("exit", () => { + clearTimeout(timeoutHandle); + resolve(); + }); + }); } const { ip, port } = await ready;