Fix 29271#29303
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds optional maxWait and timeout to batch transaction types and runtime, and consolidates the generated client batching Changes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
8af34a9 to
1dc0f6c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client/src/runtime/core/types/exported/Extensions.ts (1)
298-302: 🧹 Nitpick | 🔵 TrivialExtract the duplicated transaction options shape into a shared interface.
The same inline object type appears in both
$transactionoverloads (lines 298 and 302). Lift it into a reusable interface to avoid drift and to match the repository's TypeScript guideline for defining object shapes.♻️ Suggested refactor
+export interface DynamicTransactionOptions<TypeMap extends TypeMapDef> { + maxWait?: number + timeout?: number + isolationLevel?: TypeMap['meta']['txIsolationLevel'] +} + export type DynamicClientExtensionThisBuiltin< TypeMap extends TypeMapDef, TypeMapCb extends TypeMapCbDef, ExtArgs extends Record<string, any>, > = { $extends: ExtendsHook<'extends', TypeMapCb, ExtArgs, Call<TypeMapCb, { extArgs: ExtArgs }>> $transaction<P extends PrismaPromise<any>[]>( arg: [...P], - options?: { maxWait?: number; timeout?: number; isolationLevel?: TypeMap['meta']['txIsolationLevel'] }, + options?: DynamicTransactionOptions<TypeMap>, ): Promise<UnwrapTuple<P>> $transaction<R>( fn: (client: Omit<DynamicClientExtensionThis<TypeMap, TypeMapCb, ExtArgs>, ITXClientDenyList>) => Promise<R>, - options?: { maxWait?: number; timeout?: number; isolationLevel?: TypeMap['meta']['txIsolationLevel'] }, + options?: DynamicTransactionOptions<TypeMap>, ): Promise<R>Per coding guidelines:
**/*.{ts,tsx}: Useinterfacefor defining object shapes in TypeScript.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b02a9ec5-75e1-443d-b6cc-2937baaf32a7
📒 Files selected for processing (11)
packages/client-generator-js/src/TSClient/PrismaClient.tspackages/client-generator-ts/src/TSClient/PrismaClient.tspackages/client/src/runtime/RequestHandler.tspackages/client/src/runtime/core/engines/client/ClientEngine.tspackages/client/src/runtime/core/engines/common/Engine.tspackages/client/src/runtime/core/request/PrismaPromise.tspackages/client/src/runtime/core/types/exported/Extensions.tspackages/client/src/runtime/getPrismaClient.tspackages/client/tests/functional/issues/29271-batch-tx-timeout/_matrix.tspackages/client/tests/functional/issues/29271-batch-tx-timeout/prisma/_schema.tspackages/client/tests/functional/issues/29271-batch-tx-timeout/tests.ts
1dc0f6c to
bed05b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/client/src/runtime/core/engines/client/ClientEngine.ts (1)
573-578:⚠️ Potential issue | 🟠 Major
maxWait/timeoutare still only materialized formultibatches.This branch is the only place in
requestBatch()that turns batchtransaction.optionsintothis.transaction('start', {}, txOptions). Thecompactedbranch skips that step and runs withtxInfo === undefined, so the new batch options still depend on which batch strategy the compiler picks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fb4a8102-25ff-437f-94b1-cb0ae2853693
📒 Files selected for processing (8)
packages/client-generator-js/src/TSClient/PrismaClient.tspackages/client-generator-ts/src/TSClient/PrismaClient.tspackages/client/src/runtime/RequestHandler.tspackages/client/src/runtime/core/engines/client/ClientEngine.tspackages/client/src/runtime/core/engines/common/Engine.tspackages/client/src/runtime/core/request/PrismaPromise.tspackages/client/src/runtime/core/types/exported/Extensions.tspackages/client/src/runtime/getPrismaClient.ts
jacek-prisma
left a comment
There was a problem hiding this comment.
I think this deserves to be covered with a test in packages/client/tests/functional/interactive-transactions/tests.ts
Addressed review comments Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
3013044 to
1f35e99
Compare
addressed review comments
|
Thanks for the feedback @jacek-prisma, I just added one. Let me know if there's anything else I can do 👍 |
|
Hi @jacek-prisma! Any updates on the review? |
Closes #29271
Summary by CodeRabbit
New Features
$transactionbatching path now accepts a single optional options object exposing maxWait and timeout, and (when applicable) isolationLevel.Tests