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

feat(sdk): improved error reporting by adding detail to getConfig and getDmmf error #13736

Merged
merged 34 commits into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
df0ab4e
sdk: improved error reporting by adding detail to getConfig and getDm…
jkomyno Jun 9, 2022
2bd0e4d
Merge branch 'main' into feat/detail-errors-on-panics
jkomyno Jun 9, 2022
fac5d19
Merge branch 'main' into feat/detail-errors-on-panics
jkomyno Jun 13, 2022
993cb6c
sdk: added openssl fix proposal
jkomyno Jun 13, 2022
495cdd3
sdk: added comments to errorHelpers
jkomyno Jun 13, 2022
c0f8ddc
sdk: updated structured error output in getDmmf
jkomyno Jun 13, 2022
4f0632b
sdk: updated structured error output in getConfig
jkomyno Jun 13, 2022
5fe64b2
Merge branch 'main' into feat/detail-errors-on-panics
jkomyno Jun 13, 2022
08ebb32
Merge branch 'main' of https://github.com/prisma/prisma into feat/det…
jkomyno Jun 13, 2022
7b28482
ci: fix tests
jkomyno Jun 13, 2022
fa3be88
Merge branch 'feat/detail-errors-on-panics' of https://github.com/pri…
jkomyno Jun 13, 2022
c33e96b
sdk: add tests for loadNodeAPILibrary
jkomyno Jun 15, 2022
7c5e68c
Merge branch 'main' into feat/detail-errors-on-panics
jkomyno Jun 15, 2022
30be1be
Update packages/sdk/src/engine-commands/getConfig.ts
jkomyno Jun 17, 2022
b0c345d
Update packages/sdk/src/engine-commands/getConfig.ts
jkomyno Jun 17, 2022
3da108f
Update packages/sdk/src/engine-commands/getDmmf.ts
jkomyno Jun 17, 2022
bfee01e
Update packages/sdk/src/engine-commands/getDmmf.ts
jkomyno Jun 17, 2022
0dcc301
Merge branch 'main' into feat/detail-errors-on-panics
jkomyno Jun 17, 2022
f23287d
ci: fix tests for getDmmf
jkomyno Jun 17, 2022
2e15166
Merge branch 'main' into feat/detail-errors-on-panics
jkomyno Jun 17, 2022
37163a1
ci: fix tests
jkomyno Jun 17, 2022
05f2323
ci: fix tests
jkomyno Jun 17, 2022
09fad4d
ci: fix tests
jkomyno Jun 17, 2022
0c4d8cb
ci: fix tests
jkomyno Jun 17, 2022
bc68781
ci: fix tests
jkomyno Jun 17, 2022
fa96e90
ci: fixed getConfig tests
jkomyno Jun 17, 2022
e8d3385
Merge branch 'main' into feat/detail-errors-on-panics
jkomyno Jun 17, 2022
3a714d0
chore: merged main and fixed conflicts
jkomyno Jun 21, 2022
89aff5c
chore: simplified query-engine test assertions
jkomyno Jun 21, 2022
3278fdf
chore: fix typo
jkomyno Jun 21, 2022
9239c14
fix: tests
jkomyno Jun 21, 2022
5b30a0e
Merge branch 'main' into feat/detail-errors-on-panics
jkomyno Jun 22, 2022
f49f931
internals: updated openssl error message
jkomyno Jun 22, 2022
c67bd53
Update packages/internals/src/__tests__/engine-commands/queryEngineCo…
jkomyno Jun 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
53 changes: 52 additions & 1 deletion packages/client/src/__tests__/dmmf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import stripAnsi from 'strip-ansi'

import { getDMMF } from '../generation/getDMMF'

const describeIf = (condition: boolean) => (condition ? describe : describe.skip)

describe('dmmf', () => {
test('dmmf enum filter mysql', async () => {
const datamodel = `
Expand Down Expand Up @@ -324,7 +326,55 @@ describe('dmmf', () => {
}
`)
})
})

describeIf(process.env.PRISMA_CLI_QUERY_ENGINE_TYPE === 'library')('dmmf library', () => {
jkomyno marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line jest/no-identical-title
test('dmmf enum should fail on sqlite', async () => {
const datamodel = `
datasource db {
provider = "sqlite"
url = "file:./dev.db"
}

model User {
id Int @id @default(autoincrement())
name String
email String @unique
kind PostKind
}

enum PostKind {
NICE
AWESOME
}`

try {
await getDMMF({ datamodel })
} catch (e) {
expect(stripAnsi(e.message)).toMatchInlineSnapshot(`
Get DMMF: Schema parsing - Error while interacting with query-engine-node-api library
Error code: P1012
error: Error validating: You defined the enum \`PostKind\`. But the current connector does not support enums.
--> schema.prisma:14
|
13 |
14 | enum PostKind {
15 | NICE
16 | AWESOME
17 | }
|

Validation Error Count: 1

Prisma CLI Version : 0.0.0
`)
}
})
})

