fix(stores): guard KV JSON parse and DO complete() write failures - #163
Conversation
An adversarial review of ADR 0005 found two unguarded failure paths:
- kvStore: kv.get(key, { type: "json" }) parses inside the runtime and
throws on corrupt data, surfacing as a 500 instead of the documented
miss semantics. All four read sites now go through a guarded helper
that treats corrupt data as absent (lock() overwrites it).
- durableObjectStore.complete(): storage.put() failure propagated
uncaught through the middleware. Now a no-op — the response is served
uncached so a retry with the same key re-executes the handler.
Both paths follow the existing degradation policy (ADR 0005): every
failure converges to "the request is processed again".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 406f131 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
The adversarial review that corrected ADR 0005 (#162) surfaced two real unguarded failure paths in store adapters. This PR closes both, TDD-first (4 new tests written and confirmed failing before the fix).
kv.get(key, { type: "json" })parses inside the Cloudflare runtime and throws on corrupt stored data, so a corrupt record produced an uncaught exception (HTTP 500) instead of the documented degrade-to-miss semantics. All four read sites (get,lockinitial read,lockread-back,complete) now go through asafeGetJsonhelper that treats corrupt data as absent;lock()overwrites the corrupt entry with a fresh lock.storage.put()failure (non-cloneable value, quota error) propagated uncaught —middleware.tscallsstore.complete()without a catch, so this crashed the request after the handler had already succeeded. Now a no-op: the response is served uncached and a retry with the same key re-executes the handler, matching the degradation policy of the serializing adapters.Root-cause fix, not a workaround: the guard lives at the store's serialization boundary (KV's parse happens inside
kv.get; DO's serialization happens insidestorage.put), which is exactly where Redis/D1 already guard theirJSONcalls.Includes a patch changeset and updates ADR 0005's known-gaps bullet to record the closure.
Test plan
get/lock/complete, and a throwingstorage.put()in DOcomplete().pnpm vitest run --coverage— 219/219 pass, 100% statements/branches/functions/lines maintained.pnpm lint/pnpm typecheck— clean.🤖 Generated with Claude Code