feat(database): add an infrastructure-error classifier and read-retry helper - #4863
feat(database): add an infrastructure-error classifier and read-retry helper#4863d-cs wants to merge 1 commit into
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe database package adds Prisma and connectivity-error classification. It adds Merge Risk: 🔵 Low · up to The PR adds an opt-in database retry helper and classifier, with retries disabled by default. It is mergeable with explicit owner awareness: non-finite randomness should be normalized, and callers must use only repeat-safe operations with a shared retry budget to avoid duplicated effects or increased database load during outages. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description accurately explains the implementation and intended use, but it does not follow the repository template. It omits the issue reference, checklist, testing section, changelog, and screenshots section. Resolution Add the required template sections. Include the issue reference, completed checklist items, specific testing steps and results, a short changelog entry, and the screenshots section or an explicit indication that screenshots are not applicable. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
e35afcf to
f4f22da
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
f4f22da to
0436e22
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
0436e22 to
e77d850
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Adds a shared classifier (isInfrastructureError / looksLikeConnectivityError) recognising connection-blip failures (P1001/P1002/P1008/P1017, ECONNRESET, "connection terminated", "server has closed the connection"), and withInfraRetry — a retry helper gated by an enabled kill-switch (default off) and the existing TokenBucketRetryBudget. Only for operations safe to run more than once (reads, or writes made idempotent); it never authorises retrying a bare non-idempotent write. TRI-13553
e77d850 to
8dbc68b
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| error instanceof Prisma.PrismaClientInitializationError || | ||
| error instanceof Prisma.PrismaClientRustPanicError || | ||
| error instanceof Prisma.PrismaClientUnknownRequestError | ||
| ) { | ||
| return true; |
There was a problem hiding this comment.
🟡 Permanent failures consume retry budget
Invalid configuration and unknown query failures pass isInfrastructureError unconditionally. withInfraRetry repeats them, delaying errors and draining retries needed for connection blips.
Prompt for agents
Narrow the default retry classification in internal-packages/database/src/infraError.ts. PrismaClientInitializationError can represent persistent configuration, authentication, or database-selection failures, and PrismaClientUnknownRequestError can represent non-connectivity query-engine failures. Do not classify these classes unconditionally for retry. Inspect available error codes/messages for known transient connectivity cases, or split the broad infrastructure classifier from a narrower transient/retryable classifier and make withInfraRetry use the latter. Add negative tests for persistent initialization and unknown request errors.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Adds two shared primitives to
@internal/databasefor surviving brief database connection blips: a classifier that recognises connectivity/infrastructure failures (as opposed to real query errors), andwithInfraRetry, a helper that retries an operation on those failures.Details
isInfrastructureError/looksLikeConnectivityErrorrecognise connection-level failures (unreachable server, closed connection, connection reset, connect timeouts) and deliberately do not match query or validation errors, so retries never mask a real bug.withInfraRetry(run, config)retriesrunon a classified infrastructure error with jittered backoff, gated by a shared token-bucket budget so a mass freeze cannot amplify into a retry storm. It has anenabledkill-switch (off by default) and reuses the existing retry-budget primitive.It must only wrap operations that are safe to run more than once (reads, or writes made idempotent); it deliberately provides no path that silently retries a non-idempotent write.