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

…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>

packages/client/tests/functional/find-unique-or-throw-batching/_matrix.ts

@@ -0,0 +1,4 @@
+import { defineMatrix } from '../_utils/defineMatrix'
+import { allProviders } from '../_utils/providers'
+
+export default defineMatrix(() => [allProviders])

packages/client/tests/functional/find-unique-or-throw-batching/prisma/_schema.ts

@@ -0,0 +1,19 @@
+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)}
+  }
+  `
+})

packages/client/tests/functional/find-unique-or-throw-batching/tests.ts

@@ -0,0 +1,46 @@
+import { faker } from '@faker-js/faker'
+
+import testMatrix from './_matrix'
+// @ts-ignore
+import type { PrismaClient } from './node_modules/@prisma/client'
+
+declare let prisma: PrismaClient
+const missing = faker.database.mongodbObjectId()
+
+testMatrix.setupTestSuite((suiteConfig, suiteMeta, clientMeta) => {
+  let id1: string
+  let id2: string
+  beforeAll(async () => {
+    id1 = await prisma.user
+      .create({
+        data: {},
+      })
+      .then((user) => user.id)
+
+    id2 = await prisma.user
+      .create({
+        data: {},
+      })
+      .then((user) => user.id)
+  })
+
+  test('batched errors are when all objects in batch are found', async () => {
+    const found = prisma.user.findUniqueOrThrow({ where: { id: id1 } })
+    const foundToo = prisma.user.findUniqueOrThrow({ where: { id: id2 } })
+    const result = await Promise.allSettled([found, foundToo])
+    expect(result).toEqual([
+      { status: 'fulfilled', value: { id: id1 } },
+      { status: 'fulfilled', value: { id: id2 } },
+    ])
+  })
+
+  test('batched errors when some of the objects not found', async () => {
+    const found = prisma.user.findUniqueOrThrow({ where: { id: id1 } })
+    const notFound = prisma.user.findUniqueOrThrow({ where: { id: missing } })
+    const newResult = await Promise.allSettled([found, notFound])
+    expect(newResult).toEqual([
+      { status: 'fulfilled', value: { id: id1 } },
+      { status: 'rejected', reason: expect.objectContaining({ code: 'P2025' }) },
+    ])
+  })
+}, {})

packages/engine-core/src/binary/BinaryEngine.ts

@@ -16,6 +16,7 @@ import { URL } from 'url'
 import { promisify } from 'util'
 
 import type {
+  BatchQueryEngineResult,
   DatasourceOverwrite,
   EngineConfig,
   EngineEventType,
@@ -40,6 +41,7 @@ import type {
   QueryEngineBatchRequest,
   QueryEngineRequestHeaders,
   QueryEngineResult,
+  QueryEngineResultBatchQueryResult,
 } from '../common/types/QueryEngine'
 import type * as Tx from '../common/types/Transaction'
 import { printGeneratorConfig } from '../common/utils/printGeneratorConfig'
@@ -373,15 +375,15 @@ This probably happens, because you built Prisma Client on a different platform.
 Searched Locations:
 
 ${searchedLocations
-          .map((f) => {
-            let msg = `  ${f}`
-            if (process.env.DEBUG === 'node-engine-search-locations' && fs.existsSync(f)) {
-              const dir = fs.readdirSync(f)
-              msg += dir.map((d) => `    ${d}`).join('\n')
-            }
-            return msg
-          })
-          .join('\n' + (process.env.DEBUG === 'node-engine-search-locations' ? '\n' : ''))}\n`
+  .map((f) => {
+    let msg = `  ${f}`
+    if (process.env.DEBUG === 'node-engine-search-locations' && fs.existsSync(f)) {
+      const dir = fs.readdirSync(f)
+      msg += dir.map((d) => `    ${d}`).join('\n')
+    }
+    return msg
+  })
+  .join('\n' + (process.env.DEBUG === 'node-engine-search-locations' ? '\n' : ''))}\n`
       // The generator should always be there during normal usage
       if (this.generator) {
         // The user already added it, but it still doesn't work 🤷‍♀️
@@ -392,8 +394,8 @@ ${searchedLocations
         ) {
           errorText += `
 You already added the platform${this.generator.binaryTargets.length > 1 ? 's' : ''} ${this.generator.binaryTargets
-              .map((t) => `"${chalk.bold(t.value)}"`)
-              .join(', ')} to the "${chalk.underline('generator')}" block
+            .map((t) => `"${chalk.bold(t.value)}"`)
+            .join(', ')} to the "${chalk.underline('generator')}" block
 in the "schema.prisma" file as described in https://pris.ly/d/client-generator,
 but something went wrong. That's suboptimal.
 
