From e50510f05d7a4df6ee4b28e76753463fa3759347 Mon Sep 17 00:00:00 2001 From: Daniel Starns Date: Wed, 23 Nov 2022 22:00:54 +0000 Subject: [PATCH] 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: https://github.com/prisma/prisma/issues/12862 Co-authored-by: Alexey Orlenko --- .../client/src/__tests__/batching.test.ts | 59 ++++++-- packages/client/src/runtime/RequestHandler.ts | 16 +- .../functional/_utils/setupTestSuiteClient.ts | 6 +- .../functional/_utils/setupTestSuiteEnv.ts | 25 +++- .../functional/_utils/setupTestSuiteMatrix.ts | 1 + .../client/tests/functional/_utils/types.ts | 4 + .../_matrix.ts | 3 + .../prisma/_schema.ts | 35 +++++ .../tests.ts | 85 +++++++++++ .../src/__tests__/LibraryEngine.test.ts | 22 ++- .../engine-core/src/binary/BinaryEngine.ts | 139 ++++++++++-------- packages/engine-core/src/common/Engine.ts | 32 ++-- .../src/data-proxy/DataProxyEngine.ts | 15 +- .../engine-core/src/library/LibraryEngine.ts | 17 ++- 14 files changed, 339 insertions(+), 120 deletions(-) create mode 100644 packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/_matrix.ts create mode 100644 packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/prisma/_schema.ts create mode 100644 packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/tests.ts diff --git a/packages/client/src/__tests__/batching.test.ts b/packages/client/src/__tests__/batching.test.ts index fe8426b13d23..761558affb08 100644 --- 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 index f1eaa5778aa0..185654600c41 100644 --- 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 index a17fb8e4efe1..90854137d4d1 100644 --- 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 index ecce814f660f..49b2fdf1b15f 100644 --- 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 index 33687dde8ef5..5c60f15de615 100644 --- 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 index f9a40006bc4d..0f08452dd78c 100644 --- 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 any> = ( @@ -23,3 +25,5 @@ export type ClientMeta = { dataProxy: boolean runtime: 'node' | 'edge' } + +export type AlterStatementCallback = (provider: Providers) => string diff --git a/packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/_matrix.ts b/packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/_matrix.ts new file mode 100644 index 000000000000..877cd1b738af --- /dev/null +++ b/packages/client/tests/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/packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/prisma/_schema.ts b/packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/prisma/_schema.ts new file mode 100644 index 000000000000..9e504f25a43e --- /dev/null +++ b/packages/client/tests/functional/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? + } + ` +}) diff --git a/packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/tests.ts b/packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/tests.ts new file mode 100644 index 000000000000..655fc7c04dfc --- /dev/null +++ b/packages/client/tests/functional/issues/12862-errors-are-obfuscated-by-interactive-transactions/tests.ts @@ -0,0 +1,85 @@ +import { faker } from '@faker-js/faker' +// @ts-ignore +import type { PrismaClient } from '@prisma/client' + +import testMatrix from './_matrix' + +declare let prisma: PrismaClient + +// https://github.com/prisma/prisma/issues/12862 +testMatrix.setupTestSuite( + () => { + test('should propagate the correct error when a method fails', async () => { + const user = await prisma.user.create({ + data: { + email: faker.internet.email(), + name: faker.name.firstName(), + }, + }) + + await expect( + prisma.post.create({ + data: { + authorId: user.id, + title: faker.lorem.sentence(), + viewCount: -1, // should fail, must be >= 0 + }, + }), + ).rejects.toThrowError('violates check constraint \\"post_viewcount_check\\"') + }) + + test('should propagate the correct error when a method fails inside an transaction', async () => { + const user = await prisma.user.create({ + data: { + email: faker.internet.email(), + name: faker.name.firstName(), + }, + }) + + await expect( + prisma.$transaction([ + prisma.post.create({ + data: { + authorId: user.id, + title: faker.lorem.sentence(), + viewCount: -1, // should fail, must be >= 0 + }, + }), + ]), + ).rejects.toThrowError('violates check constraint \\"post_viewcount_check\\"') + }) + + test('should propagate the correct error when a method fails inside an interactive transaction', async () => { + await expect( + prisma.$transaction(async (client) => { + const user = await client.user.create({ + data: { + email: faker.internet.email(), + name: faker.name.firstName(), + }, + }) + + const post = await client.post.create({ + data: { + authorId: user.id, + title: faker.lorem.sentence(), + viewCount: -1, // should fail, must be >= 0 + }, + }) + + return post + }), + ).rejects.toThrowError('violates check constraint \\"post_viewcount_check\\"') + }) + }, + { + optOut: { + from: ['cockroachdb', 'mongodb', 'mysql', 'sqlite', 'sqlserver'], + reason: 'Issue relates to postgresql only', + }, + alterStatementCallback: () => ` + ALTER TABLE "Post" + ADD CONSTRAINT Post_viewCount_check CHECK ("viewCount" >= 0); + `, + }, +) diff --git a/packages/engine-core/src/__tests__/LibraryEngine.test.ts b/packages/engine-core/src/__tests__/LibraryEngine.test.ts index 7cb8fb4509bf..8b20aa11ab46 100644 --- a/packages/engine-core/src/__tests__/LibraryEngine.test.ts +++ b/packages/engine-core/src/__tests__/LibraryEngine.test.ts @@ -98,7 +98,9 @@ test('responds to panic GraphQL error with PrismaClientRustPanicError', async () }), ) - await expect(engine.request('query Foo { id }')).rejects.toBeInstanceOf(PrismaClientRustPanicError) + await expect(engine.request({ query: 'query Foo { id }', isWrite: false })).rejects.toBeInstanceOf( + PrismaClientRustPanicError, + ) }) test('responds to panic GraphQL error with an error, containing github link', async () => { @@ -110,7 +112,7 @@ test('responds to panic GraphQL error with an error, containing github link', as }), ) - await expect(engine.request('query Foo { id }')).rejects.toMatchObject({ + await expect(engine.request({ query: 'query Foo { id }', isWrite: false })).rejects.toMatchObject({ message: expect.stringContaining('https://github.com/prisma/prisma/issues'), }) }) @@ -120,7 +122,9 @@ test('responds to panic exception with PrismaClientRustPanicError', async () => rustEngineMock.query.mockRejectedValue(panicException()) - await expect(engine.request('query Foo { id }')).rejects.toBeInstanceOf(PrismaClientRustPanicError) + await expect(engine.request({ query: 'query Foo { id }', isWrite: false })).rejects.toBeInstanceOf( + PrismaClientRustPanicError, + ) }) test('responds to panic exception with an error, containing github link', async () => { @@ -128,7 +132,7 @@ test('responds to panic exception with an error, containing github link', async rustEngineMock.query.mockRejectedValue(panicException()) - await expect(engine.request('query Foo { id }')).rejects.toMatchObject({ + await expect(engine.request({ query: 'query Foo { id }', isWrite: false })).rejects.toMatchObject({ message: expect.stringContaining('https://github.com/prisma/prisma/issues'), }) }) @@ -142,7 +146,9 @@ test('responds to known error with PrismaClientKnownRequestError', async () => { }), ) - await expect(engine.request('query Foo { id }')).rejects.toBeInstanceOf(PrismaClientKnownRequestError) + await expect(engine.request({ query: 'query Foo { id }', isWrite: false })).rejects.toBeInstanceOf( + PrismaClientKnownRequestError, + ) }) test('responds to unknown error with PrismaClientUnknownRequestError', async () => { @@ -154,7 +160,9 @@ test('responds to unknown error with PrismaClientUnknownRequestError', async () }), ) - await expect(engine.request('query Foo { id }')).rejects.toBeInstanceOf(PrismaClientUnknownRequestError) + await expect(engine.request({ query: 'query Foo { id }', isWrite: false })).rejects.toBeInstanceOf( + PrismaClientUnknownRequestError, + ) }) test('responds to a non panic error without github link', async () => { @@ -166,7 +174,7 @@ test('responds to a non panic error without github link', async () => { }), ) - await expect(engine.request('query Foo { id }')).rejects.toMatchObject({ + await expect(engine.request({ query: 'query Foo { id }', isWrite: false })).rejects.toMatchObject({ message: expect.not.stringContaining('https://github.com/'), }) }) diff --git a/packages/engine-core/src/binary/BinaryEngine.ts b/packages/engine-core/src/binary/BinaryEngine.ts index f12a06a9bd68..050c37895d2f 100644 --- a/packages/engine-core/src/binary/BinaryEngine.ts +++ b/packages/engine-core/src/binary/BinaryEngine.ts @@ -17,12 +17,12 @@ import { URL } from 'url' import { promisify } from 'util' import type { - BatchTransactionOptions, DatasourceOverwrite, EngineConfig, EngineEventType, GetConfigResult, - InteractiveTransactionOptions, + RequestBatchOptions, + RequestOptions, } from '../common/Engine' import { Engine } from '../common/Engine' import { PrismaClientInitializationError } from '../common/errors/PrismaClientInitializationError' @@ -930,12 +930,13 @@ You very likely have the wrong "binaryTarget" defined in the schema.prisma file. return this.lastVersion } - async request( - query: string, - headers: QueryEngineRequestHeaders = {}, - _transaction?: InteractiveTransactionOptions, + async request({ + query, + headers = {}, numTry = 1, - ): Promise> { + isWrite, + transaction, + }: RequestOptions): Promise> { await this.start() // TODO: we don't need the transactionId "runtime header" anymore, we can use the txInfo object here @@ -965,23 +966,25 @@ You very likely have the wrong "binaryTarget" defined in the schema.prisma file. } catch (e: any) { logger('req - e', e) - await this.handleRequestError(e, numTry <= MAX_REQUEST_RETRIES) + const { error, shouldRetry } = await this.handleRequestError(e) + // retry - if (numTry <= MAX_REQUEST_RETRIES) { + if (numTry <= MAX_REQUEST_RETRIES && shouldRetry && !isWrite) { logger('trying a retry now') - return this.request(query, headers, _transaction, numTry + 1) + return this.request({ query, headers, numTry: numTry + 1, isWrite, transaction }) } - } - return null as any // needed to make TS happy + throw error + } } - async requestBatch( - queries: string[], - headers: QueryEngineRequestHeaders = {}, - transaction?: BatchTransactionOptions, + async requestBatch({ + queries, + headers = {}, + transaction, numTry = 1, - ): Promise[]> { + containsWrite, + }: RequestBatchOptions): Promise[]> { await this.start() const request: QueryEngineBatchRequest = { @@ -1013,15 +1016,21 @@ You very likely have the wrong "binaryTarget" defined in the schema.prisma file. } }) .catch(async (e) => { - const isError = await this.handleRequestError(e, numTry < 3) - if (!isError) { + const { error, shouldRetry } = await this.handleRequestError(e) + if (shouldRetry && !containsWrite) { // retry if (numTry <= MAX_REQUEST_RETRIES) { - return this.requestBatch(queries, headers, transaction, numTry + 1) + return this.requestBatch({ + queries, + headers, + transaction, + numTry: numTry + 1, + containsWrite, + }) } } - throw isError + throw error }) } @@ -1118,66 +1127,70 @@ You very likely have the wrong "binaryTarget" defined in the schema.prisma file. }) } - private handleRequestError = async (error: Error & { code?: string }, graceful = false) => { + private handleRequestError = async ( + error: Error & { code?: string }, + ): Promise<{ error: Error & { code?: string }; shouldRetry: boolean }> => { debug({ error }) + // if we are starting, wait for it before we handle any error if (this.startPromise) { await this.startPromise } + // matching on all relevant error codes from + // https://github.com/nodejs/undici/blob/2.x/lib/core/errors.js + const isNetworkError = [ + 'ECONNRESET', + 'ECONNREFUSED', + 'UND_ERR_CLOSED', + 'UND_ERR_SOCKET', + 'UND_ERR_DESTROYED', + 'UND_ERR_ABORTED', + ].includes(error.code as string) + if (error instanceof PrismaClientKnownRequestError) { - throw error + return { error, shouldRetry: false } } - this.throwAsyncErrorIfExists() - - // A currentRequestPromise is only being canceled by the sendPanic function - if (this.currentRequestPromise?.isCanceled) { + try { this.throwAsyncErrorIfExists() - } else if ( - // matching on all relevant error codes from - // https://github.com/nodejs/undici/blob/2.x/lib/core/errors.js - error.code === 'ECONNRESET' || - error.code === 'ECONNREFUSED' || - error.code === 'UND_ERR_CLOSED' || - error.code === 'UND_ERR_SOCKET' || - error.code === 'UND_ERR_DESTROYED' || - error.code === 'UND_ERR_ABORTED' || - error.message.toLowerCase().includes('client is destroyed') || - error.message.toLowerCase().includes('other side closed') || - error.message.toLowerCase().includes('the client is closed') - ) { - if (this.globalKillSignalReceived && !this.child?.connected) { - throw new PrismaClientUnknownRequestError( - `The Node.js process already received a ${this.globalKillSignalReceived} signal, therefore the Prisma query engine exited -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.`, - { clientVersion: this.clientVersion! }, - ) - } - this.throwAsyncErrorIfExists() + // A currentRequestPromise is only being canceled by the sendPanic function + if (this.currentRequestPromise?.isCanceled) { + this.throwAsyncErrorIfExists() + } else if (isNetworkError) { + if (this.globalKillSignalReceived && !this.child?.connected) { + throw new PrismaClientUnknownRequestError( + `The Node.js process already received a ${this.globalKillSignalReceived} signal, therefore the Prisma query engine exited + 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.`, + { clientVersion: this.clientVersion! }, + ) + } + + this.throwAsyncErrorIfExists() - if (this.startCount > MAX_STARTS) { - // if we didn't throw yet, which is unlikely, we want to poll on stderr / stdout here - // to get an error first - for (let i = 0; i < 5; i++) { - await new Promise((r) => setTimeout(r, 50)) - this.throwAsyncErrorIfExists(true) + if (this.startCount > MAX_STARTS) { + // if we didn't throw yet, which is unlikely, we want to poll on stderr / stdout here + // to get an error first + for (let i = 0; i < 5; i++) { + await new Promise((r) => setTimeout(r, 50)) + this.throwAsyncErrorIfExists(true) + } + + throw new Error(`Query engine is trying to restart, but can't. + Please look into the logs or turn on the env var DEBUG=* to debug the constantly restarting query engine.`) } - throw new Error(`Query engine is trying to restart, but can't. -Please look into the logs or turn on the env var DEBUG=* to debug the constantly restarting query engine.`) } - } - if (!graceful) { this.throwAsyncErrorIfExists(true) + throw error + } catch (e) { + return { error: e, shouldRetry: isNetworkError } } - - return false } async metrics(options: MetricsOptionsJson): Promise diff --git a/packages/engine-core/src/common/Engine.ts b/packages/engine-core/src/common/Engine.ts index a9f7110fd9b4..a443aa09917f 100644 --- a/packages/engine-core/src/common/Engine.ts +++ b/packages/engine-core/src/common/Engine.ts @@ -1,4 +1,4 @@ -import type { DataSource, DMMF, EnvValue, GeneratorConfig } from '@prisma/generator-helper' +import type { DataSource, DMMF, GeneratorConfig } from '@prisma/generator-helper' import { TracingConfig } from '../tracing/getTracingConfig' import type { Metrics, MetricsOptionsJson, MetricsOptionsPrometheus } from './types/Metrics' @@ -24,6 +24,22 @@ export type BatchTransactionOptions = { export type InteractiveTransactionOptions = Transaction.Info +export type RequestOptions = { + query: string + headers?: QueryEngineRequestHeaders + numTry?: number + transaction?: InteractiveTransactionOptions + isWrite: boolean +} + +export type RequestBatchOptions = { + queries: string[] + headers?: QueryEngineRequestHeaders + transaction?: BatchTransactionOptions + numTry?: number + containsWrite: boolean +} + // TODO Move shared logic in here export abstract class Engine { abstract on(event: EngineEventType, listener: (args?: any) => any): void @@ -32,18 +48,8 @@ export abstract class Engine { abstract getConfig(): Promise abstract getDmmf(): Promise abstract version(forceRun?: boolean): Promise | string - abstract request( - query: string, - headers?: QueryEngineRequestHeaders, - transaction?: InteractiveTransactionOptions, - numTry?: number, - ): Promise> - abstract requestBatch( - queries: string[], - headers?: QueryEngineRequestHeaders, - transaction?: BatchTransactionOptions, - numTry?: number, - ): Promise[]> + abstract request(options: RequestOptions): Promise> + abstract requestBatch(options: RequestBatchOptions): Promise[]> abstract transaction( action: 'start', headers: Transaction.TransactionHeaders, diff --git a/packages/engine-core/src/data-proxy/DataProxyEngine.ts b/packages/engine-core/src/data-proxy/DataProxyEngine.ts index 8167bc5741e9..c1d8c4375588 100644 --- a/packages/engine-core/src/data-proxy/DataProxyEngine.ts +++ b/packages/engine-core/src/data-proxy/DataProxyEngine.ts @@ -3,12 +3,13 @@ import { DMMF } from '@prisma/generator-helper' import EventEmitter from 'events' import type { - BatchTransactionOptions, EngineConfig, EngineEventType, GetConfigResult, InlineDatasource, InteractiveTransactionOptions, + RequestBatchOptions, + RequestOptions, } from '../common/Engine' import { Engine } from '../common/Engine' import { PrismaClientUnknownRequestError } from '../common/errors/PrismaClientUnknownRequestError' @@ -139,22 +140,14 @@ export class DataProxyEngine extends Engine { } } - request( - query: string, - headers: QueryEngineRequestHeaders = {}, - transaction?: InteractiveTransactionOptions, - ): Promise> { + request({ query, headers = {}, transaction }: RequestOptions) { this.logEmitter.emit('query', { query }) // TODO: `elapsed`? return this.requestInternal({ query, variables: {} }, headers, transaction) } - async requestBatch( - queries: string[], - headers: QueryEngineRequestHeaders = {}, - transaction?: BatchTransactionOptions, - ): Promise[]> { + async requestBatch({ queries, headers = {}, transaction }: RequestBatchOptions): Promise[]> { const isTransaction = Boolean(transaction) this.logEmitter.emit('query', { query: `Batch${isTransaction ? ' in transaction' : ''} (${queries.length}):\n${queries.join('\n')}`, diff --git a/packages/engine-core/src/library/LibraryEngine.ts b/packages/engine-core/src/library/LibraryEngine.ts index 2fd701de2b48..489f1af70823 100644 --- a/packages/engine-core/src/library/LibraryEngine.ts +++ b/packages/engine-core/src/library/LibraryEngine.ts @@ -6,7 +6,13 @@ import chalk from 'chalk' import EventEmitter from 'events' import fs from 'fs' -import type { BatchTransactionOptions, DatasourceOverwrite, EngineConfig, EngineEventType } from '../common/Engine' +import type { + DatasourceOverwrite, + EngineConfig, + EngineEventType, + RequestBatchOptions, + RequestOptions, +} from '../common/Engine' import { Engine } from '../common/Engine' import { PrismaClientInitializationError } from '../common/errors/PrismaClientInitializationError' import { PrismaClientKnownRequestError } from '../common/errors/PrismaClientKnownRequestError' @@ -25,7 +31,6 @@ import type { QueryEnginePanicEvent, QueryEngineQueryEvent, QueryEngineRequest, - QueryEngineRequestHeaders, QueryEngineResult, RustRequestError, SyncRustError, @@ -445,7 +450,7 @@ You may have to run ${chalk.greenBright('prisma generate')} for your changes to return this.library?.debugPanic(message) as Promise } - async request(query: string, headers: QueryEngineRequestHeaders = {}): Promise<{ data: T; elapsed: number }> { + async request({ query, headers = {} }: RequestOptions): Promise<{ data: T; elapsed: number }> { debug(`sending request, this.libraryStarted: ${this.libraryStarted}`) const request: QueryEngineRequest = { query, variables: {} } const headerStr = JSON.stringify(headers) // object equivalent to http headers for the library @@ -489,11 +494,7 @@ You may have to run ${chalk.greenBright('prisma generate')} for your changes to } } - async requestBatch( - queries: string[], - headers: QueryEngineRequestHeaders = {}, - transaction?: BatchTransactionOptions, - ): Promise[]> { + async requestBatch({ queries, headers = {}, transaction }: RequestBatchOptions): Promise[]> { debug('requestBatch') const request: QueryEngineBatchRequest = { batch: queries.map((query) => ({ query, variables: {} })),