From 15061cd52eb7a234fe966a251414dce3dc4ee888 Mon Sep 17 00:00:00 2001 From: Gregory Collett Date: Tue, 18 Aug 2026 16:28:03 +0100 Subject: [PATCH 1/3] fix(wrangler): keep wrangler dev alive when a proxied request to the UserWorker fails transiently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a request proxied to the UserWorker failed while the UserWorker's origin was unchanged — most commonly a reused keep-alive connection the UserWorker's HTTP server closed at the same moment the 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) — the ProxyWorker reported a fatal error and the whole dev server exited with an empty error message, leaving the port unbound. The ProxyWorker now retries bodyless (GET/HEAD) requests before reporting, absorbing the transient failure on a fresh connection, and DevEnv classifies an exhausted or non-retriable report as recoverable: it is logged with the request method, URL, attempt count and underlying exception, and the dev server keeps serving. Only the affected request fails. Co-Authored-By: Claude Fable 5 --- .changeset/tidy-moons-provide.md | 9 + .../api/startDevWorker/DevEnv.test.ts | 26 +++ .../wrangler/src/api/startDevWorker/DevEnv.ts | 16 ++ .../templates/startDevWorker/ProxyWorker.ts | 177 ++++++++++-------- 4 files changed, 155 insertions(+), 73 deletions(-) create mode 100644 .changeset/tidy-moons-provide.md diff --git a/.changeset/tidy-moons-provide.md b/.changeset/tidy-moons-provide.md new file mode 100644 index 00000000000..4169f03dcb7 --- /dev/null +++ b/.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. diff --git a/packages/wrangler/src/__tests__/api/startDevWorker/DevEnv.test.ts b/packages/wrangler/src/__tests__/api/startDevWorker/DevEnv.test.ts index 3179032a7fb..bc684efe457 100644 --- a/packages/wrangler/src/__tests__/api/startDevWorker/DevEnv.test.ts +++ b/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(); + }); }); }); diff --git a/packages/wrangler/src/api/startDevWorker/DevEnv.ts b/packages/wrangler/src/api/startDevWorker/DevEnv.ts index 6752a46f178..0f7512aed4a 100644 --- a/packages/wrangler/src/api/startDevWorker/DevEnv.ts +++ b/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 ( diff --git a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts index 59b507d72fe..9ef63bebf3a 100644 --- a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts +++ b/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); } } } From 8adab46f457226e89010a8135b5dd27d86dcef96 Mon Sep 17 00:00:00 2001 From: Gregory Collett Date: Thu, 20 Aug 2026 16:40:37 +0100 Subject: [PATCH 2/3] address review: retry only connection-level fetch failures - Move the GET/HEAD retry into fetch's rejection handler so errors thrown while post-processing a received response are reported, never retried (and can never re-run the UserWorker's handler). - Make the attempt parameter optional and 0-indexed. - Report exhausted retries explicitly ("failed after 3 attempts"). - data: {} in the DevEnv test; full issue URL in the DevEnv comment. - Rewrite the changeset to be user-facing. Co-Authored-By: Claude Fable 5 --- .changeset/tidy-moons-provide.md | 6 +- .../api/startDevWorker/DevEnv.test.ts | 4 +- .../wrangler/src/api/startDevWorker/DevEnv.ts | 2 +- .../templates/startDevWorker/ProxyWorker.ts | 125 ++++++++++++------ 4 files changed, 87 insertions(+), 50 deletions(-) diff --git a/.changeset/tidy-moons-provide.md b/.changeset/tidy-moons-provide.md index 4169f03dcb7..c5a8949ee00 100644 --- a/.changeset/tidy-moons-provide.md +++ b/.changeset/tidy-moons-provide.md @@ -2,8 +2,8 @@ "wrangler": patch --- -`wrangler dev` no longer exits when a proxied request to the UserWorker fails transiently +`wrangler dev` no longer exits when a request to your Worker 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. +Previously, a transient network failure on a single request — most commonly a request arriving just as an idle internal connection was closed, after roughly five seconds without traffic — could take down the whole dev server with an empty `✘ [ERROR]`, leaving the port unbound until restarted. In CI test suites, one such failure caused every remaining test to fail with connection errors. -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. +`wrangler dev` now automatically retries the affected request if it is safe to repeat (GET and HEAD requests). If a request still fails, it fails individually — the error is logged with the request method and URL — and the dev server keeps serving. diff --git a/packages/wrangler/src/__tests__/api/startDevWorker/DevEnv.test.ts b/packages/wrangler/src/__tests__/api/startDevWorker/DevEnv.test.ts index bc684efe457..9fb178d46db 100644 --- a/packages/wrangler/src/__tests__/api/startDevWorker/DevEnv.test.ts +++ b/packages/wrangler/src/__tests__/api/startDevWorker/DevEnv.test.ts @@ -108,10 +108,10 @@ describe("DevEnv", () => { type: "error", reason: "Error inside ProxyWorker", cause: new Error( - "GET http://127.0.0.1:8787/ (attempt 3): Network connection lost." + "GET http://127.0.0.1:8787/ (failed after 3 attempts): Network connection lost." ), source: "ProxyController", - data: undefined, + data: {}, }); expect(std.err).toContain("Error inside ProxyWorker"); diff --git a/packages/wrangler/src/api/startDevWorker/DevEnv.ts b/packages/wrangler/src/api/startDevWorker/DevEnv.ts index 0f7512aed4a..7678bbdbc5a 100644 --- a/packages/wrangler/src/api/startDevWorker/DevEnv.ts +++ b/packages/wrangler/src/api/startDevWorker/DevEnv.ts @@ -193,7 +193,7 @@ export class DevEnv extends EventEmitter implements ControllerBus { // 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. + // a dead dev server (see https://github.com/cloudflare/workers-sdk/issues/14926). Log it and keep serving. else if ( event.source === "ProxyController" && event.reason.startsWith("Error inside ProxyWorker") diff --git a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts index 9ef63bebf3a..dbb45c8f789 100644 --- a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts +++ b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts @@ -163,36 +163,90 @@ 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 - const attemptUserWorkerFetch = (attempt: number) => + /** + * Sends the request to the UserWorker and settles `deferredResponse` + * with the outcome — or requeues the request if the UserWorker is + * mid-reload. The promise chain is deliberately not awaited (`void`): + * we are in a loop and want to process the whole queue quickly + + * synchronously. + * + * A connection-level failure (the `fetch` itself rejects, so no + * response was received) on a same-origin GET/HEAD request is retried + * before being reported — see the rejection handler. + * + * Kept as a `const` arrow function: the handlers re-read + * `this.proxyData` across async boundaries to detect UserWorker + * reloads, so they need the ProxyWorker's lexical `this`. + * + * @param attempt the number of attempts that have already failed + * (`0` on the first try) + */ + const attemptUserWorkerFetch = (attempt = 0) => 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` - ); - } + .then( + async (res) => { + if (attempt > 0) { + console.warn( + `ProxyWorker: ${request.method} ${request.url} recovered on attempt ${attempt + 1} after a dropped connection to the UserWorker` + ); + } - res = new Response(res.body, res); - rewriteUrlRelatedHeaders(res.headers, innerUrl, outerUrl); + res = new Response(res.body, res); + rewriteUrlRelatedHeaders(res.headers, innerUrl, outerUrl); - await checkForPreviewTokenError(res, this.env, proxyData); + await checkForPreviewTokenError(res, this.env, proxyData); - if (isHtmlResponse(res)) { - res = insertLiveReloadScript(request, res, this.env, proxyData); - } + if (isHtmlResponse(res)) { + res = insertLiveReloadScript(request, res, this.env, proxyData); + } - if (isSseResponse(res)) { - void sendMessageToProxyController(this.env, { - type: "sseResponseDetected", - }); - } + if (isSseResponse(res)) { + void sendMessageToProxyController(this.env, { + type: "sseResponseDetected", + }); + } - deferredResponse.resolve(res); - }) + deferredResponse.resolve(res); + }, + (error: Error) => { + // the fetch itself rejected: a connection-level failure, and no + // response was received. Errors thrown while post-processing a + // received response skip this handler and land in the .catch + // below, so they are reported rather than retried and can never + // re-run the UserWorker's handler. + // + // When the UserWorker origin is unchanged (i.e. this is not a + // reload — see the same check in the .catch below), the failure + // is 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 in the + // .catch for reloads: retry immediately, then once more after + // 250ms (3 attempts in total). + if ( + isSameUserWorkerOrigin( + userWorkerUrl, + this.proxyData?.userWorkerUrl + ) && + (request.method === "GET" || request.method === "HEAD") && + attempt < 2 + ) { + setTimeout( + () => attemptUserWorkerFetch(attempt + 1), + attempt === 0 ? 0 : 250 + ); + return; + } + + throw error; + } + ) .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() + // errors here are from response post-processing, or connection- + // level failures rethrown by the rejection handler above (a + // non-retriable method, or the retry budget was exhausted) // we have crossed an async boundary, so proxyData may have changed // if proxyData.userWorkerUrl has changed, it means there is a new downstream UserWorker @@ -207,30 +261,13 @@ export class ProxyWorker implements DurableObject { 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; - } - + const attemptsNote = + attempt > 0 ? ` (failed after ${attempt + 1} attempts)` : ""; void sendMessageToProxyController(this.env, { type: "error", error: { name: error.name, - message: `${request.method} ${request.url} (attempt ${attempt}): ${error.message}`, + message: `${request.method} ${request.url}${attemptsNote}: ${error.message}`, stack: error.stack, cause: error.cause, }, @@ -268,7 +305,7 @@ export class ProxyWorker implements DurableObject { } }); - attemptUserWorkerFetch(1); + attemptUserWorkerFetch(); } } } From 5fc41f036f0fb2512248821333c6e59e5a6ef3a6 Mon Sep 17 00:00:00 2001 From: Gregory Collett Date: Thu, 20 Aug 2026 17:42:17 +0100 Subject: [PATCH 3/3] report the true attempt count on proxied-request errors Co-Authored-By: Claude Fable 5 --- packages/wrangler/templates/startDevWorker/ProxyWorker.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts index dbb45c8f789..fed5b41a4f8 100644 --- a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts +++ b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts @@ -261,8 +261,9 @@ export class ProxyWorker implements DurableObject { this.proxyData?.userWorkerUrl ) ) { - const attemptsNote = - attempt > 0 ? ` (failed after ${attempt + 1} attempts)` : ""; + const attemptsNote = ` (failed after ${attempt + 1} ${ + attempt === 0 ? "attempt" : "attempts" + })`; void sendMessageToProxyController(this.env, { type: "error", error: {