driver-adapters: Ensure error propogation works for startTransaction
#4267
Conversation
`startTransaction` was not correctly wrapped when converting `Adapter` to `ErrorCapturingAdapter`. As a result, `GenericError` message was returned in case of JS error. This PR fixes the problem. Since it is hard to test those kinds of errors with real drivers, new test suite for the error propogation is introduced. It uses fake postgres adapter that throws on every call.
SevInf
changed the title
startTransactionstartTransaction
CodSpeed Performance ReportMerging #4267 will degrade performances by 5.51%Comparing Summary
Benchmarks breakdown
|
| } | ||
| } | ||
| }) | ||
| const resultSet = await this.doQuery({ |
There was a problem hiding this comment.
TODO in a separate PR: remove this class
There was a problem hiding this comment.
Thanks for the fix and for adding the tests. Changes look correct.
Now I have a more general question.
Should we keep the smoke tests in the light of the query-engine test suite exercising each of the adapters? I think we shouldn’t given the coverage from connector-kit-rs using the js executor, and also the presence of tests in prisma/prisma. WDYT?
|
They are a quick tool to check things when developing a new driver adapter, and to surface very quickly some fundamental problems. (See the msg I just posted in Slack) I would say we keep them, at least for a while (or forever, as the first way to check if a driver adapter is complete). |
startTransactionwas not correctly wrapped when convertingAdapterto
ErrorCapturingAdapter. As a result,GenericErrormessage wasreturned in case of JS error. This PR fixes the problem.
Since it is hard to test those kinds of errors with real drivers, new
test suite for the error propogation is introduced. It uses fake
postgres adapter that throws on every call.