Improve miniflare test stability (#11642)

* update to latest ava

* Display error `cause` in Miniflare tests

* hide unwanted logging from Miniflare browser plugin tests

* quieten Miniflare secrets-store plugin tests

* quieten Miniflare Durable Object plugin tests

* quieten other Miniflare tests

* Retry removing the Browser Session temp directory

* Give the Miniflare browser plugin tests a bit longer to complete

These tests will need to download the Chrome browser in the background, which can take quite a few seconds sometimes.

* silence Miniflare pipeline plugin tests

* Silence Miniflare unsafe plugin tests

* don't run miniflare tests files concurrently to flakes

In particular the dev-registry tests appear to be particularly sensitive
to concurrency issues, likely due to shared ports/sockets.

* run the Miniflare dev-registry tests in series (rather than parallel)

* fix typo

* quieten unwanted logging from Miniflare dev-registry tests

* no keep-alive

* add changeset

* no keep-alive

* fix changeset

* fix lockfile

* fix changeset (again)

* mark Miniflare browser tests as flaky

Author: petebacondarwin

.changeset/floppy-jobs-shout.md

@@ -0,0 +1,7 @@
+---
+"miniflare": patch
+---
+
+Fix intermittent "Fetch failed" errors in Miniflare tests on Windows
+
+Miniflare tests would occasionally fail with "Fetch failed" errors (particularly on Windows CI runners) due to race conditions between undici's Keep-Alive mechanism and the Miniflare server closing idle connections. Miniflare now configures the Dispatcher to prevent connection reuse and eliminate these race condition errors.

package.json

@@ -81,7 +81,8 @@
 			"toucan-js@4.0.0": "patches/toucan-js@4.0.0.patch",
 			"postal-mime": "patches/postal-mime.patch",
 			"youch@4.1.0-beta.10": "patches/youch@4.1.0-beta.10.patch",
-			"@netlify/build-info": "patches/@netlify__build-info.patch"
+			"@netlify/build-info": "patches/@netlify__build-info.patch",
+			"concordance@5.0.4": "patches/concordance@5.0.4.patch"
 		}
 	}
 }

packages/miniflare/ava.config.mjs

@@ -12,4 +12,5 @@ export default {
 	environmentVariables: {
 		MINIFLARE_ASSERT_BODIES_CONSUMED: "true",
 	},
+	concurrency: 1,
 };

packages/miniflare/package.json

@@ -78,7 +78,7 @@
 		"@types/ws": "^8.5.7",
 		"@typescript-eslint/eslint-plugin": "catalog:default",
 		"@typescript-eslint/parser": "catalog:default",
-		"ava": "^6.0.1",
+		"ava": "^6.4.1",
 		"capnp-es": "^0.0.11",
 		"capnweb": "^0.1.0",
 		"chokidar": "^4.0.1",

packages/miniflare/src/http/fetch.ts

