Repository navigation
fix(sentry): stop reporting D1 network connection lost blips - #952
Conversation
Treat Cloudflare D1 "Network connection lost" like SQLITE_BUSY / long-running export: retry via runD1WithRetry and drop matching beforeSend events so transient binding drops do not open or regress Sentry issues. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughAdds “Network connection lost” as a retryable D1 error, verifies one retry succeeds, and tests that matching Sentry events are filtered. Related retry documentation and comments are updated. ChangesD1 network retry handling
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-952.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.ts`:
- Around line 28-34: Update isRetryableD1LockMessage to recognize only the exact
Cloudflare network-loss forms: “Network connection lost.” with an optional
“D1_ERROR:” prefix, while preserving the existing lock and export-message
matches. Add a regression test covering “Network connection lost while
uploading...” and verify it remains non-retryable and unsuppressed.
🪄 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: 3e5b1c33-9212-4cdf-a471-34b98c0974d0
📒 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
Avoid retrying or dropping unrelated "Network connection lost while …" messages; only accept the bare D1 phrase with optional Error:/D1_ERROR: prefixes. Format the retry unit test for CI format:check. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Summary
Filters Sentry noise from issue 7633338189 (
Error: D1_ERROR: Network connection lost.).One production event on
admin_user_usage(mcp.failure_phase=handler) failed insidereadEntitlementResourceUsage→countRunningPackageServices→countRowswhen the D1 binding dropped mid-query (D1DatabaseSessionAlwaysPrimary._sendOrThrow). This is transient Cloudflare D1 transport unavailability, not an application bug — the same class already handled forSQLITE_BUSYand long-running export blocks (#930).This PR extends the existing retryable-D1 matcher so:
runD1WithRetry/ cron lane skip / module-artifact transient detection treat "Network connection lost" like lock contentionbeforeSend(filterRetryableD1LockSentryEvent) drops matching events so binding blips do not open or regress Sentry issuesThe matcher accepts only the exact D1 phrase with optional
Error:/D1_ERROR:prefixes, so unrelated "Network connection lost while …" messages stay retryable/Sentry-visible.MCP requests can still fail for the caller when D1 drops the session before a retry wrapper covers that path. We stop treating that as a platform defect in Sentry.
Siblings (client
updateCurrentEntry, sandbox timeout, search validation) do not share this root cause and are left alone.Test plan
d1-retry.node.test.tsmatches exact D1 network-loss forms; rejects "Network connection lost while uploading..."; retries exact network errors; still rethrows syntax errors immediatelysentry-options.node.test.tsdrops both exact message forms, keeps unrelated network-loss wording and other D1 errorsnpm run validategreennpm run validategreenSystem recap — composes existing primitives (low risk)
Mode: recap · Base:
main@7f6f85c2· Head:c70a5d88Classification: 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.tsSystem map
Transient D1 binding drops 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
Bug Fixes
Tests
Documentation