diff --git a/packages/hosts/cloudflare/src/mcp/durable-object-errors.test.ts b/packages/hosts/cloudflare/src/mcp/durable-object-errors.test.ts index 2eca5e77d..ab939d3a3 100644 --- a/packages/hosts/cloudflare/src/mcp/durable-object-errors.test.ts +++ b/packages/hosts/cloudflare/src/mcp/durable-object-errors.test.ts @@ -43,6 +43,36 @@ describe("classifyDurableObjectError", () => { ).toEqual({ kind: "storage_internal", disposition: "transient" }); }); + it("reads a storage fault raised while the object starts up as transient", () => { + expect( + classifyDurableObjectError( + new Error( + "Internal error while starting up Durable Object storage caused object to be reset; reference = 0000aaaa1111bbbb", + ), + ), + ).toEqual({ kind: "startup_internal_error", disposition: "transient" }); + }); + + // This variant escaped as an unhandled 500 even with the bare-blip fragment + // in place, because the runtime interposes its own description between + // "internal error" and the reference id. Pinning the distinction keeps a + // future "just widen the blip fragment" from silently re-merging the two. + it("keeps the startup fault distinct from the bare platform blip", () => { + const startup = classifyDurableObjectError( + new Error( + "Internal error while starting up Durable Object storage caused object to be reset; reference = ffff9999eeee8888", + ), + ); + const blip = classifyDurableObjectError( + new Error("internal error; reference = ffff9999eeee8888"), + ); + + expect(startup?.kind).toBe("startup_internal_error"); + expect(blip?.kind, "the described fault must not be read as the bare blip").toBe( + "internal_error", + ); + }); + it("reads a blockConcurrencyWhile cancellation as transient", () => { expect( classifyDurableObjectError( @@ -53,6 +83,28 @@ describe("classifyDurableObjectError", () => { ).toEqual({ kind: "concurrency_reset", disposition: "transient" }); }); + it("reads a CPU-limit reset as transient", () => { + expect( + classifyDurableObjectError( + new Error("Durable Object exceeded its CPU time limit and was reset."), + ), + ).toEqual({ kind: "cpu_limit", disposition: "transient" }); + }); + + // The memory-limit reset is the CPU limit's sibling and is deliberately NOT + // classified: the runtime names the application as the cause (un-awaited + // writes, an oversized read), so a retry reproduces it. It has to keep being + // rethrown and reported rather than disappearing into a 503. + it("refuses to classify the sibling memory-limit reset as retryable", () => { + expect( + classifyDurableObjectError( + new Error( + "Durable Object's isolate exceeded its memory limit due to overflowing the storage cache. All objects in the isolate were reset.", + ), + ), + ).toBeNull(); + }); + it("reads a platform blip as transient, ignoring the reference id", () => { // The reference id differs on every event; it must not defeat the match. expect( @@ -185,4 +237,39 @@ describe("durableObjectFailureResponse", () => { expect(result.retryAfter, message).not.toBeNull(); } }); + + // A storage fault while the object is coming up reaches the handler as the + // same untyped Error as every other reset, and used to fall out of the worker + // as an unhandled 500. Nothing about the request caused it, so the client is + // told to retry the same id rather than to reconnect. + it("tells the client to retry the same session after a startup storage fault", async () => { + const result = await envelope( + new Error( + "Internal error while starting up Durable Object storage caused object to be reset; reference = 0000aaaa1111bbbb", + ), + ); + + expect(result.status, "HTTP status is the discriminator clients act on").toBe(503); + expect(result.body.jsonrpc).toBe("2.0"); + expect(result.body.error?.code).toBe(-32001); + expect(result.retryAfter, "the client is told how long to back off").toBe( + String(UNAVAILABLE_RETRY_AFTER_SECONDS), + ); + }); + + // An invocation cut off at the CPU ceiling reaches the handler as the same + // untyped Error as every other reset, and used to fall out of the worker as + // an unhandled 500. It must land on the retry-the-same-id envelope instead. + it("tells the client to retry the same session after a CPU-limit reset", async () => { + const result = await envelope( + new Error("Durable Object exceeded its CPU time limit and was reset."), + ); + + expect(result.status, "HTTP status is the discriminator clients act on").toBe(503); + expect(result.body.jsonrpc).toBe("2.0"); + expect(result.body.error?.code).toBe(-32001); + expect(result.retryAfter, "the client is told how long to back off").toBe( + String(UNAVAILABLE_RETRY_AFTER_SECONDS), + ); + }); }); diff --git a/packages/hosts/cloudflare/src/mcp/durable-object-errors.ts b/packages/hosts/cloudflare/src/mcp/durable-object-errors.ts index 4b77f1520..0eee14c2b 100644 --- a/packages/hosts/cloudflare/src/mcp/durable-object-errors.ts +++ b/packages/hosts/cloudflare/src/mcp/durable-object-errors.ts @@ -35,8 +35,12 @@ export type DurableObjectFailureKind = | "storage_timeout" /** The storage backend failed internally and reset the object. */ | "storage_internal" + /** The storage backend failed internally *while bringing the object up*. */ + | "startup_internal_error" /** `blockConcurrencyWhile()` ran past its cap and was cancelled. */ | "concurrency_reset" + /** An invocation ran past the per-invocation CPU ceiling; the object was reset. */ + | "cpu_limit" /** A generic platform blip: `internal error; reference = `. */ | "internal_error" /** The runtime itself flagged the error as retryable. */ @@ -78,6 +82,26 @@ const MESSAGE_PATTERNS: ReadonlyArray<{ fragment: "internal error in durable object storage", failure: { kind: "storage_internal", disposition: "transient" }, }, + { + // The startup sibling of the entry above: the storage backend faults while + // the object is being brought up rather than while it is serving, and the + // runtime says so in a different clause ("… while starting up Durable + // Object storage caused object to be reset; reference = "). + // + // Deliberately stops before "storage": the two known members of this family + // agree on "Internal error … Durable Object" and disagree on everything + // that follows the verb, so the qualifier is the part most likely to move + // and is not what identifies the failure. Equally deliberately NOT + // shortened to the shared tail "caused object to be reset" — that tail is + // common to several unrelated storage faults and would stop this bucket + // from meaning anything on a span. + // + // Transient for the same reason as its sibling: nothing about the request + // caused it, the id still routes, and the object gets a fresh start on the + // next attempt. + fragment: "internal error while starting up durable object", + failure: { kind: "startup_internal_error", disposition: "transient" }, + }, { // Deliberately the whole phrase, not the bare method name: an application // defect thrown from inside a `blockConcurrencyWhile` callback also resets @@ -87,9 +111,37 @@ const MESSAGE_PATTERNS: ReadonlyArray<{ failure: { kind: "concurrency_reset", disposition: "transient" }, }, { + // Deliberately starts at the verb, not at "Durable Object": the runtime's + // resource-limit messages disagree about the subject noun (the memory + // variant says "Durable Object's isolate exceeded its memory limit"), and + // pinning a subject here would let a rewording defeat the match. From + // "exceeded its CPU time limit" onward the phrase is the runtime's alone. + // + // Transient, not a defect: the invocation was cut off but the object's + // durable storage is untouched and the session id still routes, so the next + // attempt — a smaller unit of work, or the same one under a warm isolate — + // can succeed. Retrying the same id is strictly better than the unhandled + // 500 this produced before. + fragment: "exceeded its cpu time limit and was reset", + failure: { kind: "cpu_limit", disposition: "transient" }, + }, + { + // Only the bare blip. A reference id at the end of the message is NOT the + // marker: the runtime also appends one to described faults such as the + // startup failure above, and because this fragment includes the semicolon + // that immediately follows "internal error", any interposed description + // defeats it. Those variants each need their own entry rather than a + // loosened version of this one, which would turn every referenced error + // into an opaque "internal_error" bucket. fragment: "internal error; reference =", failure: { kind: "internal_error", disposition: "transient" }, }, + // Not listed, on purpose: the sibling memory-limit reset ("Durable Object's + // isolate exceeded its memory limit due to overflowing the storage cache … + // All objects in the isolate were reset."). The runtime tags that one as a + // user error, and it names its own cause — too many un-awaited writes, or one + // oversized read. Retrying reproduces it, so calling it transient would bury + // an application defect behind a 503 instead of surfacing it. ]; /**