allow startWorker to accept false as an inspector option (to disable the inspector server) (#9550)

Author: dario-piotrowicz

.changeset/true-cities-try.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+allow `startWorker` to accept `false` as an `inspector` option (to disable the inspector server)

fixtures/start-worker-node-test/src/config-errors.test.js

@@ -54,7 +54,7 @@ describe("startWorker - configuration errors", () => {
 				server: {
 					port: await getPort(),
 				},
-				inspector: { port: await getPort() },
+				inspector: false,
 			},
 		});
 
@@ -80,7 +80,7 @@ describe("startWorker - configuration errors", () => {
 				server: {
 					port: await getPort(),
 				},
-				inspector: { port: await getPort() },
+				inspector: false,
 			},
 		});
 

packages/wrangler/e2e/startWorker.test.ts

@@ -108,10 +108,12 @@ describe.each(OPTIONS)("DevEnv (remote: $remote)", ({ remote }) => {
 		});
 
 		const inspectorUrl = await worker.inspectorUrl;
+		assert(inspectorUrl, "missing inspectorUrl");
 		const res = await undici.fetch(`http://${inspectorUrl.host}/json`);
 
 		await expect(res.json()).resolves.toBeInstanceOf(Array);
 
+		assert(inspectorUrl, "missing inspectorUrl");
 		const ws = new WebSocket(inspectorUrl.href);
 		const openPromise = events.once(ws, "open");
 
@@ -178,6 +180,7 @@ describe.each(OPTIONS)("DevEnv (remote: $remote)", ({ remote }) => {
 
 		const inspectorUrl = await worker.inspectorUrl;
 
+		assert(inspectorUrl, "missing inspectorUrl");
 		let ws = new WebSocket(inspectorUrl.href, {
 			setHost: false,
 			headers: { Host: "example.com" },
@@ -187,6 +190,7 @@ describe.each(OPTIONS)("DevEnv (remote: $remote)", ({ remote }) => {
 		await expect(openPromise).rejects.toThrow("Unexpected server response");
 
 		// Check validates `Origin` header
+		assert(inspectorUrl, "missing inspectorUrl");
 		ws = new WebSocket(inspectorUrl.href, { origin: "https://example.com" });
 		openPromise = events.once(ws, "open");
 		await expect(openPromise).rejects.toThrow("Unexpected server response");
@@ -236,6 +240,7 @@ describe.each(OPTIONS)("DevEnv (remote: $remote)", ({ remote }) => {
 		});
 
 		const inspectorUrl = await worker.inspectorUrl;
+		assert(inspectorUrl, "missing inspectorUrl");
 		const ws = new WebSocket(inspectorUrl.href);
 
 		const consoleApiMessages: DevToolsEvent<"Runtime.consoleAPICalled">[] =
@@ -357,7 +362,7 @@ describe.each(OPTIONS)("DevEnv (remote: $remote)", ({ remote }) => {
 			dev: {
 				remote,
 				server: { port: await getPort() },
-				inspector: { port: await getPort() },
+				inspector: false,
 			},
 		});
 
@@ -373,7 +378,7 @@ describe.each(OPTIONS)("DevEnv (remote: $remote)", ({ remote }) => {
 				...worker.config.dev,
 				remote,
 				server: { port: await getPort() },
-				inspector: { port: await getPort() },
+				inspector: false,
 			},
 		});
 		const newPort = worker.config.dev?.server?.port;

packages/wrangler/src/__tests__/api/startDevWorker/LocalRuntimeController.test.ts

@@ -535,6 +535,7 @@ describe("LocalRuntimeController", () => {
 			});
 			const event = await waitForReloadComplete(controller);
 			const url = urlFromParts(event.proxyData.userWorkerUrl);
+			assert(event.proxyData.userWorkerInspectorUrl);
 			const inspectorUrl = urlFromParts(event.proxyData.userWorkerInspectorUrl);
 
 			// Connect inspector WebSocket

packages/wrangler/src/__tests__/dev.test.ts

@@ -1122,6 +1122,7 @@ describe.sequential("wrangler dev", () => {
 			});
 			fs.writeFileSync("index.js", `export default {};`);
 			const config = await runWranglerUntilConfig("dev");
+			assert(typeof config.dev.inspector === "object");
 			expect(config.dev.inspector?.port).toEqual(9229);
 		});
 
@@ -1132,6 +1133,7 @@ describe.sequential("wrangler dev", () => {
 			});
 			fs.writeFileSync("index.js", `export default {};`);
 			const config = await runWranglerUntilConfig("dev --inspector-port=9999");
+			assert(typeof config.dev.inspector === "object");
 			expect(config.dev.inspector?.port).toEqual(9999);
 		});
 
@@ -1144,6 +1146,7 @@ describe.sequential("wrangler dev", () => {
 			});
 			fs.writeFileSync("index.js", `export default {};`);
 			const config = await runWranglerUntilConfig("dev");
+			assert(typeof config.dev.inspector === "object");
 			expect(config.dev.inspector?.port).toEqual(9999);
 		});
 

