Skip to content

Commit

Permalink
fix(client): Correctly report error location in batch transaction
Browse files Browse the repository at this point in the history
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 committed Nov 14, 2022
1 parent 8e73964 commit 7b0734a
Show file tree
Hide file tree
Showing 19 changed files with 243 additions and 77 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
7 changes: 4 additions & 3 deletions packages/client/src/runtime/getPrismaClient.ts
Expand Up @@ -56,6 +56,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 +948,17 @@ 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)
})

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
36 changes: 36 additions & 0 deletions packages/client/src/runtime/utils/waitForBatch.ts
@@ -0,0 +1,36 @@
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 and index of failed request in the batch, the best error is the one
* who's index matches that
* - otherwise, first error is used (like Promise.all)
*
* @param promises
* @returns
*/
export async function waitForBatch<T extends unknown[]>(promises: T): Promise<{ [K in keyof T]: Awaited<T[K]> }> {
const results = await Promise.allSettled(promises)

let bestError: unknown = null
const successfulResults = [] as { [K in keyof T]: Awaited<T[K]> }
for (const [i, result] of results.entries()) {
if (result.status === 'rejected') {
const reason = result.reason
if (hasBatchIndex(reason) && reason.batchRequestIdx === i) {
bestError = reason
} else if (!bestError) {
bestError = reason
}
} else {
successfulResults.push(result.value)
}
}

if (bestError) {
throw bestError
}
return successfulResults
}
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,0 +1,24 @@
import { defineMatrix } from '../../_utils/defineMatrix'

export default defineMatrix(() => [
[
{
provider: 'sqlite',
},
{
provider: 'postgresql',
},
{
provider: 'mysql',
},
{
provider: 'mongodb',
},
{
provider: 'cockroachdb',
},
{
provider: 'sqlserver',
},
],
])
@@ -0,0 +1,22 @@
import { idForProvider } from '../../../_utils/idForProvider'
import testMatrix from '../_matrix'

export default testMatrix.setupSchema(({ provider }) => {
return /* Prisma */ `
generator client {
provider = "prisma-client-js"
}
datasource db {
provider = "${provider}"
url = env("DATABASE_URI_${provider}")
}
model User {
id ${idForProvider(provider)}
email String @unique
memo String?
createdAt DateTime @default(now())
}
`
})
@@ -0,0 +1,24 @@
// @ts-ignore
import type { PrismaClient } from '@prisma/client'

import testMatrix from './_matrix'

declare let prisma: PrismaClient

testMatrix.setupTestSuite(() => {
test('correctly reports location of a batch error', async () => {
const result = prisma.$transaction([
prisma.user.findMany({}),
prisma.user.update({
where: {
email: 'notthere@example.com',
},
data: {
memo: 'id is 1',
},
}),
])

await expect(result).rejects.toThrowError('Invalid `prisma.user.update()` invocation')
})
})
29 changes: 13 additions & 16 deletions packages/engine-core/src/binary/BinaryEngine.ts
Expand Up @@ -493,10 +493,9 @@ ${chalk.dim("In case we're mistaken, please report this to us 🙏.")}`)
await this.startPromise

if (!this.child && !this.engineEndpoint) {
throw new PrismaClientUnknownRequestError(
`Can't perform request, as the Engine has already been stopped`,
this.clientVersion!,
)
throw new PrismaClientUnknownRequestError(`Can't perform request, as the Engine has already been stopped`, {
clientVersion: this.clientVersion!,
})
}
}

Expand Down Expand Up @@ -951,7 +950,7 @@ You very likely have the wrong "binaryTarget" defined in the schema.prisma file.
throw prismaGraphQLToJSError(data.errors[0], this.clientVersion!)
}
// this case should not happen, as the query engine only returns one error
throw new PrismaClientUnknownRequestError(JSON.stringify(data.errors), this.clientVersion!)
throw new PrismaClientUnknownRequestError(JSON.stringify(data.errors), { clientVersion: this.clientVersion! })
}

// Rust engine returns time in microseconds and we want it in milliseconds
Expand Down Expand Up @@ -1095,10 +1094,9 @@ You very likely have the wrong "binaryTarget" defined in the schema.prisma file.
}

if (this.lastErrorLog && isRustErrorLog(this.lastErrorLog)) {
const err = new PrismaClientUnknownRequestError(
this.getErrorMessageWithLink(getMessage(this.lastErrorLog)),
this.clientVersion!,
)
const err = new PrismaClientUnknownRequestError(this.getErrorMessageWithLink(getMessage(this.lastErrorLog)), {
clientVersion: this.clientVersion!,
})

if (this.lastErrorLog?.fields?.message === 'PANIC') {
this.lastPanic = err
Expand Down Expand Up @@ -1157,7 +1155,7 @@ and your request can't be processed.
You probably have some open handle that prevents your process from exiting.
It could be an open http server or stream that didn't close yet.
We recommend using the \`wtfnode\` package to debug open handles.`,
this.clientVersion!,
{ clientVersion: this.clientVersion! },
)
}

Expand Down Expand Up @@ -1203,12 +1201,11 @@ Please look into the logs or turn on the env var DEBUG=* to debug the constantly
*/
transactionHttpErrorHandler<R>(result: Result<R>): never {
const response = result.data as { [K: string]: unknown }
throw new PrismaClientKnownRequestError(
response.message as string,
response.error_code as string,
this.clientVersion as string,
response.meta,
)
throw new PrismaClientKnownRequestError(response.message as string, {
code: response.error_code as string,
clientVersion: this.clientVersion as string,
meta: response.meta as Record<string, unknown>,
})
}
}

Expand Down
7 changes: 7 additions & 0 deletions packages/engine-core/src/common/errors/ErrorWithBatchIndex.ts
@@ -0,0 +1,7 @@
export interface ErrorWithBatchIndex {
batchRequestIdx?: number
}

export function hasBatchIndex(value: object): value is Required<ErrorWithBatchIndex> {
return typeof value['batchRequestIdx'] === 'number'
}

0 comments on commit 7b0734a

Please sign in to comment.