Skip to content

Commit

Permalink
fix(client): Correctly report error location in batch transaction (#1…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
SevInf authored and jkomyno committed Dec 21, 2022
1 parent 2ea9c95 commit c43af57
Show file tree
Hide file tree
Showing 20 changed files with 359 additions and 82 deletions.
34 changes: 27 additions & 7 deletions packages/client/src/runtime/RequestHandler.ts
@@ -1,6 +1,6 @@
import { Context } from '@opentelemetry/api'
import Debug from '@prisma/debug'
import { getTraceParent, TracingConfig } from '@prisma/engine-core'
import { getTraceParent, hasBatchIndex, TracingConfig } from '@prisma/engine-core'
import stripAnsi from 'strip-ansi'

import {
Expand Down Expand Up @@ -43,6 +43,7 @@ export type HandleErrorParams = {
error: any
clientMethod: string
callsite?: CallSite
transaction?: PrismaPromiseTransaction
}

export type Request = {
Expand Down Expand Up @@ -182,15 +183,22 @@ export class RequestHandler {
}
return unpackResult
} catch (error) {
this.handleRequestError({ error, clientMethod, callsite })
this.handleRequestError({ error, clientMethod, callsite, transaction })
}
}

handleRequestError({ error, clientMethod, callsite }: HandleErrorParams): never {
handleRequestError({ error, clientMethod, callsite, transaction }: HandleErrorParams): never {
debug(error)
// TODO: This is a workaround to keep backwards compatibility with clients
// consuming NotFoundError

if (isMismatchingBatchIndex(error, transaction)) {
// if this is batch error and current request was not it's cause, we don't add
// context information to the error: this wasn't a request that caused batch to fail
throw error
}

if (error instanceof NotFoundError) {
// TODO: This is a workaround to keep backwards compatibility with clients
// consuming NotFoundError
throw error
}

Expand All @@ -208,11 +216,19 @@ export class RequestHandler {
message = this.sanitizeMessage(message)
// TODO: Do request with callsite instead, so we don't need to rethrow
if (error.code) {
throw new PrismaClientKnownRequestError(message, error.code, this.client._clientVersion, error.meta)
throw new PrismaClientKnownRequestError(message, {
code: error.code,
clientVersion: this.client._clientVersion,
meta: error.meta,
batchRequestIdx: error.batchRequestIdx,
})
} else if (error.isPanic) {
throw new PrismaClientRustPanicError(message, this.client._clientVersion)
} else if (error instanceof PrismaClientUnknownRequestError) {
throw new PrismaClientUnknownRequestError(message, this.client._clientVersion)
throw new PrismaClientUnknownRequestError(message, {
clientVersion: this.client._clientVersion,
batchRequestIdx: error.batchRequestIdx,
})
} else if (error instanceof PrismaClientInitializationError) {
throw new PrismaClientInitializationError(message, this.client._clientVersion)
} else if (error instanceof PrismaClientRustPanicError) {
Expand Down Expand Up @@ -252,6 +268,10 @@ export class RequestHandler {
}
}

function isMismatchingBatchIndex(error: any, transaction: PrismaPromiseTransaction | undefined) {
return hasBatchIndex(error) && transaction?.kind === 'batch' && error.batchRequestIdx !== transaction.index
}

/**
* Determines which `findUnique` queries can be batched together so that the
* query engine can collapse/optimize the queries into a single one. This is
Expand Down
1 change: 1 addition & 0 deletions packages/client/src/runtime/core/request/PrismaPromise.ts
Expand Up @@ -4,6 +4,7 @@ export type PrismaPromiseBatchTransaction = {
kind: 'batch'
id: number
isolationLevel?: IsolationLevel
index: number
}

export type PrismaPromiseInteractiveTransaction = {
Expand Down
15 changes: 7 additions & 8 deletions packages/client/src/runtime/getPrismaClient.ts
Expand Up @@ -32,11 +32,7 @@ import { MetricsClient } from './core/metrics/MetricsClient'
import { applyModelsAndClientExtensions } from './core/model/applyModelsAndClientExtensions'
import { UserArgs } from './core/model/UserArgs'
import { createPrismaPromise } from './core/request/createPrismaPromise'
import type {
InteractiveTransactionOptions,
PrismaPromise,
PrismaPromiseTransaction,
} from './core/request/PrismaPromise'
import { InteractiveTransactionOptions, PrismaPromise, PrismaPromiseTransaction } from './core/request/PrismaPromise'
import { getLockCountPromise } from './core/transaction/utils/createLockCountPromise'
import { BaseDMMFHelper, DMMFHelper } from './dmmf'
import type { DMMF } from './dmmf-types'
Expand All @@ -56,6 +52,7 @@ import type { InstanceRejectOnNotFound, RejectOnNotFound } from './utils/rejectO
import { getRejectOnNotFound } from './utils/rejectOnNotFound'
import { serializeRawParameters } from './utils/serializeRawParameters'
import { validatePrismaClientOptions } from './utils/validatePrismaClientOptions'
import { waitForBatch } from './utils/waitForBatch'

const debug = Debug('prisma:client')
const ALTER_RE = /^(\s*alter\s)/i
Expand Down Expand Up @@ -947,17 +944,19 @@ new PrismaClient({
const txId = this._transactionId++
const lock = getLockCountPromise(promises.length)

const requests = promises.map((request) => {
const requests = promises.map((request, index) => {
if (request?.[Symbol.toStringTag] !== 'PrismaPromise') {
throw new Error(
`All elements of the array need to be Prisma Client promises. Hint: Please make sure you are not awaiting the Prisma client calls you intended to pass in the $transaction function.`,
)
}

return request.requestTransaction?.({ id: txId, isolationLevel: options?.isolationLevel }, lock)
return (
request.requestTransaction?.({ id: txId, index, isolationLevel: options?.isolationLevel }, lock) ?? request
)
})

return Promise.all(requests)
return waitForBatch(requests)
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/client/src/runtime/utils/rejectOnNotFound.ts
Expand Up @@ -17,7 +17,7 @@ export type InstanceRejectOnNotFound =
*/
export class NotFoundError extends PrismaClientKnownRequestError {
constructor(message: string) {
super(message, 'P2025', clientVersion)
super(message, { code: 'P2025', clientVersion })
this.name = 'NotFoundError'
}
}
Expand Down
74 changes: 74 additions & 0 deletions packages/client/src/runtime/utils/waitForBatch.test.ts
@@ -0,0 +1,74 @@
import { PrismaClientKnownRequestError } from '@prisma/engine-core'

import { waitForBatch } from './waitForBatch'

test('resolves when all promises succesfully resolve', async () => {
const result = await waitForBatch([Promise.resolve(1), Promise.resolve(2), Promise.resolve(3)])

expect(result).toEqual([1, 2, 3])
})

test('works with empty array', async () => {
const result = await waitForBatch([])

expect(result).toEqual([])
})

test('returns results in order even if they resolve out of order', async () => {
const result = await waitForBatch([
Promise.resolve(1),
new Promise((resolve) => setTimeout(() => resolve(2), 200)),
Promise.resolve(3),
])

expect(result).toEqual([1, 2, 3])
})

test('rejects with error, which batchRequestIdx matches its position in batch', async () => {
const err1 = new PrismaClientKnownRequestError('one', { code: 'P1', clientVersion: '0.0.0', batchRequestIdx: 2 })
const err2 = new PrismaClientKnownRequestError('two', { code: 'P1', clientVersion: '0.0.0', batchRequestIdx: 1 })
const err3 = new PrismaClientKnownRequestError('three', { code: 'P1', clientVersion: '0.0.0', batchRequestIdx: 0 })
const result = waitForBatch([Promise.reject(err1), Promise.reject(err2), Promise.reject(err3)])

await expect(result).rejects.toBe(err2)
})

test('rejects with error, which batchRequestIdx matches its position in batch, even if it rejects later', async () => {
const err1 = new PrismaClientKnownRequestError('one', { code: 'P1', clientVersion: '0.0.0', batchRequestIdx: 2 })
const err2 = new PrismaClientKnownRequestError('two', { code: 'P1', clientVersion: '0.0.0', batchRequestIdx: 1 })
const err3 = new PrismaClientKnownRequestError('three', { code: 'P1', clientVersion: '0.0.0', batchRequestIdx: 0 })
const result = waitForBatch([
Promise.reject(err1),
new Promise((_, reject) => setTimeout(() => reject(err2), 200)),
Promise.reject(err3),
])

await expect(result).rejects.toBe(err2)
})

test('rejects with the first error if no batchRequestIdxes match position', async () => {
const err1 = new PrismaClientKnownRequestError('one', { code: 'P1', clientVersion: '0.0.0', batchRequestIdx: 1 })
const err2 = new PrismaClientKnownRequestError('two', { code: 'P1', clientVersion: '0.0.0', batchRequestIdx: 2 })
const err3 = new PrismaClientKnownRequestError('three', { code: 'P1', clientVersion: '0.0.0', batchRequestIdx: 0 })
const result = waitForBatch([
Promise.reject(err1),
new Promise((_, reject) => setTimeout(() => reject(err2), 200)),
Promise.reject(err3),
])

await expect(result).rejects.toBe(err1)
})

test('rejects with error if one of the promises rejects with non-batch error', async () => {
const error = new Error('nope')
const result = waitForBatch([Promise.resolve(1), Promise.reject(error), Promise.resolve(3)])

await expect(result).rejects.toBe(error)
})

test('rejects with error if one of the promises rejects with non-batch error, even if other promsies never resolve', async () => {
const error = new Error('nope')
const result = waitForBatch([new Promise(() => {}), Promise.reject(error)])

await expect(result).rejects.toBe(error)
})
75 changes: 75 additions & 0 deletions packages/client/src/runtime/utils/waitForBatch.ts
@@ -0,0 +1,75 @@
import { hasBatchIndex } from '@prisma/engine-core'

/**
* Waits for result of batch $transaction and picks the best possible error to report if any
* of the request fails. Best error is determined as follows:
*
* - if engine have reported an error without batch request index, then the batch is immediately rejected
* with this error without waiting for other promises
* - if engine have reported and index of failed request in the batch and that index matches the position of the
* particular request in the batch, batch is rejected with that error
* - if batch request index is reported and it does not match current request position, wait for other requests. If no indices
* match request positions, reject with the earliest error in the batch
*
* @param promises
* @returns
*/
export function waitForBatch<T extends PromiseLike<unknown>[]>(
promises: T,
): Promise<{ [K in keyof T]: Awaited<T[K]> }> {
if (promises.length === 0) {
return Promise.resolve([] as { [K in keyof T]: Awaited<T[K]> })
}
return new Promise((resolve, reject) => {
const successfulResults = new Array(promises.length) as { [K in keyof T]: Awaited<T[K]> }
let bestError: unknown = null
let done = false
let settledPromisesCount = 0

const settleOnePromise = () => {
if (done) {
return
}
settledPromisesCount++
if (settledPromisesCount === promises.length) {
done = true
if (bestError) {
reject(bestError)
} else {
resolve(successfulResults)
}
}
}

const immediatelyReject = (error: unknown) => {
if (!done) {
done = true
reject(error)
}
}

for (let i = 0; i < promises.length; i++) {
promises[i].then(
(result) => {
successfulResults[i] = result
settleOnePromise()
},
(error) => {
if (!hasBatchIndex(error)) {
immediatelyReject(error)
return
}

if (error.batchRequestIdx === i) {
immediatelyReject(error)
} else {
if (!bestError) {
bestError = error
}
settleOnePromise()
}
},
)
}
})
}
Expand Up @@ -13,10 +13,10 @@ exports[`interactive-transactions (provider=cockroachdb) batching rollback 1`] =
Invalid \`prisma.user.create()\` invocation in
/client/tests/functional/interactive-transactions/tests.ts:0:0
XX 'batching rollback',
XX async () => {
XX const result = prisma.$transaction([
XX prisma.user.create(
XX email: 'user_1@website.com',
XX },
XX }),
→ XX prisma.user.create(
Unique constraint failed on the fields: (\`email\`)
`;

Expand All @@ -37,10 +37,10 @@ exports[`interactive-transactions (provider=mongodb) batching rollback 1`] = `
Invalid \`prisma.user.create()\` invocation in
/client/tests/functional/interactive-transactions/tests.ts:0:0
XX 'batching rollback',
XX async () => {
XX const result = prisma.$transaction([
XX prisma.user.create(
XX email: 'user_1@website.com',
XX },
XX }),
→ XX prisma.user.create(
Unique constraint failed on the constraint: \`User_email_key\`
`;

Expand Down Expand Up @@ -69,10 +69,10 @@ exports[`interactive-transactions (provider=mysql) batching rollback 1`] = `
Invalid \`prisma.user.create()\` invocation in
/client/tests/functional/interactive-transactions/tests.ts:0:0
XX 'batching rollback',
XX async () => {
XX const result = prisma.$transaction([
XX prisma.user.create(
XX email: 'user_1@website.com',
XX },
XX }),
→ XX prisma.user.create(
Unique constraint failed on the constraint: \`User_email_key\`
`;

Expand Down Expand Up @@ -101,10 +101,10 @@ exports[`interactive-transactions (provider=postgresql) batching rollback 1`] =
Invalid \`prisma.user.create()\` invocation in
/client/tests/functional/interactive-transactions/tests.ts:0:0
XX 'batching rollback',
XX async () => {
XX const result = prisma.$transaction([
XX prisma.user.create(
XX email: 'user_1@website.com',
XX },
XX }),
→ XX prisma.user.create(
Unique constraint failed on the fields: (\`email\`)
`;

Expand Down Expand Up @@ -133,10 +133,10 @@ exports[`interactive-transactions (provider=sqlite) batching rollback 1`] = `
Invalid \`prisma.user.create()\` invocation in
/client/tests/functional/interactive-transactions/tests.ts:0:0
XX 'batching rollback',
XX async () => {
XX const result = prisma.$transaction([
XX prisma.user.create(
XX email: 'user_1@website.com',
XX },
XX }),
→ XX prisma.user.create(
Unique constraint failed on the fields: (\`email\`)
`;

Expand Down Expand Up @@ -165,10 +165,10 @@ exports[`interactive-transactions (provider=sqlserver) batching rollback 1`] = `
Invalid \`prisma.user.create()\` invocation in
/client/tests/functional/interactive-transactions/tests.ts:0:0
XX 'batching rollback',
XX async () => {
XX const result = prisma.$transaction([
XX prisma.user.create(
XX email: 'user_1@website.com',
XX },
XX }),
→ XX prisma.user.create(
Unique constraint failed on the constraint: \`dbo.User\`
`;

Expand Down

0 comments on commit c43af57

Please sign in to comment.