fix(client): remove retries for all errors except network connection/…

…socket errors (#15684) Only retry network-related errors in BinaryEngine, and only if those didn't occur during a write operation. Fixes: #12862 Co-authored-by: Alexey Orlenko <alex@aqrln.net>
prisma · Nov 23, 2022 · e50510f · e50510f
1 parent 2a6c927
commit e50510f
Show file tree

Hide file tree

Showing 14 changed files with 339 additions and 120 deletions.
diff --git a/packages/client/src/__tests__/batching.test.ts b/packages/client/src/__tests__/batching.test.ts
@@ -1,5 +1,3 @@
-import { TracingConfig } from '@prisma/engine-core'
-
 import { blog } from '../fixtures/blog'
 import { getDMMF } from '../generation/getDMMF'
 import { DMMFClass, makeDocument } from '../runtime'
@@ -17,7 +15,7 @@ describe('batching', () => {
         // @ts-expect-error
         requestBatch: (batch) => {
           batches.push(batch)
-          return Promise.resolve(batch.map(() => ({ data: { data: null }, elapsed: 0.2 })))
+          return Promise.resolve(batch.queries.map(() => ({ data: { data: null }, elapsed: 0.2 })))
         },
         // @ts-expect-error
         request: (request) => {
@@ -76,8 +74,13 @@ describe('batching', () => {
 
     expect(batches).toMatchInlineSnapshot(`
       Array [
-        Array [
-          query {
+        Object {
+          containsWrite: false,
+          headers: Object {
+            traceparent: 00-10-10-00,
+          },
+          queries: Array [
+            query {
         findUniqueUser(where: {
           id: "1"
         }) {
@@ -96,7 +99,7 @@ describe('batching', () => {
           coinflips
         }
       },
-          query {
+            query {
         findUniqueUser(where: {
           id: "2"
         }) {
@@ -115,7 +118,9 @@ describe('batching', () => {
           coinflips
         }
       },
-        ],
+          ],
+          transaction: undefined,
+        },
       ]
     `)
     expect(requests).toMatchInlineSnapshot(`Array []`)
@@ -132,7 +137,7 @@ describe('batching', () => {
         // @ts-expect-error
         requestBatch: (batch) => {
           batches.push(batch)
-          return Promise.resolve(batch.map(() => ({ data: { data: null }, elapsed: 0.2 })))
+          return Promise.resolve(batch.queries.map(() => ({ data: { data: null }, elapsed: 0.2 })))
         },
         // @ts-expect-error
         request: (request) => {
@@ -188,7 +193,12 @@ describe('batching', () => {
     expect(batches).toMatchInlineSnapshot(`Array []`)
     expect(requests).toMatchInlineSnapshot(`
       Array [
-        query {
+        Object {
+          headers: Object {
+            traceparent: 00-10-10-00,
+          },
+          isWrite: false,
+          query: query {
         findUniquePost(where: {
           id: "1"
         }) {
@@ -202,7 +212,14 @@ describe('batching', () => {
           optional
         }
       },
-        query {
+          transaction: undefined,
+        },
+        Object {
+          headers: Object {
+            traceparent: 00-10-10-00,
+          },
+          isWrite: false,
+          query: query {
         findUniqueUser(where: {
           id: "2"
         }) {
@@ -221,6 +238,8 @@ describe('batching', () => {
           coinflips
         }
       },
+          transaction: undefined,
+        },
       ]
     `)
   })
@@ -236,7 +255,7 @@ describe('batching', () => {
         // @ts-expect-error
         requestBatch: (batch) => {
           batches.push(batch)
-          return Promise.resolve(batch.map(() => ({ data: { data: null }, elapsed: 0.2 })))
+          return Promise.resolve(batch.queries.map(() => ({ data: { data: null }, elapsed: 0.2 })))
         },
         // @ts-expect-error
         request: (request) => {
@@ -288,7 +307,12 @@ describe('batching', () => {
     expect(batches).toMatchInlineSnapshot(`Array []`)
     expect(requests).toMatchInlineSnapshot(`
       Array [
-        query {
+        Object {
+          headers: Object {
+            traceparent: 00-10-10-00,
+          },
+          isWrite: false,
+          query: query {
         findUniqueUser(where: {
           email: "a@a.de"
         }) {
@@ -307,7 +331,14 @@ describe('batching', () => {
           coinflips
         }
       },
-        query {
+          transaction: undefined,
+        },
+        Object {
+          headers: Object {
+            traceparent: 00-10-10-00,
+          },
+          isWrite: false,
+          query: query {
         findUniqueUser(where: {
           id: "2"
         }) {
@@ -326,6 +357,8 @@ describe('batching', () => {
           coinflips
         }
       },
+          transaction: undefined,
+        },
       ]
     `)
   })

diff --git a/packages/client/src/runtime/RequestHandler.ts b/packages/client/src/runtime/RequestHandler.ts
@@ -92,16 +92,28 @@ export class RequestHandler {
         // TODO: pass the child information to QE for it to issue links to queries
         // const links = requests.map((r) => trace.getSpanContext(r.otelChildCtx!))
 
+        const containsWrite = requests.some((r) => r.document.type === 'mutation')
+
         const batchTransaction = info.transaction?.kind === 'batch' ? info.transaction : undefined
 
-        return this.client._engine.requestBatch(queries, info.headers, batchTransaction)
+        return this.client._engine.requestBatch({
+          queries,
+          headers: info.headers,
+          transaction: batchTransaction,
+          containsWrite,
+        })
       },
       singleLoader: (request) => {
         const info = getRequestInfo(request)
         const query = String(request.document)
         const interactiveTransaction = info.transaction?.kind === 'itx' ? info.transaction : undefined
 
-        return this.client._engine.request(query, info.headers, interactiveTransaction)
+        return this.client._engine.request({
+          query,
+          headers: info.headers,
+          transaction: interactiveTransaction,
+          isWrite: request.document.type === 'mutation',
+        })
       },
       batchBy: (request) => {
         if (request.transaction?.id) {

diff --git a/packages/client/tests/functional/_utils/setupTestSuiteClient.ts b/packages/client/tests/functional/_utils/setupTestSuiteClient.ts
@@ -12,7 +12,7 @@ import {
 } from './getTestSuiteInfo'
 import { DatasourceInfo, setupTestSuiteDatabase, setupTestSuiteFiles, setupTestSuiteSchema } from './setupTestSuiteEnv'
 import type { TestSuiteMeta } from './setupTestSuiteMatrix'
-import { ClientMeta, ClientRuntime } from './types'
+import { AlterStatementCallback, ClientMeta, ClientRuntime } from './types'
 
 /**
  * Does the necessary setup to get a test suite client ready to run.
@@ -26,12 +26,14 @@ export async function setupTestSuiteClient({
   skipDb,
   datasourceInfo,
   clientMeta,
+  alterStatementCallback,
 }: {
   suiteMeta: TestSuiteMeta
   suiteConfig: NamedTestSuiteConfig
   skipDb?: boolean
   datasourceInfo: DatasourceInfo
   clientMeta: ClientMeta
+  alterStatementCallback?: AlterStatementCallback
 }) {
   const suiteFolderPath = getTestSuiteFolderPath(suiteMeta, suiteConfig)
   const previewFeatures = getTestSuitePreviewFeatures(suiteConfig.matrixOptions)
@@ -44,7 +46,7 @@ export async function setupTestSuiteClient({
   await setupTestSuiteSchema(suiteMeta, suiteConfig, schema)
   if (!skipDb) {
     process.env[datasourceInfo.envVarName] = datasourceInfo.databaseUrl
-    await setupTestSuiteDatabase(suiteMeta, suiteConfig)
+    await setupTestSuiteDatabase(suiteMeta, suiteConfig, [], alterStatementCallback)
   }
 
   process.env[datasourceInfo.envVarName] = datasourceInfo.dataProxyUrl ?? datasourceInfo.databaseUrl

diff --git a/packages/client/tests/functional/_utils/setupTestSuiteEnv.ts b/packages/client/tests/functional/_utils/setupTestSuiteEnv.ts
@@ -7,13 +7,14 @@ import { match } from 'ts-pattern'
 import { Script } from 'vm'
 
 import { DbDrop } from '../../../../migrate/src/commands/DbDrop'
+import { DbExecute } from '../../../../migrate/src/commands/DbExecute'
 import { DbPush } from '../../../../migrate/src/commands/DbPush'
 import type { NamedTestSuiteConfig } from './getTestSuiteInfo'
 import { getTestSuiteFolderPath, getTestSuiteSchemaPath } from './getTestSuiteInfo'
 import { Providers } from './providers'
 import { ProviderFlavor, ProviderFlavors } from './relationMode/ProviderFlavor'
 import type { TestSuiteMeta } from './setupTestSuiteMatrix'
-import { ClientMeta } from './types'
+import { AlterStatementCallback, ClientMeta } from './types'
 
 const DB_NAME_VAR = 'PRISMA_DB_NAME'
 
@@ -108,12 +109,34 @@ export async function setupTestSuiteDatabase(
   suiteMeta: TestSuiteMeta,
   suiteConfig: NamedTestSuiteConfig,
   errors: Error[] = [],
+  alterStatementCallback?: AlterStatementCallback,
 ) {
   const schemaPath = getTestSuiteSchemaPath(suiteMeta, suiteConfig)
 
   try {
     const consoleInfoMock = jest.spyOn(console, 'info').mockImplementation()
     await DbPush.new().parse(['--schema', schemaPath, '--force-reset', '--skip-generate'])
+
+    if (alterStatementCallback) {
+      const prismaDir = path.dirname(schemaPath)
+      const timestamp = new Date().getTime()
+      const provider = suiteConfig.matrixOptions['provider'] as Providers
+
+      await fs.promises.mkdir(`${prismaDir}/migrations/${timestamp}`, { recursive: true })
+      await fs.promises.writeFile(`${prismaDir}/migrations/migration_lock.toml`, `provider = "${provider}"`)
+      await fs.promises.writeFile(
+        `${prismaDir}/migrations/${timestamp}/migration.sql`,
+        alterStatementCallback(provider),
+      )
+
+      await DbExecute.new().parse([
+        '--file',
+        `${prismaDir}/migrations/${timestamp}/migration.sql`,
+        '--schema',
+        `${schemaPath}`,
+      ])
+    }
+
     consoleInfoMock.mockRestore()
   } catch (e) {
     errors.push(e as Error)

diff --git a/packages/client/tests/functional/_utils/setupTestSuiteMatrix.ts b/packages/client/tests/functional/_utils/setupTestSuiteMatrix.ts
@@ -76,6 +76,7 @@ function setupTestSuiteMatrix(
           skipDb: options?.skipDb,
           datasourceInfo,
           clientMeta,
+          alterStatementCallback: options?.alterStatementCallback,
         })
 
         globalThis['newPrismaClient'] = (...args) => {

diff --git a/packages/client/tests/functional/_utils/types.ts b/packages/client/tests/functional/_utils/types.ts
@@ -11,6 +11,8 @@ export type MatrixOptions = {
     runtimes: ClientRuntime[]
     reason: string
   }
+  // SQL Migraiton to apply after inital generated migration
+  alterStatementCallback?: AlterStatementCallback
 }
 
 export type NewPrismaClient<T extends new (...args: any) => any> = (
@@ -23,3 +25,5 @@ export type ClientMeta = {
   dataProxy: boolean
   runtime: 'node' | 'edge'
 }
+
+export type AlterStatementCallback = (provider: Providers) => string
diff --git a/...ests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/_matrix.ts b/...ests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/_matrix.ts
@@ -0,0 +1,3 @@
+import { defineMatrix } from '../../_utils/defineMatrix'
+
+export default defineMatrix(() => [[{ provider: 'postgresql' }]])
diff --git a/...nctional/issues/12862-errors-are-obfuscated-by-interactive-transactions/prisma/_schema.ts b/...nctional/issues/12862-errors-are-obfuscated-by-interactive-transactions/prisma/_schema.ts
@@ -0,0 +1,35 @@
+import { idForProvider } from '../../../_utils/idForProvider'
+import testMatrix from '../_matrix'
+
+export default testMatrix.setupSchema(({ provider }) => {
+  return /* Prisma */ `
+    generator client {
+      provider = "prisma-client-js"
+      previewFeatures = ["interactiveTransactions"]
+    }
+    
+    datasource db {
+      provider = "${provider}"
+      url      = env("DATABASE_URI_${provider}")
+    }
+    
+    model User {
+      id    ${idForProvider(provider)}
+      email String  @unique
+      name  String?
+      posts Post[]
+    }
+
+    model Post {
+      id        ${idForProvider(provider)}
+      createdAt DateTime @default(now())
+      updatedAt DateTime @updatedAt
+      title     String
+      content   String?
+      published Boolean  @default(false)
+      viewCount Int      @default(0)
+      author    User?    @relation(fields: [authorId], references: [id])
+      authorId  String?
+    }
+  `
+})