-
-
Notifications
You must be signed in to change notification settings - Fork 299
fix: auto-retry database connection after server restart (#340) #342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@Razano26 is attempting to deploy a commit to the kmkoushik's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds automatic retry logic to the Prisma client in apps/web/src/server/db.ts. Introduces constants for max retries and delays, a set of retryable error codes, and an 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/src/server/db.ts">
<violation number="1" location="apps/web/src/server/db.ts:22">
P2: All PrismaClientInitializationError instances are retried, including permanent misconfigurations (invalid credentials/URL), causing pointless retry delays instead of failing fast.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/server/db.ts`:
- Around line 52-105: The retry wrapper currently retries every Prisma operation
in PrismaClient.$extends -> query.$allOperations (function $allOperations),
which can re-run non-idempotent mutations; update $allOperations to only perform
the retry loop for read-only operations (e.g., findUnique, findUniqueOrThrow,
findFirst, findFirstOrThrow, findMany, count, aggregate, groupBy) by checking
the incoming operation string against that allowlist before entering the
for-loop; for all other operations (creates/updates/deletes) skip retries and
immediately call and return await query(args) (or let errors bubble) so
non-idempotent mutations are not retried. Ensure isRetryableError, MAX_RETRIES,
calculateDelay, sleep and logging remain unchanged for the read-only retry path.
🧹 Nitpick comments (1)
apps/web/src/server/db.ts (1)
17-39: Consider tightening the message heuristics.Broad substrings like
"connection"can match non-transient configuration errors (e.g., invalid connection string), leading to unnecessary retries. Narrowing the match or adding a small denylist would reduce false positives.
Signed-off-by: Louis Labeyrie <labeyrielouis@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/server/db.ts`:
- Around line 29-55: The fallback string matching in isRetryableError is too
broad (matching "connection"/"socket"/"timeout") and can cause false positives;
narrow it to explicit network error signatures instead. Update the Error branch
in isRetryableError to check for specific tokens/phrases such as "ec
CONNREFUSED"/"econnrefused"/"connection refused", "etimedout"/"timed out",
"econnreset"/"connection reset", "socket hang up", "getaddrinfo
ENOTFOUND"/"enotfound", "ehostunreach"/"host unreachable",
"econnaborted"/"connection aborted" (or an equivalent regex that matches these
exact network error patterns) instead of generic
"connection"/"socket"/"timeout"; keep the existing Prisma checks
(PrismaClientKnownRequestError, PrismaClientInitializationError and
RETRYABLE_ERROR_CODES) unchanged. Ensure the string comparisons are
case-insensitive and avoid matching generic words like "connection" used in
validation messages.
| function isRetryableError(error: unknown): boolean { | ||
| if (error instanceof Prisma.PrismaClientKnownRequestError) { | ||
| return RETRYABLE_ERROR_CODES.has(error.code); | ||
| } | ||
|
|
||
| if (error instanceof Prisma.PrismaClientInitializationError) { | ||
| // Only retry transient connection errors, not permanent misconfigurations | ||
| // (e.g., invalid credentials, wrong database URL, schema errors) | ||
| return ( | ||
| error.errorCode !== undefined && RETRYABLE_ERROR_CODES.has(error.errorCode) | ||
| ); | ||
| } | ||
|
|
||
| if (error instanceof Error) { | ||
| const message = error.message.toLowerCase(); | ||
| return ( | ||
| message.includes("econnrefused") || | ||
| message.includes("etimedout") || | ||
| message.includes("econnreset") || | ||
| message.includes("connection") || | ||
| message.includes("socket") || | ||
| message.includes("timeout") | ||
| ); | ||
| } | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tighten the fallback string match to avoid retrying unrelated errors.
The broad "connection"/"socket" checks can match non-network errors (e.g., validation messages mentioning a connection field), causing unnecessary retries and noisy logs. Consider narrowing to explicit network signatures.
🔧 Suggested refinement
- return (
- message.includes("econnrefused") ||
- message.includes("etimedout") ||
- message.includes("econnreset") ||
- message.includes("connection") ||
- message.includes("socket") ||
- message.includes("timeout")
- );
+ return (
+ message.includes("econnrefused") ||
+ message.includes("etimedout") ||
+ message.includes("econnreset") ||
+ message.includes("socket hang up") ||
+ message.includes("timeout")
+ );🤖 Prompt for AI Agents
In `@apps/web/src/server/db.ts` around lines 29 - 55, The fallback string matching
in isRetryableError is too broad (matching "connection"/"socket"/"timeout") and
can cause false positives; narrow it to explicit network error signatures
instead. Update the Error branch in isRetryableError to check for specific
tokens/phrases such as "ec CONNREFUSED"/"econnrefused"/"connection refused",
"etimedout"/"timed out", "econnreset"/"connection reset", "socket hang up",
"getaddrinfo ENOTFOUND"/"enotfound", "ehostunreach"/"host unreachable",
"econnaborted"/"connection aborted" (or an equivalent regex that matches these
exact network error patterns) instead of generic
"connection"/"socket"/"timeout"; keep the existing Prisma checks
(PrismaClientKnownRequestError, PrismaClientInitializationError and
RETRYABLE_ERROR_CODES) unchanged. Ensure the string comparisons are
case-insensitive and avoid matching generic words like "connection" used in
validation messages.
|
Max retries upto 1 minute is the best option |
Summary by cubic
Automatically retries Prisma read-only database operations after server restarts to recover from connection drops and reduce timeouts. Uses capped exponential backoff with jitter and clearer logging.
Written for commit a8e9b5b. Summary will update on new commits.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.