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): remove retries for all errors except network connection/socket errors #15684

Merged
merged 55 commits into from
Nov 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
4dd3827
test: add coverage for gh-12862
danstarns Sep 29, 2022
5a982c0
test: only test postgresql
danstarns Sep 29, 2022
dfbfec2
test: * add alterStatement
danstarns Sep 29, 2022
53d98be
Merge https://github.com/prisma/prisma into fix/12862
danstarns Sep 29, 2022
d306858
refactor: remove only binary engine test
danstarns Sep 30, 2022
71d99ec
Merge branch 'main' into fix/12862
danstarns Oct 4, 2022
32ac035
refactor: remove retries
danstarns Oct 6, 2022
17575a1
Revert "refactor: remove retries"
danstarns Oct 20, 2022
2a1012e
Merge branch 'main' into remove-retries
danstarns Oct 20, 2022
d2e405b
fix: only throw on async errors
danstarns Oct 20, 2022
2ffae9b
fix: move error throw under retry logic
danstarns Oct 20, 2022
7b58b1b
Merge branch 'main' into remove-retries
danstarns Oct 20, 2022
6cf16e0
Merge branch 'main' into fix/12862
danstarns Oct 20, 2022
aba2558
fix: only retry when clientMethod is a read
danstarns Oct 24, 2022
dd76c15
Merge branch 'main' into remove-retries
danstarns Oct 24, 2022
d6a4c4c
Merge branch 'remove-retries' of github.com:prisma/prisma into remove…
danstarns Oct 24, 2022
de6a513
Merge branch 'main' into remove-retries
danstarns Oct 25, 2022
ebe9bc5
Merge branch 'main' into remove-retries
danstarns Oct 25, 2022
67669e6
Merge branch 'main' into remove-retries
danstarns Oct 26, 2022
8fc7f1a
Merge branch 'main' into remove-retries
danstarns Oct 31, 2022
56f2281
test: correct types and snapshots for obj style batch
danstarns Oct 31, 2022
8d291a1
Merge branch 'main' into fix/12862
danstarns Oct 31, 2022
d44833a
refactor: add a callback to alterStatement
danstarns Oct 31, 2022
4b3cddf
refactor: remove unused imports
danstarns Oct 31, 2022
fdc0fd0
Merge branch 'main' into remove-retries
danstarns Nov 1, 2022
49f76e7
Merge branch 'main' into fix/12862
danstarns Nov 1, 2022
82ff11e
Merge branch 'main' into remove-retries
danstarns Nov 1, 2022
a7194e3
Merge branch 'main' into remove-retries
danstarns Nov 2, 2022
f2eda75
Merge branch 'main' into fix/12862
danstarns Nov 2, 2022
7c4eba1
Merge branch 'main' into remove-retries
danstarns Nov 9, 2022
e519dea
Merge branch 'remove-retries' of github.com:prisma/prisma into remove…
danstarns Nov 9, 2022
2e45a08
Merge branch 'main' into fix/12862
danstarns Nov 9, 2022
7636a8f
Merge branch 'fix/12862' into remove-retries
danstarns Nov 9, 2022
275915b
test: update snapshots
danstarns Nov 9, 2022
6e73192
Merge branch 'main' into remove-retries
danstarns Nov 9, 2022
4700313
Merge branch 'main' into remove-retries
danstarns Nov 15, 2022
99657ea
fix: return from some
danstarns Nov 15, 2022
e3aeba4
Merge branch 'remove-retries' of github.com:prisma/prisma into remove…
danstarns Nov 15, 2022
8a5ccb9
refactor: name test correctly
danstarns Nov 15, 2022
a7f3d06
fix: pass thru _transaction
danstarns Nov 15, 2022
5c383af
Merge branch 'main' into remove-retries
danstarns Nov 17, 2022
3f7edc8
feat: add raw to isWrite check
danstarns Nov 17, 2022
4c146c7
Merge branch 'main' into remove-retries
danstarns Nov 19, 2022
56701ca
refactor: add isWriteRequest method
danstarns Nov 19, 2022
c36848f
Merge branch 'main' into remove-retries
danstarns Nov 21, 2022
b6d5e16
Update packages/engine-core/src/binary/BinaryEngine.ts
danstarns Nov 22, 2022
be4ad59
Update packages/engine-core/src/binary/BinaryEngine.ts
danstarns Nov 22, 2022
daf88d9
Update packages/engine-core/src/binary/BinaryEngine.ts
danstarns Nov 22, 2022
3bd5e2b
Merge branch 'main' into remove-retries
danstarns Nov 22, 2022
edd1b6d
Merge remote-tracking branch 'origin/main' into remove-retries
aqrln Nov 23, 2022
5d13702
Extract duplicate type definitions
aqrln Nov 23, 2022
bb181f4
Check document type instead of hardcoding the list of write ops
aqrln Nov 23, 2022
1b623f6
Fix type error in tests
aqrln Nov 23, 2022
47e94c5
Remove unused clientMethod
aqrln Nov 23, 2022
b7ed64b
Fix snapshot
aqrln Nov 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 46 additions & 13 deletions packages/client/src/__tests__/batching.test.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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) => {
Expand Down Expand Up @@ -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"
}) {
Expand All @@ -96,7 +99,7 @@ describe('batching', () => {
coinflips
}
},
query {
query {
findUniqueUser(where: {
id: "2"
}) {
Expand All @@ -115,7 +118,9 @@ describe('batching', () => {
coinflips
}
},
],
],
transaction: undefined,
},
]
`)
expect(requests).toMatchInlineSnapshot(`Array []`)
Expand All @@ -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) => {
Expand Down Expand Up @@ -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"
}) {
Expand All @@ -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"
}) {
Expand All @@ -221,6 +238,8 @@ describe('batching', () => {
coinflips
}
},
transaction: undefined,
},
]
`)
})
Expand All @@ -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) => {
Expand Down Expand Up @@ -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"
}) {
Expand All @@ -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"
}) {
Expand All @@ -326,6 +357,8 @@ describe('batching', () => {
coinflips
}
},
transaction: undefined,
},
]
`)
})
Expand Down
16 changes: 14 additions & 2 deletions packages/client/src/runtime/RequestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand All @@ -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
Expand Down
25 changes: 24 additions & 1 deletion packages/client/tests/functional/_utils/setupTestSuiteEnv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ function setupTestSuiteMatrix(
skipDb: options?.skipDb,
datasourceInfo,
clientMeta,
alterStatementCallback: options?.alterStatementCallback,
})

globalThis['newPrismaClient'] = (...args) => {
Expand Down
4 changes: 4 additions & 0 deletions packages/client/tests/functional/_utils/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> = (
Expand All @@ -23,3 +25,5 @@ export type ClientMeta = {
dataProxy: boolean
runtime: 'node' | 'edge'
}

export type AlterStatementCallback = (provider: Providers) => string
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { defineMatrix } from '../../_utils/defineMatrix'

export default defineMatrix(() => [[{ provider: 'postgresql' }]])
Original file line number Diff line number Diff line change
@@ -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?
}
`
})