fix(wrangler): keep wrangler dev alive when a proxied request to the UserWorker fails transiently

.changeset/tidy-moons-provide.md

@@ -0,0 +1,9 @@
+---
+"wrangler": patch
+---
+
+`wrangler dev` no longer exits when a proxied request to the UserWorker fails transiently
+
+When a request proxied to the UserWorker failed while the UserWorker's origin was unchanged — most commonly a reused keep-alive connection that the UserWorker's HTTP server closed at the same moment the request was written to it — the ProxyWorker reported a fatal error and the whole dev server exited with an empty `✘ [ERROR]`, leaving the port unbound. In CI test suites one such transient failure killed every remaining test.
+
+The ProxyWorker now retries bodyless (GET/HEAD) requests before reporting, which absorbs the transient failure on a fresh connection, and an exhausted or non-retriable failure is logged — including the request method, URL and underlying exception — while the dev server keeps serving. Only the affected request fails.

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

@@ -95,5 +95,31 @@ describe("DevEnv", () => {
 
 			void devEnv.teardown();
 		});
+
+		test("should log ProxyWorker request errors without tearing down the dev session", ({
+			expect,
+		}) => {
+			const devEnv = new DevEnv();
+
+			const fatalEvents: unknown[] = [];
+			devEnv.on("error", (event) => fatalEvents.push(event));
+
+			devEnv.dispatch({
+				type: "error",
+				reason: "Error inside ProxyWorker",
+				cause: new Error(
+					"GET http://127.0.0.1:8787/ (attempt 3): Network connection lost."
+				),
+				source: "ProxyController",
+				data: undefined,
+			});
+
+			expect(std.err).toContain("Error inside ProxyWorker");
+			expect(std.err).toContain("Network connection lost.");
+			// one failed proxied request must not become a fatal dev-session error
+			expect(fatalEvents).toHaveLength(0);
+
+			void devEnv.teardown();
+		});
 	});
 });

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

@@ -187,6 +187,22 @@ export class DevEnv extends EventEmitter implements ControllerBus {
 			logger.debug(`Error in ${event.source}: ${event.reason}\n`, event.cause);
 			logger.debug("=> Error contextual data:", event.data);
 		}