@@ -234,6 +234,12 @@ export class DispatchFetchDispatcher extends undici.Dispatcher {
 
 			options.headers = headers;
 
+			// Sometimes, keep-alive connections can sometimes cause issues with sockets
+			// disconnecting unexpectedly. To mitigate this, try to avoid keep-alive race
+			// conditions by telling the runtime to close the connection immediately after
+			// the request is complete
+			options.reset = true;
+
 			// Dispatch with runtime dispatcher to avoid certificate errors if using
 			// self-signed certificate
 			return this.runtimeDispatcher.dispatch(options, handler);

packages/miniflare/src/plugins/browser-rendering/index.ts

@@ -216,7 +216,11 @@ export async function launchBrowser({
 		pipe: false,
 		onExit: async () => {
 			await fs.promises
-				.rm(tempUserData, { recursive: true, force: true })
+				.rm(tempUserData, {
+					recursive: true,
+					force: true,
+					maxRetries: 5, // In case of Windows file locks
+				})
 				.catch((e) => {
 					log.debug(
 						`Unable to remove Chrome user data directory: ${String(e)}`

packages/miniflare/src/shared/dev-registry.ts

@@ -225,6 +225,7 @@ export class DevRegistry {
 			type: "setup",
 			runtimeEntryURL,
 			fallbackServicePorts,
+			logLevel: this.log.level,
 		});
 	}
 

packages/miniflare/src/shared/dev-registry.worker.ts

@@ -21,13 +21,12 @@ interface ProxyAddress {
 	path?: string;
 }
 
-const log = new Log();
-
 /**
  * A HTTP proxy server for the dev registry.
  * This runs in a separate thread to prevent a deadlock when using a node binding
  */
 class ProxyServer {
+	private log = new Log();
 	private registry: Record<string, WorkerDefinition> = {};
 	private runtimeEntryURL: string | null = null;
 	private fallbackServicePorts: Record<string, Record<string, number>> = {};
@@ -46,6 +45,7 @@ class ProxyServer {
 				case "setup":
 					this.runtimeEntryURL = message.runtimeEntryURL;
 					this.fallbackServicePorts = message.fallbackServicePorts;
+					this.log = new Log(message.logLevel);
 					break;
 				default:
 					// Ignore unknown message types
@@ -54,48 +54,41 @@ class ProxyServer {
 		});
 	}
 
-	public async start() {
-		try {
-			// Listen on a random port
-			await new Promise<void>((resolve, reject) => {
-				const server = http.createServer({
-					// There might be no HOST header when proxying a fetch request made over service binding
-					//  e.g. env.MY_WORKER.fetch("https://example.com")
-					requireHostHeader: false,
-					// Disable request and headers timeout for long-lived WebSocket connections
-					// Node.js's headersTimeout (default: min(60s, requestTimeout)) is checked periodically
-					// by connectionsCheckingInterval (default: 30s), causing timeouts around 60-90s.
-					// Setting both to 0 disables timeout enforcement for WebSocket proxying.
-					requestTimeout: 0,
-					headersTimeout: 0,
-				});
-
-				server.on("request", this.handleRequest.bind(this));
-				server.on("connect", this.handleConnect.bind(this));
-				server.listen(0, "127.0.0.1", () => {
-					const address = server.address();
+	public start() {
+		// Listen on a random port
+		return new Promise<void>((resolve, reject) => {
+			const server = http.createServer({
+				// There might be no HOST header when proxying a fetch request made over service binding
+				//  e.g. env.MY_WORKER.fetch("https://example.com")
+				requireHostHeader: false,
+				// Disable request and headers timeout for long-lived WebSocket connections
+				// Node.js's headersTimeout (default: min(60s, requestTimeout)) is checked periodically
+				// by connectionsCheckingInterval (default: 30s), causing timeouts around 60-90s.
+				// Setting both to 0 disables timeout enforcement for WebSocket proxying.
+				requestTimeout: 0,
+				headersTimeout: 0,
+			});
 
-					if (!address || typeof address === "string") {
-						reject(new Error("Failed to get server address"));
-						return;
-					}
+			server.on("request", this.handleRequest.bind(this));
+			server.on("connect", this.handleConnect.bind(this));
+			server.listen(0, "127.0.0.1", () => {
+				const address = server.address();
 
-					// Notify parent that server is ready with the port
-					this.messagePort.postMessage({
-						type: "ready",
-						address: `${address.address}:${address.port}`,
-					});
+				if (!address || typeof address === "string") {
+					reject(new Error("Failed to get server address"));
+					return;
+				}
 
-					resolve();
+				// Notify parent that server is ready with the port
+				this.messagePort.postMessage({
+					type: "ready",
+					address: `${address.address}:${address.port}`,
 				});
-				server.on("error", reject);
-			});
-		} catch (error) {
-			this.messagePort.postMessage({
-				type: "error",
-				error: error instanceof Error ? error.message : String(error),
+
+				resolve();
 			});
-		}
+			server.on("error", reject);
+		});
 	}
 
 	private handleRequest(
@@ -162,19 +155,21 @@ class ProxyServer {
 
 			// Errors on either side
 			serverSocket.on("error", (error) => {
-				log.error(error);
+				this.log.error(error);
 				clientSocket.destroy();
 			});
 			clientSocket.on("error", () => serverSocket.destroy());
 			// Close the tunnel if the service is updated
 			// This makes sure workerd will re-connect to the latest address
 			this.subscribe(serviceName, () => {
-				log.debug(`Closing tunnel as service "${serviceName}" was updated`);
+				this.log.debug(
+					`Closing tunnel as service "${serviceName}" was updated`
+				);
 				serverSocket.end();
 				clientSocket.end();
 			});
 		} catch (e) {
-			log.error(e instanceof Error ? e : new Error(`${e}`));
+			this.log.error(e instanceof Error ? e : new Error(`${e}`));
 			clientSocket.end();
 		}
 	}
@@ -308,7 +303,7 @@ class ProxyServer {
 		});
 
 		upstream.on("error", (error) => {
-			log.error(
+			this.log.error(
 				new Error(
 					`Failed to proxy request to ${target.protocol}://${target.host}:${target.port}${target.path ?? ""}`,
 					{
@@ -428,15 +423,15 @@ function runProxyServer() {
 		throw new Error("This script must be run in a worker thread");
 	}
 
-	const server = new ProxyServer(parentPort);
+	const messagePort = parentPort;
+	const server = new ProxyServer(messagePort);
 
 	// Start the server
 	server.start().catch((error) => {
-		log.error(
-			new Error("ProxyServer: Failed to start proxy server:", {
-				cause: error,
-			})
-		);
+		messagePort.postMessage({
+			type: "error",
+			error: error instanceof Error ? error.message : String(error),
+		});
 	});
 }
 

packages/miniflare/src/workers/pipelines/pipeline.worker.ts

@@ -1,7 +1,5 @@
 import { WorkerEntrypoint } from "cloudflare:workers";
 
 export default class Pipeline extends WorkerEntrypoint {
-	async send(data: object[]): Promise<void> {
-		console.log("Request received", data);
-	}
+	async send(_data: object[]): Promise<void> {}
 }