packages/wrangler/src/api/mixedMode/index.ts

@@ -53,12 +53,7 @@ export async function startMixedModeSession(
 			server: {
 				port: await getPort(),
 			},
-			// TODO(DEVX-1861): we set this to a random port so that it doesn't conflict with the
-			//                  default one, we should ideally add an option to actually disable
-			//                  the inspector
-			inspector: {
-				port: await getPort(),
-			},
+			inspector: false,
 			logLevel: "error",
 		},
 		bindings: rawBindings,

packages/wrangler/src/api/startDevWorker/ConfigController.ts

@@ -128,12 +128,15 @@ async function resolveDevConfig(
 			httpsKeyPath: input.dev?.server?.httpsKeyPath,
 			httpsCertPath: input.dev?.server?.httpsCertPath,
 		},
-		inspector: {
-			port:
-				input.dev?.inspector?.port ??
-				config.dev.inspector_port ??
-				(await getInspectorPort()),
-		},
+		inspector:
+			input.dev?.inspector === false
+				? false
+				: {
+						port:
+							input.dev?.inspector?.port ??
+							config.dev.inspector_port ??
+							(await getInspectorPort()),
+					},
 		origin: {
 			secure:
 				input.dev?.origin?.secure ?? config.dev.upstream_protocol === "https",

packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts

@@ -109,7 +109,15 @@ export async function convertToConfigBundle(
 		initialPort: undefined,
 		initialIp: "127.0.0.1",
 		rules: [],
-		inspectorPort: 0,
+		...(event.config.dev.inspector === false
+			? {
+					inspect: false,
+					inspectorPort: undefined,
+				}
+			: {
+					inspect: true,
+					inspectorPort: 0,
+				}),
 		localPersistencePath: event.config.dev.persist,
 		liveReload: event.config.dev?.liveReload ?? false,
 		crons,
@@ -119,7 +127,6 @@ export async function convertToConfigBundle(
 		httpsKeyPath: event.config.dev?.server?.httpsKeyPath,
 		localUpstream: event.config.dev?.origin?.hostname,
 		upstreamProtocol: event.config.dev?.origin?.secure ? "https" : "http",
-		inspect: true,
 		services: bindings.services,
 		serviceBindings: fetchers,
 		bindVectorizeToProd: event.config.dev?.bindVectorizeToProd ?? false,
@@ -204,7 +211,13 @@ export class LocalRuntimeController extends RuntimeController {
 			// `inspectorUrl` for this set of `options`, we protect `#mf` with a mutex,
 			// so only one update can happen at a time.
 			const userWorkerUrl = await this.#mf.ready;
-			const userWorkerInspectorUrl = await this.#mf.getInspectorURL();
+			// TODO: Miniflare should itself return undefined on
+			//       `getInspectorURL` when no inspector is in use
+			//       (currently the function just hangs)
+			const userWorkerInspectorUrl =
+				options.inspectorPort === undefined
+					? undefined
+					: await this.#mf.getInspectorURL();
 			// If we received a new `bundleComplete` event before we were able to
 			// dispatch a `reloadComplete` for this bundle, ignore this bundle.
 			if (id !== this.#currentBundleId) {
@@ -228,12 +241,16 @@ export class LocalRuntimeController extends RuntimeController {
 						hostname: userWorkerUrl.hostname,
 						port: userWorkerUrl.port,
 					},
-					userWorkerInspectorUrl: {
-						protocol: userWorkerInspectorUrl.protocol,
-						hostname: userWorkerInspectorUrl.hostname,
-						port: userWorkerInspectorUrl.port,
-						pathname: `/core:user:${getName(data.config)}`,
-					},
+					...(userWorkerInspectorUrl
+						? {
+								userWorkerInspectorUrl: {
+									protocol: userWorkerInspectorUrl.protocol,
+									hostname: userWorkerInspectorUrl.hostname,
+									port: userWorkerInspectorUrl.port,
+									pathname: `/core:user:${getName(data.config)}`,
+								},
+							}
+						: {}),
 					userWorkerInnerUrlOverrides: {
 						protocol: data.config?.dev?.origin?.secure ? "https:" : "http:",
 						hostname: data.config?.dev?.origin?.hostname,