docs: add transaction API related spec, plan and ADR#345
Conversation
📝 WalkthroughWalkthroughA new architectural decision record (ADR 187) has been added documenting transaction-scoped streaming behavior. The design uses invalidation flags and runtime error messages to prevent consuming lazy Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 docstrings
🧪 Generate unit tests (beta)
Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/middleware-telemetry
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
@prisma-next/vite-plugin-contract-emit
@prisma-next/runtime-executor
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-pipeline-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/architecture` docs/adrs/ADR 187 - Transaction-scoped streams use
invalidation not buffering.md:
- Around line 50-58: Trim the implementation-specific driver internals from "ADR
187 - Transaction-scoped streams use invalidation not buffering": remove
sentences that describe low-level library behavior (e.g., the node-postgres
socket handling, tokio-postgres futures/polling model, and exact cursor
execution mechanics) and replace them with a concise decision-level statement
that the ADR assumes drivers will eagerly surface server errors and that cursor
semantics are a separate concern; move the removed details into a linked
investigation or subsystem doc (e.g., "driver-internals" notes) and add a brief
pointer/link from the ADR to that doc so readers can follow up for
implementation specifics.
- Around line 19-49: Add a single framing sentence immediately before the
"Alternatives considered" list that states the explicit comparison axis (the
invariant/trade-off) being evaluated — e.g., whether each option favors
preserving lazy/streaming semantics inside transactions versus enforcing eager
safety and runtime/type complexity — so readers understand that all alternatives
are being compared on the same knob; place this sentence above the "Eager
buffering inside transactions" subsection and reference the options named there
("Eager buffering inside transactions", "Type-level prevention", "Runtime error
before commit (tracking approach)", "Do nothing (document the pitfall)") to make
clear which approaches are being evaluated against that axis.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: ffda6f75-9532-43db-9bd2-d87b0a8762b1
⛔ Files ignored due to path filters (2)
projects/orm-client-transaction-api/plan.mdis excluded by!projects/**projects/orm-client-transaction-api/spec.mdis excluded by!projects/**
📒 Files selected for processing (1)
docs/architecture docs/adrs/ADR 187 - Transaction-scoped streams use invalidation not buffering.md
| ## Alternatives considered | ||
|
|
||
| ### Eager buffering inside transactions | ||
|
|
||
| `tx.execute` would internally drain all rows to an array, then wrap them in a pre-materialized `AsyncIterableResult`. Same return type, no leak possible. | ||
|
|
||
| **Rejected because:** it silently changes `execute()` semantics inside transactions (always eager) vs outside (lazy/streaming). This is surprising, increases memory pressure, and creates a behavioral difference that is invisible at the type level. Streaming inside transactions is not inherently wrong — the user may want to process a large result set row-by-row within the transaction without materializing all rows in memory. | ||
|
|
||
| ### Type-level prevention | ||
|
|
||
| Constrain the callback's return type to exclude `AsyncIterableResult`: | ||
|
|
||
| ```typescript | ||
| type NoAsyncIterable<T> = T extends AsyncIterable<unknown> ? never : T; | ||
| transaction<R>(fn: (tx: TxCtx) => PromiseLike<NoAsyncIterable<R>>): Promise<R>; | ||
| ``` | ||
|
|
||
| **Rejected because:** conditional types on generic return types interact poorly with inference — `R` may resolve to `never` instead of producing a useful error. Cannot catch `AsyncIterableResult` nested inside returned objects. Adds type complexity without runtime safety. | ||
|
|
||
| ### Runtime error before commit (tracking approach) | ||
|
|
||
| Track every `AsyncIterableResult` created by the transaction. Before commit, check if any are unconsumed and throw. | ||
|
|
||
| **Rejected because:** it is an error to have an unconsumed stream only if the user tries to read from it after the transaction — an unconsumed stream that is simply discarded is harmless. Throwing before commit would reject valid patterns where the user intentionally ignores a result. | ||
|
|
||
| ### Do nothing (document the pitfall) | ||
|
|
||
| The direct return case (`return tx.execute(plan)`) is already safe because `PromiseLike` causes auto-drain. Only the "wrap in object" case is hazardous. | ||
|
|
||
| **Rejected because:** `return { users: tx.execute(plan1), posts: tx.execute(plan2) }` is a natural and common pattern. Relying on documentation for a failure mode that produces confusing errors (connection gone, recycled connection) is insufficient. | ||
|
|
There was a problem hiding this comment.
Make the alternatives comparison axis explicit.
This section compares options, but it does not explicitly state the shared axis (invariant/trade-off) up front. Add one short framing sentence before the alternatives list.
Suggested doc tweak
## Alternatives considered
+Compared on a shared axis: semantic consistency of `execute()`, runtime safety, memory profile, and developer ergonomics.
+
### Eager buffering inside transactionsAs per coding guidelines: "Research comparisons must explicitly state the shared axis (knob/invariant/trade-off) being compared."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/architecture` docs/adrs/ADR 187 - Transaction-scoped streams use
invalidation not buffering.md around lines 19 - 49, Add a single framing
sentence immediately before the "Alternatives considered" list that states the
explicit comparison axis (the invariant/trade-off) being evaluated — e.g.,
whether each option favors preserving lazy/streaming semantics inside
transactions versus enforcing eager safety and runtime/type complexity — so
readers understand that all alternatives are being compared on the same knob;
place this sentence above the "Eager buffering inside transactions" subsection
and reference the options named there ("Eager buffering inside transactions",
"Type-level prevention", "Runtime error before commit (tracking approach)", "Do
nothing (document the pitfall)") to make clear which approaches are being
evaluated against that axis.
| ## Why this is safe: wire protocol analysis | ||
|
|
||
| A separate concern was whether unconsumed result streams could hide database errors (e.g., constraint violations) within a transaction, allowing a COMMIT that should have been a ROLLBACK. Investigation confirmed this is not an issue: | ||
|
|
||
| - **Not a wire protocol issue.** The Postgres wire protocol is push-oriented — the server sends `ErrorResponse` immediately. The delayed error surfacing observed in the Rust tokio-postgres driver was an architectural issue specific to that library's futures/polling model, not a protocol limitation. | ||
| - **node-postgres reads eagerly.** The `pg` library attaches `stream.on('data', ...)` to the socket. The event loop drains all server responses regardless of whether user code has consumed the result. | ||
| - **DML executes fully.** INSERT/UPDATE/DELETE (with or without RETURNING) executes atomically — cursors (DECLARE CURSOR) are SELECT/VALUES-only. Mutation errors are always surfaced. | ||
| - **Failed transaction state is server-side.** If any statement errors, the server marks the transaction as aborted. A subsequent COMMIT returns the command tag ROLLBACK regardless of whether the client read the original error. | ||
| - **Cursors defer execution, not errors.** With cursors, unfetched SELECT rows are genuinely never evaluated by the server — this is not error suppression but incomplete execution. For pure reads, this is harmless. For SELECT FOR UPDATE, locks are acquired per-FETCH, but this is only meaningful if the user reads the results to act on them within the same transaction — partial consumption of a FOR UPDATE query is already an application logic bug independent of the streaming API. |
There was a problem hiding this comment.
Trim implementation-specific driver internals from ADR scope.
This section includes low-level library behavior details (e.g., exact pg socket handling) that read like implementation notes. Keep ADR text at decision/constraint level and move deep driver mechanics to subsystem docs or linked investigation notes.
Based on learnings: "ADRs in prisma/prisma-next should document architectural design decisions and the key constraints/assumptions only. Do not include implementation algorithms...".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/architecture` docs/adrs/ADR 187 - Transaction-scoped streams use
invalidation not buffering.md around lines 50 - 58, Trim the
implementation-specific driver internals from "ADR 187 - Transaction-scoped
streams use invalidation not buffering": remove sentences that describe
low-level library behavior (e.g., the node-postgres socket handling,
tokio-postgres futures/polling model, and exact cursor execution mechanics) and
replace them with a concise decision-level statement that the ADR assumes
drivers will eagerly surface server errors and that cursor semantics are a
separate concern; move the removed details into a linked investigation or
subsystem doc (e.g., "driver-internals" notes) and add a brief pointer/link from
the ADR to that doc so readers can follow up for implementation specifics.
PR Stack: Transaction API
maindb.transaction()+tx.sql#345tx.orminto transactions#346Part of https://linear.app/prisma-company/issue/TML-1912/orm-transaction-support
Summary
Adds the foundational design documents for the ORM Client Transaction API (
db.transaction(callback)):db.transaction()onPostgresClientSpec highlights
The API follows a callback-scoped pattern — no dangling transactions, guaranteed cleanup:
The transaction context exposes three surfaces:
tx.orm— full ORM collection API bound to the transaction connectiontx.sql— typed SQL builder (Db<TContract>) bound to the same connectiontx.execute(plan)— execute a built plan directlyExplicit non-goals: configurable isolation levels, savepoints/nesting, automatic retry, interactive begin/commit/rollback handle, timeout, MongoDB support.
Plan
withTransactionhelper,db.transaction(),tx.sql, unit + e2e teststx.ormwiring,withMutationScopefallback, nested-mutation e2e testsPromiseLikedrain, post-commit invalidation, lazy connectADR 187: Transaction-scoped streams use invalidation, not buffering
Decision: When a transaction ends, the scoped
RuntimeQueryablesets aninvalidatedflag. AnyAsyncIterableResultconsumed after that point throws a clear error rather than hitting a released connection.Rejected alternatives: eager buffering (silently changes semantics), type-level prevention (breaks inference), pre-commit tracking (rejects valid patterns), do nothing (confusing errors).
The ADR includes a wire-protocol safety analysis confirming that unconsumed result streams cannot hide database errors or cause an incorrect COMMIT in Postgres.
Files
projects/orm-client-transaction-api/spec.md(new)projects/orm-client-transaction-api/plan.md(new)docs/architecture docs/adrs/ADR 187 - Transaction-scoped streams use invalidation not buffering.md(new)Summary by CodeRabbit