Repository navigation
fix(sentry): drop D1 storage object-reset blips - #954
Conversation
Extend the retryable D1 internal-error matcher to Cloudflare's "Internal error in D1 DB storage caused object to be reset; reference = …" so runD1WithRetry and beforeSend treat it like other opaque D1 faults. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThe D1 retry matcher now recognizes referenced storage object-reset errors as retryable. Tests cover classification, retry execution, and Sentry filtering for prefixed and unprefixed variants. ChangesD1 retry classification
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
🔎 Preview deployed: https://kody-pr-954.kody-a99.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/d1-retry.node.test.ts`:
- Around line 51-64: Add a negative test in
packages/worker/src/d1-retry.node.test.ts around isRetryableD1LockError to
verify the unreferenced object-reset message returns false, while preserving the
existing referenced cases. Also update
packages/worker/src/sentry-options.node.test.ts around filterSentryEvent to
verify the corresponding unreferenced event is returned unchanged; both tests
should cover the required-reference boundary without changing matcher behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 943850ca-8ae0-4c1c-a5ff-9dfa39cec523
📒 Files selected for processing (4)
packages/worker/src/d1-retry.node.test.tspackages/worker/src/d1-retry.tspackages/worker/src/sentry-options.node.test.tspackages/worker/src/sentry-options.ts
Cover the required-reference boundary so a bare storage object-reset message is neither retried nor dropped by beforeSend. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Summary
Filters Sentry noise from KODY-CLOUDFLARE-28 (
Error: D1_ERROR: Internal error in D1 DB storage caused object to be reset; reference = 8t4dqqpoq1ctvjr8kca8fl4c).One production event on MCP capability
admin_user_usagefailed insidereadEntitlementResourceUsage→readDailyEntitlementCounterwhen Cloudflare D1 returned an opaque storage fault (D1DatabaseSessionAlwaysPrimary._sendOrThrow, support reference8t4dqqpoq1ctvjr8kca8fl4c). This is transient D1 backend unavailability, not an application bug — the same class already handled forSQLITE_BUSY, long-running export blocks,Network connection lost(#952), andinternal error; reference = …(#953).PR #953 already filters the short
internal error; reference = <id>form. This event used a longer Cloudflare phrasing that the exact matcher missed. This PR extends that matcher so:runD1WithRetry/ cron lane skip / module-artifact transient detection treat the storage object-reset form like other D1 platform blipsbeforeSend(filterRetryableD1LockSentryEvent) drops matching events so these blips do not open or regress Sentry issuesThe matcher still requires a
reference = <token>and only accepts the two known D1 phrasings (internal error; …andinternal error in D1 DB storage caused object to be reset; …), so bare"internal error"and unrelated wording stay retryable/Sentry-visible.Siblings (client
updateCurrentEntry, sandbox timeout, search validation) do not share this root cause and are left alone.Test plan
d1-retry.node.test.tsmatches the object-reset+reference form; rejects unreferenced object-reset and mismatched wording; retries once then succeedssentry-options.node.test.tsdrops both prefixed and unprefixed object-reset forms; keeps unreferenced object-resetnpm run validategreennpm run validategreenSystem recap — composes existing primitives (low risk)
Mode: recap · Base:
main@6fed00ac· Head:e4bc82d8Classification: composes — narrow extension of the existing D1 transient matcher + Sentry
beforeSendfilter; no D1 schema, auth, or backup export contract changes.Primitives touched
d1-retry.ts/sentry-options.tsreference = …like SQLITE_BUSY for retry + Sentry dropSystem map
Transient D1 storage object-reset faults still fail uncovered queries; Sentry and retry paths no longer treat that blip as a novel platform failure.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Summary by CodeRabbit