+		// A proxied request to the UserWorker failed while the UserWorker was NOT
+		// being reloaded — e.g. the UserWorker's HTTP server closed a reused
+		// keep-alive connection at the same moment the ProxyWorker wrote a
+		// request into it. The affected request has already failed (and the
+		// ProxyWorker retries GET/HEAD before reporting), but the dev session
+		// itself is healthy: tearing it down would turn one failed request into
+		// a dead dev server (see #14926). Log it and keep serving.
+		else if (
+			event.source === "ProxyController" &&
+			event.reason.startsWith("Error inside ProxyWorker")
+		) {
+			logger.error(
+				`${event.reason} (the affected request failed; the dev server continues): ${event.cause.message}`
+			);
+			logger.debug("=> Error contextual data:", event.data);
+		}
 		// Parse errors are recoverable by changing your Wrangler configuration file and saving
 		// All other errors from the ConfigController are non-recoverable
 		else if (

packages/wrangler/templates/startDevWorker/ProxyWorker.ts

@@ -164,80 +164,111 @@ export class ProxyWorker implements DurableObject {
 			}
 
 			// explicitly NOT await-ing this promise, we are in a loop and want to process the whole queue quickly + synchronously
-			void fetch(userWorkerUrl, new Request(request, { headers }))
-				.then(async (res) => {
-					res = new Response(res.body, res);
-					rewriteUrlRelatedHeaders(res.headers, innerUrl, outerUrl);
-
-					await checkForPreviewTokenError(res, this.env, proxyData);
-
-					if (isHtmlResponse(res)) {
-						res = insertLiveReloadScript(request, res, this.env, proxyData);
-					}
-
-					if (isSseResponse(res)) {
-						void sendMessageToProxyController(this.env, {
-							type: "sseResponseDetected",
-						});
-					}
-
-					deferredResponse.resolve(res);
-				})
-				.catch((error: Error) => {
-					// errors here are network errors or from response post-processing
-					// to catch only network errors, use the 2nd param of the fetch.then()
-
-					// we have crossed an async boundary, so proxyData may have changed
-					// if proxyData.userWorkerUrl has changed, it means there is a new downstream UserWorker
-					// and that this error is stale since it was for a request to the old UserWorker
-					// only report the error if the request still targets the current
-					// UserWorker. isSameUserWorkerOrigin compares origin (not href) so a
-					// genuine error on a non-root path isn't misread as a reload — see
-					// its docs.
-					if (
-						isSameUserWorkerOrigin(userWorkerUrl, this.proxyData?.userWorkerUrl)
-					) {
-						void sendMessageToProxyController(this.env, {
-							type: "error",
-							error: {
-								name: error.name,
-								message: error.message,
-								stack: error.stack,
-								cause: error.cause,
-							},
-						});
-
-						deferredResponse.reject(error);
-					}
-
-					// if the request can be retried (subset of idempotent requests which have no body), requeue it
-					else if (request.method === "GET" || request.method === "HEAD") {
-						this.requestRetryQueue.set(request, deferredResponse);
-						// we would only end up here if the downstream UserWorker is chang*ing*
-						// i.e. we are in a `pause`d state and expecting a `play` message soon
-						// this request will be processed (retried) when the `play` message arrives
-						// for that reason, we do not need to call `this.processQueue` here
-						// (but, also, it can't hurt to call it since it bails when
-						// in a `pause`d state i.e. `this.proxyData` is undefined)
-					}
-
-					// if the request cannot be retried, respond with 503 Service Unavailable
-					// important to note, this is not an (unexpected) error -- it is an acceptable flow of local development
-					// it would be incorrect to retry non-idempotent requests
-					// and would require cloning all body streams to avoid stream reuse (which is inefficient but not out of the question in the future)
-					// this is a good enough UX for now since it solves the most common GET use-case
-					else {
-						deferredResponse.resolve(
-							new Response(
-								"Your worker restarted mid-request. Please try sending the request again. Only GET or HEAD requests are retried automatically.",
-								{
-									status: 503,
-									headers: { "Retry-After": "0" },
-								}
+			const attemptUserWorkerFetch = (attempt: number) =>
+				void fetch(userWorkerUrl, new Request(request, { headers }))
+					.then(async (res) => {
+						if (attempt > 1) {
+							console.warn(
+								`ProxyWorker: ${request.method} ${request.url} recovered on attempt ${attempt} after a dropped connection to the UserWorker`
+							);
+						}
+
+						res = new Response(res.body, res);
+						rewriteUrlRelatedHeaders(res.headers, innerUrl, outerUrl);
+
+						await checkForPreviewTokenError(res, this.env, proxyData);
+
+						if (isHtmlResponse(res)) {
+							res = insertLiveReloadScript(request, res, this.env, proxyData);
+						}
+
+						if (isSseResponse(res)) {
+							void sendMessageToProxyController(this.env, {
+								type: "sseResponseDetected",
+							});
+						}
+
+						deferredResponse.resolve(res);
+					})
+					.catch((error: Error) => {
+						// errors here are network errors or from response post-processing
+						// to catch only network errors, use the 2nd param of the fetch.then()
+
+						// we have crossed an async boundary, so proxyData may have changed
+						// if proxyData.userWorkerUrl has changed, it means there is a new downstream UserWorker
+						// and that this error is stale since it was for a request to the old UserWorker
+						// only report the error if the request still targets the current
+						// UserWorker. isSameUserWorkerOrigin compares origin (not href) so a
+						// genuine error on a non-root path isn't misread as a reload — see
+						// its docs.
+						if (
+							isSameUserWorkerOrigin(
+								userWorkerUrl,
+								this.proxyData?.userWorkerUrl
 							)
-						);
-					}
-				});
+						) {
+							// the UserWorker origin is unchanged, so this is a transient
+							// network failure rather than a reload — most commonly the
+							// UserWorker's HTTP server closing a reused keep-alive
+							// connection at the same moment this request was written to it
+							// (kj's client pool idleTimeout and server pipelineTimeout both
+							// default to 5s, so a connection idling ~5s races the close).
+							// Retrying bodyless requests draws a fresh connection and
+							// absorbs the race, mirroring the requeue below for reloads.
+							if (
+								(request.method === "GET" || request.method === "HEAD") &&
+								attempt < 3
+							) {
+								setTimeout(
+									() => attemptUserWorkerFetch(attempt + 1),
+									attempt === 1 ? 0 : 250
+								);
+								return;
+							}
+
+							void sendMessageToProxyController(this.env, {
+								type: "error",
+								error: {
+									name: error.name,
+									message: `${request.method} ${request.url} (attempt ${attempt}): ${error.message}`,
+									stack: error.stack,
+									cause: error.cause,
+								},
+							});
+
+							deferredResponse.reject(error);
+						}
+
+						// if the request can be retried (subset of idempotent requests which have no body), requeue it
+						else if (request.method === "GET" || request.method === "HEAD") {
+							this.requestRetryQueue.set(request, deferredResponse);
+							// we would only end up here if the downstream UserWorker is chang*ing*
+							// i.e. we are in a `pause`d state and expecting a `play` message soon
+							// this request will be processed (retried) when the `play` message arrives
+							// for that reason, we do not need to call `this.processQueue` here
+							// (but, also, it can't hurt to call it since it bails when
+							// in a `pause`d state i.e. `this.proxyData` is undefined)
+						}
+
+						// if the request cannot be retried, respond with 503 Service Unavailable
+						// important to note, this is not an (unexpected) error -- it is an acceptable flow of local development
+						// it would be incorrect to retry non-idempotent requests
+						// and would require cloning all body streams to avoid stream reuse (which is inefficient but not out of the question in the future)
+						// this is a good enough UX for now since it solves the most common GET use-case
+						else {
+							deferredResponse.resolve(
+								new Response(
+									"Your worker restarted mid-request. Please try sending the request again. Only GET or HEAD requests are retried automatically.",
+									{
+										status: 503,
+										headers: { "Retry-After": "0" },
+									}
+								)
+							);
+						}
+					});
+
+			attemptUserWorkerFetch(1);
 		}
 	}
 }