feat: sequential transcation support (#30)#31
Conversation
* feat: sequential transcation support * fix comments
📝 WalkthroughWalkthroughThe PR increments the package version to 0.4.0, adds the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 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. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
Adds a new sequential transaction endpoint to the proxy server so multiple Prisma/ZendStack model operations can be executed in order within a single $transaction, with SuperJSON (de)serialization support.
Changes:
- Introduce a
/api/model/$transaction/sequentialPOST endpoint that validates and executes a list of model operations inside an interactive Prisma transaction. - Add SuperJSON support to deserialize request args and serialize transaction results.
- Bump package version and add
superjsondependency (plus lockfile updates).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/server.ts |
Adds sequential transaction handler, request validation, and SuperJSON (de)serialization; registers a new Express route. |
package.json |
Bumps version to 0.4.0 and adds superjson dependency. |
pnpm-lock.yaml |
Locks new dependency tree for superjson and transitive packages. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| processedOps.push({ | ||
| model: lowerCaseFirst(itemModel), | ||
| op: itemOp, | ||
| args: processRequestPayload(itemArgs), |
There was a problem hiding this comment.
args is allowed to be null and is passed through to Prisma delegate calls. Prisma client methods generally expect args to be an object or undefined; passing null will typically throw at runtime. Consider rejecting null in validation or normalizing null to undefined before invoking the operation.
| processedOps.push({ | |
| model: lowerCaseFirst(itemModel), | |
| op: itemOp, | |
| args: processRequestPayload(itemArgs), | |
| const normalizedArgs = itemArgs === null ? undefined : processRequestPayload(itemArgs) | |
| processedOps.push({ | |
| model: lowerCaseFirst(itemModel), | |
| op: itemOp, | |
| args: normalizedArgs, |
| app.post('/api/model/\\$transaction/sequential', async (_req, res) => { | ||
| const response = await handleTransaction( | ||
| modelMeta, | ||
| enhanceFunc( | ||
| prisma, | ||
| {}, | ||
| { | ||
| kinds: Enhancements, | ||
| } | ||
| ), | ||
| _req.body | ||
| ) |
There was a problem hiding this comment.
The request object is named _req but is actively used (_req.body). Since the underscore prefix typically indicates an intentionally unused parameter, rename it to req to avoid confusion.
| } | ||
|
|
||
| function isValidModel(modelMeta: any, modelName: string): boolean { | ||
| return lowerCaseFirst(modelName) in modelMeta.models |
There was a problem hiding this comment.
isValidModel uses the in operator with untrusted input. This can return true for prototype properties like __proto__/constructor, potentially allowing unexpected property access when later indexing tx[model]. Use an own-property check (e.g., Object.hasOwn(...) / hasOwnProperty.call) against modelMeta.models instead of in.
| return lowerCaseFirst(modelName) in modelMeta.models | |
| const name = lowerCaseFirst(modelName) | |
| return Object.prototype.hasOwnProperty.call(modelMeta.models, name) |
|
|
||
| // ZenStack API endpoint | ||
|
|
||
| app.post('/api/model/\\$transaction/sequential', async (_req, res) => { |
There was a problem hiding this comment.
The route is registered as '/api/model/\\$transaction/sequential', which evaluates to a path containing a literal backslash (/api/model/\$transaction/sequential). Requests to /api/model/$transaction/sequential won’t match. Use a literal $ in the route string instead.
| app.post('/api/model/\\$transaction/sequential', async (_req, res) => { | |
| app.post('/api/model/$transaction/sequential', async (_req, res) => { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/server.ts`:
- Around line 307-312: In the catch block that handles the "$transaction"
request, stop returning the raw backend error message and avoid treating all
failures as 500; instead log the full error via console.error (or processLogger)
including err and stack, map expected client-side errors to appropriate 4xx
responses (e.g., detect types like ValidationError, BadRequestError or an
err.status/statusCode property) and return makeError with a generic "Transaction
failed" message and a 500 status for true server errors; keep references to the
existing symbols (the catch(err) block around the "$transaction" handler and the
makeError call) and implement detection of known client error classes or status
codes to return proper 4xx codes while always logging detailed err internally.
- Around line 243-245: processRequestPayload is invoked before the surrounding
try block and can throw, which causes unhandled rejections when
handleTransaction is called from the async route handler; wrap the call to
processRequestPayload(itemArgs) in a try-catch (or move it inside the existing
try) and on error forward the exception to Express (e.g., call next(err) or
rethrow so the outer try/catch handles it) so that malformed meta.serialization
doesn't produce an unhandled rejection; update the code around
processRequestPayload, handleTransaction, and the route handler to ensure thrown
errors are caught and propagated to Express's error handling.
- Around line 24-41: The current static VALID_OPS whitelist can include
operations not available on the runtime Prisma delegate; update the request
validation to ensure the requested op is both in VALID_OPS and actually exists
as a function on the resolved delegate before invoking it. In the handler that
resolves the delegate (the code that gets the model delegate, e.g.
prisma[modelName] or the variable named delegate) add a runtime check like
typeof delegate[op] === 'function' and if false return a 4xx validation error
(bad request) instead of proceeding; only call delegate[op](...) when that check
passes. This ensures ops like createManyAndReturn/updateManyAndReturn are
rejected when not present on the loaded delegate.
🪄 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.yaml
Review profile: CHILL
Plan: Pro
Run ID: fbf6ae78-82d2-4e81-82a1-d02b7856007b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.jsonsrc/server.ts
| const VALID_OPS = new Set([ | ||
| 'findMany', | ||
| 'findUnique', | ||
| 'findFirst', | ||
| 'create', | ||
| 'createMany', | ||
| 'createManyAndReturn', | ||
| 'update', | ||
| 'updateMany', | ||
| 'updateManyAndReturn', | ||
| 'upsert', | ||
| 'delete', | ||
| 'deleteMany', | ||
| 'count', | ||
| 'aggregate', | ||
| 'groupBy', | ||
| 'exists', | ||
| ]) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Prisma expose the same model delegate methods across all supported providers and versions, or are operations such as createManyAndReturn/updateManyAndReturn provider- or version-dependent?
💡 Result:
Prisma does not expose an identical set of model-delegate CRUD methods/behaviors across all providers and versions—some operations are provider- and/or version-dependent.
- Version-dependent:
createManyAndReturn()/updateManyAndReturn()are only available starting in Prisma ORM 5.14.0 (and later). [1] - Provider-dependent: those “AndReturn” bulk operations are supported only on PostgreSQL, CockroachDB, and SQLite (not on MySQL, SQL Server, MongoDB, etc.). [1][2]
- Even when a method exists, capabilities can differ by provider (e.g.,
createMany({ skipDuplicates: true })is not supported on some connectors like SQLite). [1]
So if you rely on createManyAndReturn / updateManyAndReturn, you must gate it by Prisma version and datasource provider (and regenerate Prisma Client after upgrades/provider changes). [1][2]
Sources:
[1] Prisma Docs – CRUD (v6) (notes provider + “available in 5.14.0 and later”) (docs.prisma.io)
[2] Prisma Docs – Prisma Client API reference (method-level support list) (docs.prisma.io)
Citations:
- 1: https://docs.prisma.io/docs/v6/orm/prisma-client/queries/crud?utm_source=openai
- 2: https://docs.prisma.io/docs/orm/reference/prisma-client-reference?utm_source=openai
Validate operations against the actual loaded delegate, not only VALID_OPS.
The whitelist is static, but the Prisma delegate surface is provider- and version-dependent. Operations like createManyAndReturn and updateManyAndReturn are only available in Prisma 5.14.0+, and only on PostgreSQL, CockroachDB, and SQLite—not on MySQL, SQL Server, or MongoDB. If an op is in VALID_OPS but unavailable on the current client instance, the code at Line 293 will call undefined(...), resulting in a 500 error instead of validation failure.
The suggested fix prevents this by checking the delegate at request time before execution.
Suggested change
- if (!VALID_OPS.has(itemOp)) {
+ const model = lowerCaseFirst(itemModel)
+ const delegate = (client as any)[model]
+ if (!VALID_OPS.has(itemOp) || typeof delegate?.[itemOp] !== 'function') {
return makeError(`operation at index ${i} has invalid op: ${itemOp}`)
}
@@
processedOps.push({
- model: lowerCaseFirst(itemModel),
+ model,
op: itemOp,
args: processRequestPayload(itemArgs),
})Also applies to: Lines 268–269, 293
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server.ts` around lines 24 - 41, The current static VALID_OPS whitelist
can include operations not available on the runtime Prisma delegate; update the
request validation to ensure the requested op is both in VALID_OPS and actually
exists as a function on the resolved delegate before invoking it. In the handler
that resolves the delegate (the code that gets the model delegate, e.g.
prisma[modelName] or the variable named delegate) add a runtime check like
typeof delegate[op] === 'function' and if false return a 4xx validation error
(bad request) instead of proceeding; only call delegate[op](...) when that check
passes. This ensures ops like createManyAndReturn/updateManyAndReturn are
rejected when not present on the loaded delegate.
| if (meta?.serialization) { | ||
| // superjson deserialization | ||
| return SuperJSON.deserialize({ json: rest, meta: meta.serialization }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Express 4 automatically convert rejected promises from async route handlers into error responses, or do they need to be caught and passed to next(err) manually?
💡 Result:
No, Express 4 does not automatically convert rejected promises from async route handlers into error responses. They need to be caught manually and passed to next(err). In Express 4, only synchronous errors in route handlers are automatically caught by Express. For asynchronous code, including rejected promises from async/await handlers, developers must explicitly handle errors using try-catch blocks or .catch(next) and call next(err) to propagate them to Express error handling middleware. Unhandled rejections will not trigger Express error handlers and may cause the process to emit unhandledRejection events or crash. This behavior changes in Express 5, where rejected promises from async route handlers automatically call next with the error. However, since the query specifies Express 4 and both 4.x and 5.x are actively maintained as of 2026, Express 4 requires manual error handling for async routes.
Citations:
- 1: https://expressjs.com/en/guide/error-handling
- 2: https://expressjs.com/en/guide/error-handling.html
- 3: https://stackoverflow.com/questions/55504066/how-to-gracefully-handle-promise-rejection-in-express
- 4: https://oneuptime.com/blog/post/2026-01-25-handle-async-await-errors-properly/view
- 5: https://oneuptime.com/blog/post/2026-01-26-express-error-handling/view
- 6: Support catching rejected promises in middleware functions expressjs/express#3604
- 7: https://stackoverflow.com/questions/49417580/express-middleware-cannot-trap-errors-thrown-by-async-await-but-why
- 8: https://stackoverflow.com/questions/77596099/how-to-stop-express-server-from-exiting-after-error-in-synchronous-code
- 9: https://expressjs.com/en/4x/api.html
🏁 Script executed:
# First, find the server.ts file
fd -n "server.ts" -t f
# Then read the relevant sectionRepository: zenstackhq/proxy
Length of output: 290
🏁 Script executed:
# Find server.ts file
fd "server\.ts" src/
# Then read the relevant section with line numbers
cat -n src/server.ts | sed -n '240,295p'Repository: zenstackhq/proxy
Length of output: 2431
🏁 Script executed:
# Check what processRequestPayload returns and if it can throw
cat -n src/server.ts | sed -n '235,249p'
# Search for other calls to processRequestPayload
rg "processRequestPayload" src/Repository: zenstackhq/proxy
Length of output: 651
🏁 Script executed:
# Search for where handleTransaction is called
rg "handleTransaction" src/
# Also check if there's a route handler wrapping it
rg -A 5 -B 5 "handleTransaction" src/ | head -80Repository: zenstackhq/proxy
Length of output: 1227
🏁 Script executed:
# Find the exact line where handleTransaction is called
rg -n "handleTransaction" src/server.ts
# Then read that sectionRepository: zenstackhq/proxy
Length of output: 199
🏁 Script executed:
# Read the route handler containing the handleTransaction call
cat -n src/server.ts | sed -n '345,380p'Repository: zenstackhq/proxy
Length of output: 1015
🏁 Script executed:
# Check SuperJSON imports and usage
rg "SuperJSON" src/
# Also check if there's error handling elsewhere in the codebase for async route handlers
rg -B 3 -A 3 "async.*req.*res.*=>" src/server.ts | head -100Repository: zenstackhq/proxy
Length of output: 456
Wrap processRequestPayload in try-catch to prevent unhandled rejection in async route handler.
processRequestPayload(itemArgs) can throw on malformed meta.serialization at line 285, before the try block at line 289. Since handleTransaction is async and called without error handling in the route handler (line 351), the unhandled rejection will not be automatically converted to the JSON error response by Express 4.
Suggested change
- processedOps.push({
- model: lowerCaseFirst(itemModel),
- op: itemOp,
- args: processRequestPayload(itemArgs),
- })
+ let processedArgs: unknown
+ try {
+ processedArgs = processRequestPayload(itemArgs)
+ } catch {
+ return makeError(`operation at index ${i} has invalid serialization metadata`)
+ }
+
+ processedOps.push({
+ model: lowerCaseFirst(itemModel),
+ op: itemOp,
+ args: processedArgs,
+ })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server.ts` around lines 243 - 245, processRequestPayload is invoked
before the surrounding try block and can throw, which causes unhandled
rejections when handleTransaction is called from the async route handler; wrap
the call to processRequestPayload(itemArgs) in a try-catch (or move it inside
the existing try) and on error forward the exception to Express (e.g., call
next(err) or rethrow so the outer try/catch handles it) so that malformed
meta.serialization doesn't produce an unhandled rejection; update the code
around processRequestPayload, handleTransaction, and the route handler to ensure
thrown errors are caught and propagated to Express's error handling.
| } catch (err) { | ||
| console.error('error occurred when handling "$transaction" request:', err) | ||
| return makeError( | ||
| 'Transaction failed: ' + (err instanceof Error ? err.message : String(err)), | ||
| 500 | ||
| ) |
There was a problem hiding this comment.
Don't return raw backend exception text in every 500 response.
This branch turns all execution failures into 500 and echoes err.message back to the caller. That leaks internal query/schema details and misclassifies request errors as server faults. Keep the detailed error in logs, but return a generic 500 body after mapping expected client-side failures to 4xx.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server.ts` around lines 307 - 312, In the catch block that handles the
"$transaction" request, stop returning the raw backend error message and avoid
treating all failures as 500; instead log the full error via console.error (or
processLogger) including err and stack, map expected client-side errors to
appropriate 4xx responses (e.g., detect types like ValidationError,
BadRequestError or an err.status/statusCode property) and return makeError with
a generic "Transaction failed" message and a 500 status for true server errors;
keep references to the existing symbols (the catch(err) block around the
"$transaction" handler and the makeError call) and implement detection of known
client error classes or status codes to return proper 4xx codes while always
logging detailed err internally.
feat: sequential transcation support
fix comments
Summary by CodeRabbit