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): Correctly report error location in batch transaction #16240

Merged
merged 1 commit into from Nov 17, 2022

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Nov 11, 2022

Engine PR: prisma/prisma-engines#3384

Uses newly added batch_request_idx property of an errors to identify
and correctly report error location within a batch.

Fix #15433
Fix #14373

@SevInf SevInf force-pushed the fix/tx-error-location branch 2 times, most recently from af2774f to 6a2d4d2 Compare November 11, 2022 15:18
packages/client/package.json Outdated Show resolved Hide resolved
@SevInf SevInf marked this pull request as ready for review November 11, 2022 16:26
@SevInf SevInf requested a review from a team November 11, 2022 16:26
@SevInf SevInf requested review from danstarns and removed request for a team November 11, 2022 16:26
@SevInf SevInf added this to the 4.7.0 milestone Nov 11, 2022
@SevInf SevInf requested review from aqrln and millsp November 14, 2022 16:22
@SevInf
Copy link
Contributor Author

SevInf commented Nov 14, 2022

@millsp @aqrln I'd ask for another re-review here. I had to rewrite waitForBatch function to be closer to how Promise.all behaves and reject early on all non-batch errors. This is necessary, because if one of the batch requests fails early, with, for example, validation error, batch will not be executed at all and remaining promises won't settle. Promise.all rejected batch promise correctly in this case, older waitForBatch, based on Promise.allSetlled did not. Now it fixed, with unfortunate downside of a more complicated code.

Copy link
Member

@aqrln aqrln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work!

@SevInf SevInf force-pushed the fix/tx-error-location branch 2 times, most recently from a42cb3c to 85c30e7 Compare November 15, 2022 17:23
@SevInf SevInf force-pushed the fix/tx-error-location branch 2 times, most recently from b21c526 to 78a760e Compare November 16, 2022 12:19
Engine PR: prisma/prisma-engines#3384

Uses newly added `batch_request_idx` property of an errors to identify
and correctly report error location within a batch.

Fix #15433
Fix #14373
@SevInf SevInf merged commit ff7fb92 into main Nov 17, 2022
@SevInf SevInf deleted the fix/tx-error-location branch November 17, 2022 10:37
jkomyno pushed a commit that referenced this pull request Dec 21, 2022
…6240)

Engine PR: prisma/prisma-engines#3384

Uses newly added `batch_request_idx` property of an errors to identify
and correctly report error location within a batch.

Fix #15433
Fix #14373
jkomyno pushed a commit that referenced this pull request Dec 21, 2022
…6240)

Engine PR: prisma/prisma-engines#3384

Uses newly added `batch_request_idx` property of an errors to identify
and correctly report error location within a batch.

Fix #15433
Fix #14373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants