Skip to content

Commit

Permalink
Fix local registry being able to start only once in unstable_dev (#2044)
Browse files Browse the repository at this point in the history
* Fix local registry being able to start only once in unstable_dev

When using `unstable_dev`, it's not possible to start multiple workers in a sequence. Once you stop one worker, the local registry is closed and cannot be started again. This fixed that by clearing the `server` variable once the server is stopped

* Add a test for the fix

* Add changeset

* Move tests to Wrangler unit tests

---------

Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
  • Loading branch information
kuba-orlik and petebacondarwin committed Apr 3, 2023
1 parent c0c447e commit eebad0d
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 65 deletions.
5 changes: 5 additions & 0 deletions .changeset/purple-books-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

fix: allow programmatic dev workers to be stopped and started in a single session
64 changes: 0 additions & 64 deletions packages/wrangler/src/__tests__/api-devregistry.test.js

This file was deleted.

121 changes: 121 additions & 0 deletions packages/wrangler/src/__tests__/api-devregistry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import { fetch } from "undici";
import { unstable_dev } from "../api";

jest.unmock("undici");

/**
* a huge caveat to how testing multi-worker scripts works:
* you can't shutdown the first worker you spun up, or it'll kill the devRegistry
*/
describe("multi-worker testing", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let childWorker: any;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let parentWorker: any;

beforeAll(async () => {
childWorker = await unstable_dev(
"src/__tests__/helpers/worker-scripts/hello-world-worker.js",
{
config: "src/__tests__/helpers/worker-scripts/child-wrangler.toml",
experimental: {
disableExperimentalWarning: true,
},
}
);
parentWorker = await unstable_dev(
"src/__tests__/helpers/worker-scripts/parent-worker.js",
{
config: "src/__tests__/helpers/worker-scripts/parent-wrangler.toml",
experimental: {
disableExperimentalWarning: true,
},
}
);
});

afterAll(async () => {
await childWorker.stop();
await parentWorker.stop();
});

it("parentWorker and childWorker should be added devRegistry", async () => {
const resp = await fetch("http://localhost:6284/workers");
if (resp) {
const parsedResp = (await resp.json()) as {
parent: unknown;
child: unknown;
};
expect(parsedResp.parent).toBeTruthy();
expect(parsedResp.child).toBeTruthy();
}
});

it("childWorker should return Hello World itself", async () => {
const resp = await childWorker.fetch();
if (resp) {
const text = await resp.text();
expect(text).toMatchInlineSnapshot(`"Hello World!"`);
}
});

it("parentWorker should return Hello World by invoking the child worker", async () => {
const resp = await parentWorker.fetch();
if (resp) {
const parsedResp = await resp.text();
expect(parsedResp).toEqual("Parent worker sees: Hello World!");
}
});

it("should be able to stop and start the server with no warning logs", async () => {
// Spy on all the console methods
let logs = "";
(["debug", "info", "log", "warn", "error"] as const).forEach((method) =>
jest
.spyOn(console, method)
.mockImplementation((...args: unknown[]) => (logs += `\n${args}`))
);

// Spy on the std out that is written to by Miniflare 2
jest
.spyOn(process.stdout, "write")
.mockImplementation((chunk: unknown) => ((logs += `\n${chunk}`), true));

async function startWorker() {
return await unstable_dev(
"src/__tests__/helpers/worker-scripts/hello-world-worker.js",
{
// We need the wrangler.toml config to specify a Worker name
// otherwise unstable_dev will not register the worker with the DevRegistry
config: "src/__tests__/helpers/worker-scripts/child-wrangler.toml",
// We need debug logs because this is where the message is written if registering the worker fails.
logLevel: "debug",
experimental: {
disableExperimentalWarning: true,
},
}
);
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
let worker: any;
try {
worker = await startWorker();

// Stop the worker and start it again
await worker.stop();
await new Promise((r) => setTimeout(r, 2000));

worker = await startWorker();

const resp = await worker.fetch();
expect(resp).not.toBe(undefined);

expect(logs).not.toMatch(
/Failed to register worker in local service registry/
);
} finally {
await worker?.stop();
}
}, 10000);
});
3 changes: 2 additions & 1 deletion packages/wrangler/src/dev-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { HttpTerminator } from "http-terminator";
const DEV_REGISTRY_PORT = "6284";
const DEV_REGISTRY_HOST = `http://localhost:${DEV_REGISTRY_PORT}`;

let server: Server;
let server: Server | null;
let terminator: HttpTerminator;

export type WorkerRegistry = Record<string, WorkerDefinition>;
Expand Down Expand Up @@ -102,6 +102,7 @@ export async function startWorkerRegistry() {
*/
export async function stopWorkerRegistry() {
await terminator?.terminate();
server = null;
}

/**
Expand Down

0 comments on commit eebad0d

Please sign in to comment.