feat(node): Support Prisma 8 in prismaIntegration - #24682
Conversation
|
bugbot run |
| end(error); | ||
| return Promise.reject(error); | ||
| } | ||
| return settle(runUnderSpan(span, () => iterator.throw!(error))); |
There was a problem hiding this comment.
Uncommented non-null assertions
Low Severity
iterator.return! and iterator.throw! are non-null assertions with no comment on why a narrower type is not possible. Flagged because the review rules require that explanation on new ! assertions in SDK source.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 4e7097b. Configure here.
size-limit report 📦
|
4e7097b to
685f3c5
Compare
|
bugbot run |
| expect(operationSpans).toEqual([]); | ||
| querySpans.forEach(span => expect(span.parent_span_id).toBe(segment?.span_id)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Test branches on instrumentation mode
Low Severity
The new Prisma v8 test branches on expectsOperationSpans and returns early for the CJS-on-older-Node path. That hides two different user-visible outcomes in one case: operation spans nested over pg queries versus pg-only spans parented to the transaction. This was flagged because the review rules call out conditionals in a single test.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 685f3c5. Configure here.
685f3c5 to
3e27580
Compare
|
bugbot run |
3e27580 to
268cb2f
Compare
|
bugbot run |
| (createRunner, test) => { | ||
| test('should instrument PostgreSQL queries from Prisma ORM via pg', { timeout: 75_000 }, async () => { | ||
| (createRunner, test, mode) => { | ||
| if (mode === 'cjs' && !cjsLoadsInstrumentedEsm) { |
There was a problem hiding this comment.
q: Can't we use a conditionalTest({ min: 22 }) for this instead?
There was a problem hiding this comment.
Done: conditionalTest({ min: 22 }) and conditionalTest({ max: 21 }) now pick the two CJS variants; the manual version parsing is gone.
268cb2f to
ed14806
Compare
|
bugbot run |
|
bugbot run |
Prisma 8 has no tracing surface, so wrap its ORM terminals via orchestrion instead. Each ORM call gets a `prisma:client:operation` span (`db.operation.name`, `db.collection.name`, plus v7's `method`, `model` and `name`) with the `pg` query spans nested underneath. No user code is needed. The patch point is `CollectionImpl` in the transitive `@prisma/orm-family-sql` package, pinned to `>=8.0.0-rc.8 <9`. Lazy terminals (`all`, `createAll`, `updateAll`, `deleteAll`) return a single-use thenable that orchestrion's promise wrapper would consume, so they are wrapped `Sync` and the subscriber hooks `toArray` and the async iterator instead. Terminals calling terminals reuse the outer span. Operation spans require a parent span. CommonJS apps load the ESM-only runtime through `require(esm)`, which only the synchronous module hooks (Node 24.13 / 25.1) transform; older Node keeps `pg`-only spans in CJS. ESM apps are fully covered. Fixes #24237 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ed14806 to
8f706a5
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8f706a5. Configure here.


Wraps Prisma 8's ORM terminals via orchestrion: each ORM call gets a
prisma:client:operationspan (sentry.op: db,db.operation.name,db.collection.name, plus v7'smethod/model/name) with thepgquery spans nested underneath. No user code needed.CollectionImplin the transitive@prisma/orm-family-sql, pinned>=8.0.0-rc.8 <9; verified on rc.8.all,createAll,updateAll,deleteAll) return a single-use thenable that orchestrion's promise wrapper would consume, so they're wrappedSyncand the subscriber hooks consumption instead.pg-only spans, sincerequire(esm)bypasses the module hooks there. From Node 22 on they're covered; with the async hooks (before Node 24.13 / 25.1) a call in the same tick as therequireruns before the SDK subscribes. ESM apps are fully covered.Fixes #24237