keystone icon indicating copy to clipboard operation
keystone copied to clipboard

Prisma Errors no longer throw in resolvers and instead are swallowed and returned as a GraphQL error object

Open AdamBerri opened this issue 1 year ago • 3 comments

Prisma errors (like PrismaClientKnownRequestError) are being caught and transformed into GraphQL errors. These transformed errors are being returned as part of the query result rather than being thrown.

  1. Create a resolver
  2. Add a try/catch block
  3. In the try block use context.prisma.post.findUniqueOrThrow({where: { id: postId }}) or context.prisma.post.findFirstOrThrow({where: { id: postId }})
  4. Return the result in the error
  5. in the catch block, log the error
  6. import resolver into keystone.ts
  7. call the resolver with an ID that you know does not exist (in order to create the prisma error)

Note: This will also occur on context.prisma.post.update() if the ID being updated does not exist.

I expect the catch block to be invoked. Instead an error object is returned and the catch block is never reached.

image image

Resolver Incorporated into schema.ts this way: return graphql.extend((base) => { return { query: { ...resolverForTesting(base) }, mutation: {} } })(merged)

Issue seems to be introduced by this commit: https://github.com/keystonejs/keystone/commit/c0dacbf3426961e645a56fb9451415ac6b561b0e If I understand correctly, the removal of runWithPrisma may have had an unintended consequence where errors no longer throw when caused by Prisma.

AdamBerri avatar Aug 01 '24 22:08 AdamBerri

I can confirm this behavior as well. The main problem here is that now raw prisma operations can return either the expected result or this new hidden GraphQL error. Which isn’t reflected in typescript, thus unforeseen bugs. A simple workaround could be to extendPrismaSchema to do the type checking of the results and throw if a GraphQLError, but that seems like a workaround to the system itself

flippinjoe avatar Aug 02 '24 02:08 flippinjoe

@AdamBerri @flippinjoe do you have a minimal reproduction that I can use to add tests for this? I'm not following exactly what behaviour you had previously compared to now.

dcousens avatar Aug 02 '24 09:08 dcousens

@dcousens what kind of reproduction would be useful. The steps that @AdamBerri laid out are pretty thorough IMO. Happy to provide whatever you need. In my use case:

const result = await context.prisma.post.findFirstOrThrow({ where: { id: "some-id" })

^ Previously, this would throw GraphQLError. Now that behavior has changed, and result is actually being returned as a GraphQLError.

flippinjoe avatar Aug 02 '24 16:08 flippinjoe

The systems I have built with Keystone rely heavily on GraphQL schema extensions and custom Prisma code. It is totally unexpected that Prisma exceptions are swallowed.

Error handling now requires checking the result after every Prisma call and doing string comparisions to determine the underlying Prisma error as the Prisma error code is not exposed. I now have duplicate error handling in my code as I have left the try/catch blocks intact in case this behaviour is fixed. This is obviously not an ideal situation.

davidmclark avatar Jan 25 '25 12:01 davidmclark