Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: client handling of batched queries when some fail, and some not #16574

Merged
merged 9 commits into from
Dec 2, 2022

Conversation

miguelff
Copy link
Contributor

@miguelff miguelff commented Dec 1, 2022

Addresses prisma/prisma-engines#3458 (comment)

Depends on merging and updating engine hashes for prisma/prisma-engines#3458 as I need to write some unit tests to prevent regressions.

Before the release of native findXOrThrow, there were no instances of batching where commonly some promises failed while others succeeded. And the code handling that scenario was broken. This PR attempts to fix it.

@miguelff miguelff changed the title Fix some error management in engine code fix: client handling of batched queries when some fail, and some not Dec 1, 2022
@miguelff miguelff added this to the 4.8.0 milestone Dec 1, 2022
@miguelff miguelff added the kind/bug A reported bug. label Dec 1, 2022
@miguelff miguelff force-pushed the fix/batched_queries_with_errors branch from 251a02a to 4ef83c8 Compare December 1, 2022 13:35
@miguelff miguelff marked this pull request as ready for review December 1, 2022 15:13
@miguelff miguelff requested a review from a team December 1, 2022 15:13
@miguelff miguelff requested review from SevInf and removed request for a team December 1, 2022 15:13
@miguelff
Copy link
Contributor Author

miguelff commented Dec 1, 2022

The regression affected binary and library engines. For data proxy, there's a different problem that manifests when executing the same test (The test exercises two different problems)

The problem in particular (after debugging the data proxy) is that the engine adapter is not able to discern errors coming from individual queries in a batch, and as a consequence, doesn't reject a promise that errored.

If we do this:

    const found = prisma.user.findUniqueOrThrow({ where: { id } })
    const notFound = prisma.user.findUniqueOrThrow({ where: { id: missing } })
    const result = await Promise.allSettled([found, notFound])

We get:

    [
      { status: 'fulfilled', value: { id: id } },
      { reason: new NotFoundError('No User found'), status: 'rejected' },
    ]

while with the data proxy, we get.

    [
      { status: 'fulfilled', value: { id: 'xqwhv5rgkt2' } },
      { status: 'fulfilled', value: undefined }
    ]

I leave to @aqrln the investigation, ticket creation, and (if doable directly) addressing of the problem described here. To reproduce it, it is enough to unskip the test provided in this patch.

@SevInf SevInf merged commit a02b80e into main Dec 2, 2022
@SevInf SevInf deleted the fix/batched_queries_with_errors branch December 2, 2022 08:13
SevInf added a commit that referenced this pull request Dec 2, 2022
…16574)

* Fix some error management in engine code

* Bump versions

* Added regression test

* Allow for data proxy tests to run

* Run in every provider

* Temporary skip data proxy

* Make tests resilient to mongo id format

* Try to unskip the test

* Reproduce & fix batch query issue for data proxy

Co-authored-by: Sergey Tatarintsev <tatarintsev@prisma.io>
jkomyno pushed a commit that referenced this pull request Dec 21, 2022
…16574)

* Fix some error management in engine code

* Bump versions

* Added regression test

* Allow for data proxy tests to run

* Run in every provider

* Temporary skip data proxy

* Make tests resilient to mongo id format

* Try to unskip the test

* Reproduce & fix batch query issue for data proxy

Co-authored-by: Sergey Tatarintsev <tatarintsev@prisma.io>
jkomyno pushed a commit that referenced this pull request Dec 21, 2022
…16574)

* Fix some error management in engine code

* Bump versions

* Added regression test

* Allow for data proxy tests to run

* Run in every provider

* Temporary skip data proxy

* Make tests resilient to mongo id format

* Try to unskip the test

* Reproduce & fix batch query issue for data proxy

Co-authored-by: Sergey Tatarintsev <tatarintsev@prisma.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants