Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/tidy-moons-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"wrangler": patch
---

`wrangler dev` no longer exits when a request to your Worker fails transiently

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.

`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.
Original file line number Diff line number Diff line change
Expand Up @@ -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/ (failed after 3 attempts): Network connection lost."
),
source: "ProxyController",
data: {},
});

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();
});
});
});
16 changes: 16 additions & 0 deletions packages/wrangler/src/api/startDevWorker/DevEnv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 https://github.com/cloudflare/workers-sdk/issues/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}`
);
Comment on lines +201 to +203

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Logged error for a failed dev request shows a blank reason instead of the request details

The failed proxied request is logged (event.cause.message at packages/wrangler/src/api/startDevWorker/DevEnv.ts:202) using the wrong property, so the method, URL, attempt count and underlying error come out blank and the user only sees a generic message with nothing after the colon.
Impact: When a request to the Worker fails, the dev server keeps running (good) but the logged error is empty of any useful detail, reproducing the very empty-message symptom this change claims to fix.

Why event.cause.message is empty after the JSON boundary

The ProxyWorker packs the useful text (method, URL, attempt count, underlying error.message) into message.error.message and posts it as JSON (packages/wrangler/templates/startDevWorker/ProxyWorker.ts:267-275). On the controller side ProxyController.onProxyWorkerMessage calls emitErrorEvent("Error inside ProxyWorker", message.error) (packages/wrangler/src/api/startDevWorker/ProxyController.ts:478), passing a plain SerializedError object (it crossed JSON.stringify/await req.json(), so it is NOT an Error instance).

emitErrorEvent sets cause: castErrorCause(message.error) (packages/wrangler/src/api/startDevWorker/ProxyController.ts:653). Because the serialized error is not instanceof Error, castErrorCause returns new Error() with an empty message and stashes the serialized object on .cause (packages/wrangler/src/api/startDevWorker/events.ts:32-41).

So in production event.cause.message is "", while the real detail lives at event.cause.cause.message. The new log at DevEnv.ts:201-203 interpolates the empty event.cause.message, yielding Error inside ProxyWorker (the affected request failed; the dev server continues): with nothing after the colon.

The new test DevEnv.test.ts:107-115 masks this because it dispatches an ErrorEvent whose cause is a real new Error("GET ... Network connection lost.") rather than the empty-wrapper Error that castErrorCause actually produces in production.

Prompt for agents
In DevEnv.handleErrorEvent, the recoverable branch for "Error inside ProxyWorker" logs `event.cause.message`, but for this path `event.cause` is an empty-message wrapper Error produced by castErrorCause (events.ts) because ProxyController.onProxyWorkerMessage passes a plain SerializedError object (post-JSON) to emitErrorEvent. The human-readable detail (method, URL, attempt count, underlying message that ProxyWorker.ts assembles) is therefore located at event.cause.cause.message, not event.cause.message. As written the log prints a blank tail after the colon, which reproduces the empty-message symptom this PR intends to fix. Fix the log so it surfaces the underlying message: either read the nested serialized cause's message when event.cause.message is empty, or pass event.cause as an additional argument to logger.error so its cause chain is rendered. Consider also updating the test in DevEnv.test.ts to dispatch a cause shaped like what castErrorCause actually produces (an empty-message Error with the real SerializedError on .cause) so the test reflects production behavior.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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 (
Expand Down
217 changes: 143 additions & 74 deletions packages/wrangler/templates/startDevWorker/ProxyWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,81 +163,150 @@ 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" },
}
/**
* 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 > 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);

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);
},
(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 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
// 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
)
);
}
});
) {
const attemptsNote = ` (failed after ${attempt + 1} ${
attempt === 0 ? "attempt" : "attempts"
})`;
void sendMessageToProxyController(this.env, {
type: "error",
error: {
name: error.name,
message: `${request.method} ${request.url}${attemptsNote}: ${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();
}
}
}
Expand Down
Loading