fix(client-engine-runtime): populate standard meta fields in rethrowAsUserFacing#29349
fix(client-engine-runtime): populate standard meta fields in rethrowAsUserFacing#29349Yashsingh045 wants to merge 5 commits intoprisma:mainfrom
Conversation
|
|
|
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 (2)
WalkthroughReplaced inline driver-adapter error meta with a centralized Changes
Possibly related issues
🚥 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 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 |
|
Hey @nurul3101 , |
jacek-prisma
left a comment
jacek-prisma
left a comment
There was a problem hiding this comment.
I think this deserves a regression test, you could have a look at packages/client/tests/functional/issues/29309-datetime-cursor for reference
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9d9d738f-f233-440b-921a-ab36425ac2b3
📒 Files selected for processing (3)
packages/client/tests/functional/issues/29344-adapter-constraint-meta/_matrix.tspackages/client/tests/functional/issues/29344-adapter-constraint-meta/prisma/_schema.tspackages/client/tests/functional/issues/29344-adapter-constraint-meta/tests.ts
| expect(e.code).toBe('P2002') | ||
| expect(e.meta).toBeDefined() | ||
| expect(e.meta.target).toBeDefined() | ||
| } |
There was a problem hiding this comment.
Add name assertions to match functional test error assertion standards.
The catches assert code, but the suite rule for functional tests expects asserting both error name and code.
💡 Proposed fix
} catch (e: any) {
+ expect(e.name).toBe('PrismaClientKnownRequestError')
expect(e.code).toBe('P2002')
expect(e.meta).toBeDefined()
expect(e.meta.target).toBeDefined()
}
@@
} catch (e: any) {
+ expect(e.name).toBe('PrismaClientKnownRequestError')
expect(e.code).toBe('P2003')
expect(e.meta).toBeDefined()
if (suiteConfig.provider !== Providers.SQLITE && suiteConfig.provider !== Providers.SQLSERVER) {
expect(e.meta.field_name).toBeDefined()
}
}
@@
if (caught.code === 'P2011') {
+ expect(caught.name).toBe('PrismaClientKnownRequestError')
expect(caught.meta).toBeDefined()
expect(caught.meta.target).toBeDefined()
}Also applies to: 34-38, 52-55
| test('P2011: NullConstraintViolation has meta.target', async () => { | ||
| // P2011 is not easy to trigger with Prisma abstractions if the field is typed as non-nullable | ||
| // We can use $executeRaw instead to insert a null optionally if the driver allows | ||
| // However, we just cover it using a raw query | ||
| try { | ||
| await prisma.$executeRaw`INSERT INTO "User" ("email") VALUES (NULL)` | ||
| } catch (e: any) { | ||
| // Some adapters might throw different errors for raw queries compared to client queries, | ||
| // but if it throws P2011 it should have meta.target. | ||
| // We'll optionally ignore it if the code isn't P2011. | ||
| if (e.code === 'P2011') { | ||
| expect(e.meta).toBeDefined() | ||
| expect(e.meta.target).toBeDefined() | ||
| } | ||
| } |
There was a problem hiding this comment.
P2011 test is currently non-assertive for the common raw-query path.
At Line 52, assertions run only when e.code === 'P2011', but raw adapter errors are mapped via the raw path and commonly surface as P2010. The test also has no explicit failure path if no error is thrown.
💡 Proposed fix
test('P2011: NullConstraintViolation has meta.target', async () => {
- try {
- await prisma.$executeRaw`INSERT INTO "User" ("email") VALUES (NULL)`
- } catch (e: any) {
- if (e.code === 'P2011') {
- expect(e.meta).toBeDefined()
- expect(e.meta.target).toBeDefined()
- }
- }
+ let caught: any
+ try {
+ await prisma.$executeRaw`INSERT INTO "User" ("email") VALUES (NULL)`
+ } catch (e: any) {
+ caught = e
+ }
+
+ expect(caught).toBeDefined()
+ if (caught.code === 'P2011') {
+ expect(caught.meta).toBeDefined()
+ expect(caught.meta.target).toBeDefined()
+ } else {
+ // Raw query path may map adapter errors to P2010.
+ expect(caught.code).toBe('P2010')
+ }
})
|
Hey @jacek-prisma , Added a functional regression test at The test verifies:
Tests pass locally with |
packages/client/tests/functional/issues/29344-adapter-constraint-meta/prisma/_schema.ts
Outdated
Show resolved
Hide resolved
|
Where did the idea for |
Merging this PR will not alter performance
Comparing 
Footnotes
|
hey @jacek-prisma , "Regarding the
I have now refined the implementation in
This ensures that the meta object remains accurate and strictly represents database columns, while the constraint name itself still appears correctly in the human-readable error message (handled separately)." Summary of Final Changes:
|
|
Looks like the tests need to some adjustments for your change, the CI is failing |
|
hey @jacek-prisma ,
The PR is ready for another Look |
3 participants
This PR fixes issue #29344 where MySQL unique constraint violations (P2002) were missing the meta.target field when using driver adapters.
Changes
packages/client-engine-runtime/src/user-facing-error.tsto extract standard Prisma metadata fromDriverAdapterError.meta.targetforUniqueConstraintViolation(P2002) andNullConstraintViolation(P2011).meta.field_nameforForeignKeyConstraintViolation(P2003).Verification
@prisma/client-engine-runtime. All tests passed.Summary by CodeRabbit
Bug Fixes
Tests