Skip to content

Commit

Permalink
Revert "Revert "startDevWorker - Milestone 1" (#4171)"
Browse files Browse the repository at this point in the history
This reverts commit 88f15f6.
  • Loading branch information
RamIdeas authored and penalosa committed Nov 23, 2023
1 parent beed157 commit 8a3f423
Show file tree
Hide file tree
Showing 14 changed files with 351 additions and 418 deletions.
2 changes: 1 addition & 1 deletion fixtures/dev-env/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"@types/ws": "^8.5.7",
"@cloudflare/workers-tsconfig": "workspace:^",
"get-port": "^7.0.0",
"miniflare": "3.20231025.1",
"miniflare": "3.20231002.1",
"undici": "^5.23.0",
"wrangler": "workspace:*",
"ws": "^8.14.2"
Expand Down
18 changes: 7 additions & 11 deletions fixtures/dev-env/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,18 @@ const fakeBundle = {} as EsbuildBundle;

let devEnv: DevEnv;
let mf: Miniflare | undefined;
let res: MiniflareResponse | undici.Response;
let res: MiniflareResponse | undici.Response | undefined;
let ws: WebSocket | undefined;
let fireAndForgetPromises: Promise<any>[] = [];

type OptionalKeys<T, K extends keyof T> = Omit<T, K> & Partial<Pick<T, K>>;

beforeEach(() => {
devEnv = new DevEnv();
mf = undefined;
res = undefined as any;
res = undefined;
ws = undefined;
});
afterEach(async () => {
await Promise.allSettled(fireAndForgetPromises);
await devEnv?.teardown();
await mf?.dispose();
await ws?.close();
Expand All @@ -52,7 +50,7 @@ async function fakeStartUserWorker(options: {
};
const mfOpts: MiniflareOptions = Object.assign(
{
port: undefined,
port: 0,
inspectorPort: 0,
modules: true,
compatibilityDate: "2023-08-01",
Expand All @@ -69,11 +67,10 @@ async function fakeStartUserWorker(options: {
fakeReloadStart(config);

const worker = devEnv.startWorker(config);
const { proxyWorker } = await devEnv.proxy.ready.promise;
const { proxyWorker, inspectorProxyWorker } = await devEnv.proxy.ready
.promise;
const proxyWorkerUrl = await proxyWorker.ready;
const inspectorProxyWorkerUrl = await proxyWorker.unsafeGetDirectURL(
"InspectorProxyWorker"
);
const inspectorProxyWorkerUrl = await inspectorProxyWorker.ready;

mf = new Miniflare(mfOpts);

Expand Down Expand Up @@ -138,8 +135,7 @@ function fireAndForgetFakeUserWorkerChanges(
...args: Parameters<typeof fakeUserWorkerChanges>
) {
// fire and forget the reload -- this let's us test request buffering
const promise = fakeUserWorkerChanges(...args);
fireAndForgetPromises.push(promise);
void fakeUserWorkerChanges(...args);
}

function fakeConfigUpdate(config: StartDevWorkerOptions) {
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"emit-types": "tsc -p tsconfig.emit.json && node -r esbuild-register scripts/emit-types.ts",
"prepublishOnly": "SOURCEMAPS=false npm run build",
"start": "pnpm run bundle && cross-env NODE_OPTIONS=--enable-source-maps ./bin/wrangler.js",
"test": "pnpm run assert-git-version && jest",
"test": "pnpm run assert-git-version && jest --runInBand",
"test:ci": "pnpm run test --coverage",
"test:debug": "pnpm run test --silent=false --verbose=true",
"test:e2e": "vitest --test-timeout 240000 --single-thread --dir ./e2e --retry 2 run",
Expand Down
71 changes: 17 additions & 54 deletions packages/wrangler/src/api/startDevWorker/DevEnv.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from "node:assert";
import { EventEmitter } from "node:events";
import { fetch, Request, type RequestInit } from "miniflare";
import { logger } from "../../logger";
import { BundlerController } from "./BundlerController";
import { ConfigController } from "./ConfigController";
Expand Down Expand Up @@ -47,14 +47,8 @@ export class DevEnv extends EventEmitter {
controller.on("error", (event) => this.emitErrorEvent(event))
);

this.on("error", (event: ErrorEvent) => {
// TODO: when we're are comfortable with StartDevWorker/DevEnv stability,
// we can remove this handler and let the user handle the unknowable errors
// or let the process crash. For now, log them to stderr
// so we can identify knowable vs unknowable error candidates

logger.error(`Error in ${event.source}: ${event.reason}\n`, event.cause);
logger.debug("=> Error contextual data:", event.data);
this.on("error", (event) => {
logger.error(event);
});

config.on("configUpdate", (event) => {
Expand Down Expand Up @@ -105,48 +99,8 @@ export class DevEnv extends EventEmitter {
]);
}

emitErrorEvent(ev: ErrorEvent) {
if (
ev.source === "ProxyController" &&
ev.reason === "Failed to start ProxyWorker or InspectorProxyWorker"
) {
assert(ev.data.config); // we must already have a `config` if we've already tried (and failed) to instantiate the ProxyWorker(s)

const { config } = ev.data;
const port = config.dev?.server?.port;
const inspectorPort = config.dev?.inspector?.port;
const randomPorts = [0, undefined];

// console.log({ port, inspectorPort, ev });
if (!randomPorts.includes(port) || !randomPorts.includes(inspectorPort)) {
// emit the event here while the ConfigController is unimplemented
// this will cause the ProxyController to try reinstantiating the ProxyWorker(s)
// TODO: change this to `this.config.updateOptions({ dev: { server: { port: 0 }, inspector: { port: 0 } } });` when the ConfigController is implemented
this.config.emitConfigUpdateEvent({
type: "configUpdate",
config: {
...config,
dev: {
...config.dev,
server: { ...config.dev?.server, port: 0 }, // override port
inspector: { ...config.dev?.inspector, port: 0 }, // override port
},
},
});
}
} else if (
ev.source === "ProxyController" &&
(ev.reason.startsWith("Failed to send message to") ||
ev.reason.startsWith("Could not connect to InspectorProxyWorker"))
) {
logger.debug(`Error in ${ev.source}: ${ev.reason}\n`, ev.cause);
logger.debug("=> Error contextual data:", ev.data);
}
// if other knowable + recoverable errors occur, handle them here
else {
// otherwise, re-emit the unknowable errors to the top-level
this.emit("error", ev);
}
emitErrorEvent(data: ErrorEvent) {
this.emit("error", data);
}
}

Expand All @@ -166,9 +120,18 @@ export function createWorkerObject(devEnv: DevEnv): DevWorker {
},
async fetch(...args) {
const { proxyWorker } = await devEnv.proxy.ready.promise;
await devEnv.proxy.runtimeMessageMutex.drained();

return proxyWorker.dispatchFetch(...args);
// return proxyWorker.dispatchFetch(...args);
// ^ bug: Miniflare#dispatchFetch uses one HTTP/1.1 connection, preventing parallel requests (pause/play requests + buffered eyeball requests)
// workaround: use undici.fetch
const proxyWorkerUrl = await proxyWorker.ready;
const req = new Request(...args);
const url = new URL(req.url);
url.protocol = proxyWorkerUrl.protocol;
url.hostname = proxyWorkerUrl.hostname;
url.port = proxyWorkerUrl.port;
// /workaround

return fetch(url, req as RequestInit);
},
async queue(..._args) {
// const { worker } = await devEnv.proxy.ready;
Expand Down
Loading

0 comments on commit 8a3f423

Please sign in to comment.