Conversation
📝 WalkthroughWalkthroughPrisma dependencies upgraded to version 6.19.2 with Node.js engine constraints added. Prisma schema views refactored to remove primary key designations from multiple fields. PrismaService restructured to directly extend PrismaClient with Proxy-based transaction handling and field encryption extension integration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/src/prisma/prisma.service.ts (1)
37-67: Prefer NestJS factory provider pattern over constructor return.The Proxy pattern works with NestJS lifecycle hooks (Nest calls
onModuleInit()on the returned instance), but returning from a constructor is not the idiomatic NestJS approach. Use a factory provider instead for cleaner DI integration and to avoid prototype/instanceof issues:const PRISMA_SERVICE = Symbol('PRISMA_SERVICE'); `@Module`({ providers: [ { provide: PRISMA_SERVICE, useFactory: () => { const client = new PrismaClient({ log: logConfig }); const extended = client.$extends(fieldEncryptionExtension()); return new Proxy(extended, { /* handler */ }); }, }, ], exports: [PRISMA_SERVICE], }) export class PrismaModule {}This keeps the proxy behavior while maintaining NestJS best practices for custom providers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/prisma/prisma.service.ts` around lines 37 - 67, Replace the constructor-based Proxy return with a Nest factory provider: instead of returning a Proxy from the PrismaService constructor, create a provider (e.g. PRISMA_SERVICE) with a useFactory that instantiates PrismaClient, applies the fieldEncryptionExtension() to produce extended, and then returns the Proxy wrapping extended with the existing handler that implements onModuleInit and the $transaction retry logic (using RetryPromise). Ensure the factory provider is registered in the module's providers and exported so DI consumers receive the proxied instance and Nest lifecycle hooks (onModuleInit) and instanceof behavior work correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/prisma/prisma.service.ts`:
- Line 50: The jitter argument passed to RetryPromise in the two calls using
RetryPromise(transactionPromiseFn, 10, 10000, 2000) is too large (2000) and,
given the backoff formula, can lead to multi-hour sleeps; update both call sites
that invoke RetryPromise(transactionPromiseFn, 10, 10000, 2000) to use a much
smaller fixed jitter (e.g., 200) or compute a bounded jitter relative to the
base delay (for example cap jitter to a small percentage of the base delay),
ensuring jitter cannot grow unbounded and replacing the literal 2000 in both
occurrences.
---
Nitpick comments:
In `@backend/src/prisma/prisma.service.ts`:
- Around line 37-67: Replace the constructor-based Proxy return with a Nest
factory provider: instead of returning a Proxy from the PrismaService
constructor, create a provider (e.g. PRISMA_SERVICE) with a useFactory that
instantiates PrismaClient, applies the fieldEncryptionExtension() to produce
extended, and then returns the Proxy wrapping extended with the existing handler
that implements onModuleInit and the $transaction retry logic (using
RetryPromise). Ensure the factory provider is registered in the module's
providers and exported so DI consumers receive the proxied instance and Nest
lifecycle hooks (onModuleInit) and instanceof behavior work correctly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
backend/package.jsonbackend/prisma/schema.prismabackend/src/api-logs/backup-scheduler.service.tsbackend/src/prisma/prisma.service.ts
| const transactionPromiseFn = async () => { | ||
| return target.$transaction(arg, options); | ||
| }; | ||
| return RetryPromise(transactionPromiseFn, 10, 10000, 2000); |
There was a problem hiding this comment.
Fix RetryPromise jitter argument value.
On Line 50 and Line 59, RetryPromise(transactionPromiseFn, 10, 10000, 2000) sets jitter to 2000. With your RetryPromise formula, that can produce multi-hour retry sleeps.
🐛 Proposed fix
- return RetryPromise(transactionPromiseFn, 10, 10000, 2000);
+ return RetryPromise(transactionPromiseFn, 10, 10_000, 0.2);
...
- return RetryPromise(transactionPromiseFn, 10, 10000, 2000);
+ return RetryPromise(transactionPromiseFn, 10, 10_000, 0.2);Also applies to: 59-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/prisma/prisma.service.ts` at line 50, The jitter argument passed
to RetryPromise in the two calls using RetryPromise(transactionPromiseFn, 10,
10000, 2000) is too large (2000) and, given the backoff formula, can lead to
multi-hour sleeps; update both call sites that invoke
RetryPromise(transactionPromiseFn, 10, 10000, 2000) to use a much smaller fixed
jitter (e.g., 200) or compute a bounded jitter relative to the base delay (for
example cap jitter to a small percentage of the base delay), ensuring jitter
cannot grow unbounded and replacing the literal 2000 in both occurrences.
1 participant
Summary by CodeRabbit
Release Notes
Chores
Refactor