@@ -951,7 +953,7 @@ You very likely have the wrong "binaryTarget" defined in the schema.prisma file.
     transaction,
     numTry = 1,
     containsWrite,
-  }: RequestBatchOptions): Promise<QueryEngineResult<T>[]> {
+  }: RequestBatchOptions): Promise<BatchQueryEngineResult<T>[]> {
     await this.start()
 
     const request: QueryEngineBatchRequest = {
@@ -970,8 +972,8 @@ You very likely have the wrong "binaryTarget" defined in the schema.prisma file.
         const { batchResult, errors } = data
         if (Array.isArray(batchResult)) {
           return batchResult.map((result) => {
-            if (result.errors) {
-              throw prismaGraphQLToJSError(data.errors[0], this.clientVersion!)
+            if (result.errors && result.errors.length > 0) {
+              return prismaGraphQLToJSError(result.errors[0], this.clientVersion!)
             }
             return {
               data: result,

packages/engine-core/src/common/Engine.ts

@@ -7,7 +7,7 @@ import type { QueryEngineRequestHeaders, QueryEngineResult } from './types/Query
 import type * as Transaction from './types/Transaction'
 
 export interface FilterConstructor {
-  new(config: EngineConfig): Engine
+  new (config: EngineConfig): Engine
 }
 
 export type NullableEnvValue = {
@@ -41,6 +41,8 @@ export type RequestBatchOptions = {
   containsWrite: boolean
 }
 
+export type BatchQueryEngineResult<T> = QueryEngineResult<T> | Error
+
 // TODO Move shared logic in here
 export abstract class Engine {
   abstract on(event: EngineEventType, listener: (args?: any) => any): void
@@ -50,7 +52,7 @@ export abstract class Engine {
   abstract getDmmf(): Promise<DMMF.Document>
   abstract version(forceRun?: boolean): Promise<string> | string
   abstract request<T>(options: RequestOptions<unknown>): Promise<QueryEngineResult<T>>
-  abstract requestBatch<T>(options: RequestBatchOptions): Promise<QueryEngineResult<T>[]>
+  abstract requestBatch<T>(options: RequestBatchOptions): Promise<BatchQueryEngineResult<T>[]>
   abstract transaction(
     action: 'start',
     headers: Transaction.TransactionHeaders,

packages/engine-core/src/common/types/QueryEngine.ts

@@ -1,5 +1,6 @@
 import type { DataSource, GeneratorConfig } from '@prisma/generator-helper'
 
+import { RequestError } from '../errors/types/RequestError'
 import * as Transaction from './Transaction'
 
 // Events
@@ -79,6 +80,15 @@ export type QueryEngineResult<T> = {
   elapsed: number
 }
 
+export type QueryEngineResultBatchQueryResult<T> =
+  | {
+      data: T
+      elapsed: number
+    }
+  | {
+      errors: RequestError[]
+    }
+
 export type QueryEngineRequestHeaders = {
   traceparent?: string
   transactionId?: string

packages/engine-core/src/data-proxy/DataProxyEngine.ts

@@ -2,20 +2,26 @@ import Debug from '@prisma/debug'
 import { DMMF } from '@prisma/generator-helper'
 
 import type {
+  BatchQueryEngineResult,
   EngineConfig,
   EngineEventType,
   GetConfigResult,
   InlineDatasource,
-  RequestOptions,
+  InteractiveTransactionOptions,
   RequestBatchOptions,
-  InteractiveTransactionOptions
+  RequestOptions,
 } from '../common/Engine'
 import { Engine } from '../common/Engine'
 import { PrismaClientUnknownRequestError } from '../common/errors/PrismaClientUnknownRequestError'
 import { prismaGraphQLToJSError } from '../common/errors/utils/prismaGraphQLToJSError'
 import { EventEmitter } from '../common/types/Events'
 import { EngineMetricsOptions, Metrics, MetricsOptionsJson, MetricsOptionsPrometheus } from '../common/types/Metrics'
-import { QueryEngineBatchRequest, QueryEngineRequestHeaders, QueryEngineResult } from '../common/types/QueryEngine'
+import {
+  QueryEngineBatchRequest,
+  QueryEngineRequestHeaders,
+  QueryEngineResult,
+  QueryEngineResultBatchQueryResult,
+} from '../common/types/QueryEngine'
 import type * as Tx from '../common/types/Transaction'
 import { DataProxyError } from './errors/DataProxyError'
 import { ForcedRetryError } from './errors/ForcedRetryError'
@@ -77,8 +83,8 @@ export class DataProxyEngine extends Engine {
     return 'unknown'
   }
 
-  async start() { }
-  async stop() { }
+  async start() {}
+  async stop() {}
 
   on(event: EngineEventType, listener: (args?: any) => any): void {
     if (event === 'beforeExit') {
@@ -145,7 +151,11 @@ export class DataProxyEngine extends Engine {
     return this.requestInternal<T>({ query, variables: {} }, headers, transaction)
   }
 
-  async requestBatch<T>({ queries, headers = {}, transaction }: RequestBatchOptions): Promise<QueryEngineResult<T>[]> {
+  async requestBatch<T>({
+    queries,
+    headers = {},
+    transaction,
+  }: RequestBatchOptions): Promise<BatchQueryEngineResult<T>[]> {
     const isTransaction = Boolean(transaction)
     this.logEmitter.emit('query', {
       query: `Batch${isTransaction ? ' in transaction' : ''} (${queries.length}):\n${queries.join('\n')}`,
@@ -157,18 +167,26 @@ export class DataProxyEngine extends Engine {
       isolationLevel: transaction?.isolationLevel,
     }
 
-    const { batchResult } = await this.requestInternal<T, true>(body, headers)
+    const { batchResult, elapsed } = await this.requestInternal<T, true>(body, headers)
 
-    // TODO: add elapsed to each result similar to BinaryEngine
-    // also check that the error handling is correct for batch
-    return batchResult
+    return batchResult.map((result) => {
+      if ('errors' in result && result.errors.length > 0) {
+        return prismaGraphQLToJSError(result.errors[0], this.clientVersion!)
+      }
+      return {
+        data: result as T,
+        elapsed,
+      }
+    })
   }
 
   private requestInternal<T, Batch extends boolean = false>(
     body: Record<string, any>,
     headers: QueryEngineRequestHeaders,
     itx?: InteractiveTransactionOptions<DataProxyTxInfoPayload>,
-  ): Promise<Batch extends true ? { batchResult: QueryEngineResult<T>[] } : QueryEngineResult<T>> {
+  ): Promise<
+    Batch extends true ? { batchResult: QueryEngineResultBatchQueryResult<T>[]; elapsed: number } : QueryEngineResult<T>
+  > {
     return this.withRetry({
       actionGerund: 'querying',
       callback: async ({ logHttpCall }) => {

packages/engine-core/src/library/LibraryEngine.ts

@@ -6,6 +6,7 @@ import chalk from 'chalk'
 import fs from 'fs'
 
 import type {
+  BatchQueryEngineResult,
   DatasourceOverwrite,
   EngineConfig,
   EngineEventType,
@@ -489,7 +490,11 @@ You may have to run ${chalk.greenBright('prisma generate')} for your changes to
     }
   }
 
-  async requestBatch<T>({ queries, headers = {}, transaction }: RequestBatchOptions): Promise<QueryEngineResult<T>[]> {
+  async requestBatch<T>({
+    queries,
+    headers = {},
+    transaction,
+  }: RequestBatchOptions): Promise<BatchQueryEngineResult<T>[]> {
     debug('requestBatch')
     const request: QueryEngineBatchRequest = {
       batch: queries.map((query) => ({ query, variables: {} })),
@@ -516,8 +521,8 @@ You may have to run ${chalk.greenBright('prisma generate')} for your changes to
     const { batchResult, errors } = data
     if (Array.isArray(batchResult)) {
       return batchResult.map((result) => {
-        if (result.errors) {
-          return this.loggerRustPanic ?? this.buildQueryError(data.errors[0])
+        if (result.errors && result.errors.length > 0) {
+          return this.loggerRustPanic ?? this.buildQueryError(result.errors[0])
         }
         return {
           data: result,