Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File based dev registry #5214

Merged
merged 9 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/yellow-coins-serve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

feat: Experimental file based service discovery when running multiple Wrangler instances locally. To try it out, make sure all your local Wrangler instances are running with the `--x-registry` flag.
3 changes: 1 addition & 2 deletions fixtures/ai-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ describe("'wrangler dev' correctly renders pages", () => {
expect((content as Record<string, object>).run).toEqual("function");
});

// TODO: unskip when https://github.com/cloudflare/workerd/pull/2095 is merged and released
it.skip("ai binding properties", async ({ expect }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

it("ai binding properties", async ({ expect }) => {
const response = await fetch(`http://${ip}:${port}/`);
const content = await response.json();
expect((content as Record<string, object>).binding).toEqual({
Expand Down
5 changes: 5 additions & 0 deletions fixtures/external-durable-objects-app/a/wrangler.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
name = "a"
compatibility_date = "2024-06-10"

[durable_objects]
bindings = [
{ name = "MY_DO", class_name = "MyDurableObject" }
]

[[migrations]]
tag = "v1"
new_classes = ["MyDurableObject"]
1 change: 1 addition & 0 deletions fixtures/external-durable-objects-app/b/wrangler.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
name = "b"
compatibility_date = "2024-06-10"

[durable_objects]
bindings = [
Expand Down
1 change: 1 addition & 0 deletions fixtures/external-durable-objects-app/c/wrangler.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
name = "c"
compatibility_date = "2024-06-10"

[durable_objects]
bindings = [
Expand Down
30 changes: 21 additions & 9 deletions fixtures/external-durable-objects-app/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { fork } from "child_process";
import * as path from "path";
import { setTimeout } from "timers/promises";
import { fetch } from "undici";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import { unstable_dev } from "wrangler";
Expand All @@ -8,7 +9,7 @@ import type { UnstableDevWorker } from "wrangler";

// TODO: reenable when https://github.com/cloudflare/workers-sdk/pull/4241 lands
// and improves reliability of this test.
describe.skip(
describe(
"Pages Functions",
() => {
let a: UnstableDevWorker;
Expand All @@ -26,15 +27,28 @@ describe.skip(
beforeAll(async () => {
a = await unstable_dev(path.join(__dirname, "../a/index.ts"), {
config: path.join(__dirname, "../a/wrangler.toml"),
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
});
await setTimeout(1000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the need for these pauses? If this is a shortcoming in the file-based reg approach, perhaps it is not ideal?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline. This is a limitation to how unstable_dev works right now, not related to this refactor.
The problem is that unstable_dev will only read the dev-registry once, rather than polling it for changes.
So we need to make sure that upstream workers are in-place and ready before starting downstream workers.
The wrangler dev command does do polling, so does not have this problem. So normal users would not need such pauses.

Going forward we should add polling or watching to the unstable_dev stuff so that these are no longer needed.

b = await unstable_dev(path.join(__dirname, "../b/index.ts"), {
config: path.join(__dirname, "../b/wrangler.toml"),
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
});

await setTimeout(1000);
c = await unstable_dev(path.join(__dirname, "../c/index.ts"), {
config: path.join(__dirname, "../c/wrangler.toml"),
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
});

await setTimeout(1000);
dWranglerProcess = fork(
path.join(
"..",
Expand All @@ -48,10 +62,10 @@ describe.skip(
[
"pages",
"dev",
"--x-registry",
"public",
"--compatibility-date=2024-03-04",
"--do=PAGES_REFERENCED_DO=MyDurableObject@a",
"--port=0",
"--inspector-port=0",
],
{
stdio: ["ignore", "ignore", "ignore", "ipc"],
Expand All @@ -63,6 +77,7 @@ describe.skip(
dPort = parsedMessage.port;
dResolveReadyPromise(undefined);
});
await setTimeout(1000);
});

afterAll(async () => {
Expand All @@ -85,10 +100,7 @@ describe.skip(

it("connects up Durable Objects and keeps state across wrangler instances", async () => {
await dReadyPromise;

// Service registry is polled every 300ms,
// so let's give all the Workers a little time to find each other
await new Promise((resolve) => setTimeout(resolve, 700));
await setTimeout(1000);

const responseA = await a.fetch(`/`, {
headers: {
Expand Down
37 changes: 34 additions & 3 deletions fixtures/external-service-bindings-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type WranglerInstance = {
port: string;
};

describe.skip("Pages Functions", () => {
describe("Pages Functions", () => {
let wranglerInstances: (WranglerInstance | UnstableDevWorker)[] = [];
let pagesAppPort: string;

Expand All @@ -38,34 +38,64 @@ describe.skip("Pages Functions", () => {
path.join(__dirname, "../module-worker-a/index.ts"),
{
config: path.join(__dirname, "../module-worker-a/wrangler.toml"),
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
}
);
await setTimeout(1000);

wranglerInstances[1] = await unstable_dev(
path.join(__dirname, "../module-worker-b/index.ts"),
{
config: path.join(__dirname, "../module-worker-b/wrangler.toml"),
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
}
);
await setTimeout(1000);

wranglerInstances[2] = await unstable_dev(
path.join(__dirname, "../service-worker-a/index.ts"),
{
config: path.join(__dirname, "../service-worker-a/wrangler.toml"),
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
}
);
await setTimeout(1000);

wranglerInstances[3] = await unstable_dev(
path.join(__dirname, "../module-worker-c/index.ts"),
{
config: path.join(__dirname, "../module-worker-c/wrangler.toml"),
env: "staging",
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
}
);
await setTimeout(1000);

wranglerInstances[4] = await unstable_dev(
path.join(__dirname, "../module-worker-d/index.ts"),
{
config: path.join(__dirname, "../module-worker-d/wrangler.toml"),
env: "production",
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
}
);
await setTimeout(1000);

wranglerInstances[5] = await getWranglerInstance({
pages: true,
dirName: "pages-functions-app",
Expand All @@ -79,6 +109,7 @@ describe.skip("Pages Functions", () => {
});

pagesAppPort = wranglerInstances[5].port;
await setTimeout(1000);
});

afterAll(async () => {
Expand Down Expand Up @@ -111,7 +142,7 @@ describe.skip("Pages Functions", () => {
expect(json).toMatchInlineSnapshot(`
{
"moduleWorkerCResponse": "Hello from module worker c (staging)",
"moduleWorkerDResponse": "You should start up wrangler dev --local on the STAGING_MODULE_D_SERVICE worker",
"moduleWorkerDResponse": "[wrangler] Couldn't find \`wrangler dev\` session for service "module-worker-d-staging" to proxy to",
}
`);
});
Expand All @@ -132,9 +163,9 @@ async function getWranglerInstance({
[
...(pages ? ["pages"] : []),
"dev",
"--x-registry",
...(pages ? ["public"] : ["index.ts"]),
"--local",
`--port=0`,
...extraArgs,
],
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
import { readdir } from "fs/promises";
import path from "path";
import { setTimeout } from "timers/promises";
import {
D1Database,
DurableObjectNamespace,
Fetcher,
R2Bucket,
} from "@cloudflare/workers-types";
import { beforeEach, describe, expect, it, vi } from "vitest";
import {
afterAll,
beforeAll,
beforeEach,
describe,
expect,
it,
vi,
} from "vitest";
import { unstable_dev } from "wrangler";
import { getBindingsProxy } from "./shared";
import type { KVNamespace } from "@cloudflare/workers-types";
Expand Down Expand Up @@ -36,18 +45,13 @@ describe("getBindingsProxy - bindings", () => {
vi.spyOn(console, "log").mockImplementation(() => {});
});

// Note: we're skipping the service workers and durable object tests
// so there's no need to start separate workers right now, the
// following beforeAll and afterAll should be un-commented when
// we reenable the tests

// beforeAll(async () => {
// devWorkers = await startWorkers();
// });
beforeAll(async () => {
devWorkers = await startWorkers();
});

// afterAll(async () => {
// await Promise.allSettled(devWorkers.map((i) => i.stop()));
// });
afterAll(async () => {
await Promise.allSettled(devWorkers.map((i) => i.stop()));
});

describe("var bindings", () => {
it("correctly obtains var bindings from both wrangler.toml and .dev.vars", async () => {
Expand Down Expand Up @@ -134,11 +138,10 @@ describe("getBindingsProxy - bindings", () => {
}
});

// Note: the following test is skipped due to flakiness caused by the local registry not working reliably
// when we run all our fixtures together (possibly because of race condition issues)
it.skip("provides service bindings to external local workers", async () => {
it("provides service bindings to external local workers", async () => {
const { bindings, dispose } = await getBindingsProxy<Bindings>({
configPath: wranglerTomlFilePath,
experimentalRegistry: true,
});
try {
const { MY_SERVICE_A, MY_SERVICE_B } = bindings;
Expand All @@ -164,11 +167,10 @@ describe("getBindingsProxy - bindings", () => {
await dispose();
});

// Note: the following test is skipped due to flakiness caused by the local registry not working reliably
// when we run all our fixtures together (possibly because of race condition issues)
it.skip("correctly obtains functioning DO bindings (provided by external local workers)", async () => {
it("correctly obtains functioning DO bindings (provided by external local workers)", async () => {
const { bindings, dispose } = await getBindingsProxy<Bindings>({
configPath: wranglerTomlFilePath,
experimentalRegistry: true,
});
try {
const { MY_DO_A, MY_DO_B } = bindings;
Expand Down Expand Up @@ -235,11 +237,16 @@ async function startWorkers(): Promise<UnstableDevWorker[]> {
const workersDirPath = path.join(__dirname, "..", "workers");
const workers = await readdir(workersDirPath);
return await Promise.all(
workers.map((workerName) => {
workers.map(async (workerName) => {
const workerPath = path.join(workersDirPath, workerName);
await setTimeout(2000);
return unstable_dev(path.join(workerPath, "index.ts"), {
config: path.join(workerPath, "wrangler.toml"),
ip: "127.0.0.1",
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
});
})
);
Expand Down
19 changes: 12 additions & 7 deletions fixtures/service-bindings-app/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,30 @@
import path from "node:path";
import { setTimeout } from "node:timers/promises";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import { unstable_dev, UnstableDevWorker } from "wrangler";

// TODO: reenable when https://github.com/cloudflare/workers-sdk/pull/4241 lands
// and improves reliability of this test.
describe.skip("Service Bindings", () => {
describe("Service Bindings", () => {
let aWorker: UnstableDevWorker;

let bWorker: UnstableDevWorker;

beforeAll(async () => {
bWorker = await unstable_dev(path.join(__dirname, "../b/index.ts"), {
config: path.join(__dirname, "../b/wrangler.toml"),
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
});
await setTimeout(1000);
aWorker = await unstable_dev(path.join(__dirname, "../a/index.ts"), {
config: path.join(__dirname, "../a/wrangler.toml"),
experimental: {
fileBasedRegistry: true,
disableExperimentalWarning: true,
},
});
// Service registry is polled every 300ms,
// so let's give worker A some time to find B

await new Promise((resolve) => setTimeout(resolve, 700));
await setTimeout(1000);
});

it("connects up Durable Objects and keeps state across wrangler instances", async () => {
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"@esbuild-plugins/node-modules-polyfill": "^0.2.2",
"blake3-wasm": "^2.1.5",
"chokidar": "^3.5.3",
"date-fns": "^3.6.0",
"esbuild": "0.17.19",
"miniflare": "workspace:*",
"nanoid": "^3.3.3",
Expand Down
Loading
Loading