describeIf(process.env.PRISMA_CLI_QUERY_ENGINE_TYPE === 'binary')('dmmf binary', () => {
// eslint-disable-next-line jest/no-identical-title
test('dmmf enum should fail on sqlite', async () => {
const datamodel = `
datasource db {
Expand All @@ -348,7 +398,8 @@ describe('dmmf', () => {
await getDMMF({ datamodel })
} catch (e) {
expect(stripAnsi(e.message)).toMatchInlineSnapshot(`
Get DMMF: Schema parsing
Get DMMF: Schema parsing - Error while interacting with query-engine binary
Error code: P1012
error: Error validating: You defined the enum \`PostKind\`. But the current connector does not support enums.
--> schema.prisma:14
|
Expand Down
9 changes: 7 additions & 2 deletions packages/client/src/__tests__/generation/generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ if (process.env.CI) {
jest.setTimeout(100_000)
}

describe('generator', () => {
const describeIf = (condition: boolean) => (condition ? describe : describe.skip)

describeIf(
process.env.PRISMA_CLI_QUERY_ENGINE_TYPE === 'library' || process.env.PRISMA_CLI_QUERY_ENGINE_TYPE === undefined,
)('generator', () => {
test('minimal', async () => {
const prismaClientTarget = path.join(__dirname, './node_modules/@prisma/client')
// Make sure, that nothing is cached.
Expand Down Expand Up @@ -117,7 +121,8 @@ describe('generator', () => {
})
} catch (e) {
expect(stripAnsi(e.message)).toMatchInlineSnapshot(`
Get DMMF: Schema parsing
Get DMMF: Schema parsing - Error while interacting with query-engine-node-api library
Error code: P1012
error: Error validating model "public": The model name \`public\` is invalid. It is a reserved name. Please change it. Read more at https://pris.ly/d/naming-models
--> schema.prisma:10
|
Expand Down
187 changes: 123 additions & 64 deletions packages/migrate/src/__tests__/MigrateDev.test.ts

Large diffs are not rendered by default.

15 changes: 11 additions & 4 deletions packages/sdk/src/__tests__/engine-commands/getDmmf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ if (process.env.CI) {
jest.setTimeout(60_000)
}

describe('getDMMF', () => {
const describeIf = (condition: boolean) => (condition ? describe : describe.skip)

describeIf(
process.env.PRISMA_CLI_QUERY_ENGINE_TYPE === 'library' || process.env.PRISMA_CLI_QUERY_ENGINE_TYPE === undefined,
)('getDMMF', () => {
test('simple model, no datasource', async () => {
const dmmf = await getDMMF({
datamodel: `model A {
Expand Down Expand Up @@ -144,7 +148,8 @@ describe('getDMMF', () => {
await getDMMF({ datamodel })
} catch (e) {
expect(stripAnsi(e.message)).toMatchInlineSnapshot(`
"Get DMMF: Schema parsing
"Get DMMF: Schema parsing - Error while interacting with query-engine-node-api library
Error code: P1012
error: Error parsing attribute \\"@default\\": The \`autoincrement()\` default value is used on a non-id field even though the datasource does not support this.
--> schema.prisma:7
|
Expand Down Expand Up @@ -184,7 +189,8 @@ describe('getDMMF', () => {
await getDMMF({ datamodel })
} catch (e) {
expect(stripAnsi(e.message)).toMatchInlineSnapshot(`
"Get DMMF: Schema parsing
"Get DMMF: Schema parsing - Error while interacting with query-engine-node-api library
Error code: P1012
error: Error parsing attribute \\"@default\\": The \`autoincrement()\` default value is used on a non-indexed field even though the datasource does not support this.
--> schema.prisma:7
|
Expand Down Expand Up @@ -340,7 +346,8 @@ describe('getDMMF', () => {
await getDMMF({ datamodel })
} catch (e) {
expect(stripAnsi(e.message)).toMatchInlineSnapshot(`
"Get DMMF: Schema parsing
"Get DMMF: Schema parsing - Error while interacting with query-engine-node-api library
Error code: P1012
error: Field \\"id\\" is already defined on model \\"User\\".
--> schema.prisma:12
|
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { BinaryType } from '@prisma/fetch-engine'
import * as E from 'fp-ts/Either'

import { loadNodeAPILibrary } from '../../engine-commands/queryEngineCommons'
import { resolveBinary } from '../../resolveBinary'
import * as loadUtils from '../../utils/load'

const describeIf = (condition: boolean) => (condition ? describe : describe.skip)

describeIf(process.env.PRISMA_CLI_QUERY_ENGINE_TYPE === 'library')('loadNodeAPILibrary', () => {
it('error path', async () => {
const spyLoadTag = 'error-load'
const spyLoad = jest.spyOn(loadUtils, 'load').mockImplementation((id: string) => {
throw new Error(spyLoadTag)
})

try {
const queryEnginePath = await resolveBinary(BinaryType.libqueryEngine)
const result = await loadNodeAPILibrary(queryEnginePath)()

expect(E.isLeft(result)).toBe(true)

if (E.isLeft(result)) {
expect(result.left.type).toEqual('connection-error')
expect(result.left.reason).toEqual('Unable to establish a connection to query-engine-node-api library.')
expect(result.left.error).toBeTruthy()
}
} finally {
spyLoad.mockRestore()
}
})

it('error path, openssl', async () => {
const spyLoadTag = 'error-load, something something openssl installation'
const spyLoad = jest.spyOn(loadUtils, 'load').mockImplementation((id: string) => {
throw new Error(spyLoadTag)
})

try {
const queryEnginePath = await resolveBinary(BinaryType.libqueryEngine)
const result = await loadNodeAPILibrary(queryEnginePath)()

expect(E.isLeft(result)).toBe(true)

if (E.isLeft(result)) {
expect(result.left.type).toEqual('connection-error')
expect(result.left.reason).toEqual(
`Unable to establish a connection to query-engine-node-api library. Have you tried installing 'openssl'?`,
)
expect(result.left.error).toBeTruthy()
}
} finally {
spyLoad.mockRestore()
}
})

it('happy path', async () => {
const queryEnginePath = await resolveBinary(BinaryType.libqueryEngine)
const result = await loadNodeAPILibrary(queryEnginePath)()

expect(E.isRight(result)).toBe(true)

if (E.isRight(result)) {
expect(result.right.NodeAPIQueryEngineLibrary).toBeTruthy()
expect(result.right.NodeAPIQueryEngineLibrary.QueryEngine).toBeTruthy()
}
})
})
3 changes: 3 additions & 0 deletions packages/sdk/src/engine-commands/errorHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { formatTable } from '../utils/formatTable'
import { version } from '../utils/getVersion'

/**
* Adds `Prisma CLI Version : x.x.x` at the bottom of the error output.
*/
export function addVersionDetailsToErrorMessage(message: string) {
const rows = [['Prisma CLI Version', version]]
return `${message}
Expand Down
87 changes: 70 additions & 17 deletions packages/sdk/src/engine-commands/getConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,46 @@ export type GetConfigOptions = {
retry?: number
ignoreEnvVarErrors?: boolean
}

type GetConfigErrorInit = {
// e.g., `Schema parsing - Error while interacting with query-engine-node-api library`
reason: string

// e.g., Error validating model "public": The model name \`public\` is invalid.
message: string
} & (
| {
// parsed as JSON
readonly _tag: 'parsed'
jkomyno marked this conversation as resolved.
Show resolved Hide resolved

// e.g., `P1012`
errorCode?: string
}
| {
// text
readonly _tag: 'unparsed'
jkomyno marked this conversation as resolved.
Show resolved Hide resolved
}
)

export class GetConfigError extends Error {
constructor(message: string, public readonly _error?: Error) {
super(addVersionDetailsToErrorMessage(`${chalk.redBright.bold('Get config: ')}${message}`))
constructor(params: GetConfigErrorInit) {
const headline = chalk.redBright.bold('Get Config: ')

const constructedErrorMessage = match(params)
.with({ _tag: 'parsed' }, ({ errorCode, message, reason }) => {
const errorCodeMessage = errorCode ? `Error code: ${errorCode}` : ''
return `${reason}
${errorCodeMessage}
${message}`
})
.with({ _tag: 'unparsed' }, ({ message, reason }) => {
const detailsHeader = chalk.red.bold('Details:')
return `${reason}
${detailsHeader}${message}`
})
.exhaustive()

super(addVersionDetailsToErrorMessage(`${headline}${constructedErrorMessage}`))
}
}

Expand Down Expand Up @@ -67,7 +104,7 @@ async function getConfigNodeAPI(options: GetConfigOptions) {
if (E.isLeft(preliminaryEither)) {
const { left: e } = preliminaryEither
debugErrorType(e)
throw new GetConfigError(e.reason, e.error)
throw new GetConfigError({ _tag: 'unparsed', message: e.error.message, reason: e.reason })
}
const { queryEnginePath } = preliminaryEither.right
debug(`Using CLI Query Engine (Node-API Library) at: ${queryEnginePath}`)
Expand Down Expand Up @@ -129,7 +166,7 @@ async function getConfigNodeAPI(options: GetConfigOptions) {
() => JSON.parse(errorOutput),
() => {
debug(`Coudln't apply JSON.parse to "${errorOutput}"`)
return new GetConfigError(errorOutput, e.error)
return new GetConfigError({ _tag: 'unparsed', message: errorOutput, reason: e.reason })
},
),
E.map((errorOutputAsJSON: Record<string, string>) => {
Expand All @@ -148,12 +185,20 @@ async function getConfigNodeAPI(options: GetConfigOptions) {

const message = match(errorOutputAsJSON)
.with({ error_code: 'P1012' }, (error: Record<string, string>) => {
return chalk.redBright(`Schema Parsing ${error.error_code}\n\n`) + error.message + '\n'
return chalk.redBright(`Schema parsing ${error.error_code}\n\n`) + error.message + '\n'
})
.otherwise((error: any) => {
return chalk.redBright(`${error.error_code}\n\n`) + error
})
return new GetConfigError(message, e.error)

const { error_code: errorCode } = errorOutputAsJSON as { error_code: string | undefined }

return new GetConfigError({
_tag: 'parsed',
message: errorOutputAsJSON.message,
reason: `${chalk.redBright.bold('Schema parsing')} - ${e.reason}`,
errorCode,
})
}),
E.getOrElseW(identity),
)
Expand All @@ -162,7 +207,7 @@ async function getConfigNodeAPI(options: GetConfigOptions) {
})
.otherwise((e) => {
debugErrorType(e)
return new GetConfigError(e.reason, e.error)
return new GetConfigError({ _tag: 'unparsed', message: e.error.message, reason: e.reason })
})

throw error
Expand All @@ -179,7 +224,7 @@ async function getConfigBinary(options: GetConfigOptions) {
if (E.isLeft(preliminaryEither)) {
const { left: e } = preliminaryEither
debugErrorType(e)
throw new GetConfigError(e.reason, e.error)
throw new GetConfigError({ _tag: 'unparsed', message: e.error.message, reason: e.reason })
}
const { queryEnginePath, tempDatamodelPath } = preliminaryEither.right
debug(`Using CLI Query Engine (Binary) at: ${queryEnginePath}`)
Expand Down Expand Up @@ -287,31 +332,39 @@ async function getConfigBinary(options: GetConfigOptions) {
() => JSON.parse(errorOutput),
() => {
debug(`Coudln't apply JSON.parse to "${errorOutput}"`)
return new GetConfigError(errorOutput, e.error)
return new GetConfigError({ _tag: 'unparsed', message: errorOutput, reason: e.reason })
},
),
E.map((errorOutputAsJSON: Record<string, string>) => {
const defaultMessage = `${chalk.redBright(errorOutputAsJSON.message)}\n`
const message = match(errorOutputAsJSON)
.with({ error_code: 'P1012' }, (error) => {
return chalk.redBright(`Schema Parsing ${error.error_code}\n\n`) + defaultMessage
const getConfigErrorInit = match(errorOutputAsJSON)
.with({ error_code: 'P1012' }, (eJSON) => {
return {
reason: `${chalk.redBright.bold('Schema parsing')} - ${e.reason}`,
errorCode: eJSON.error_code,
}
})
.with({ error_code: P.string }, (error) => {
return chalk.redBright(`${error.error_code}\n\n`) + defaultMessage
.with({ error_code: P.string }, (eJSON) => {
return {
reason: e.reason,
errorCode: eJSON.error_code,
}
})
.otherwise(() => {
return defaultMessage
return {
reason: e.reason,
}
})

return new GetConfigError(message, e.error)
return new GetConfigError({ _tag: 'parsed', message: defaultMessage, ...getConfigErrorInit })
}),
E.getOrElse(identity),
)
return actualError
})
.otherwise((e) => {
debugErrorType(e)
return new GetConfigError(e.reason, e.error)
return new GetConfigError({ _tag: 'unparsed', message: e.error.message, reason: e.reason })
})

throw error
Expand Down