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

Prisma Integration Incompatible with 5.0.0 #8532

Closed
3 tasks done
mwillbanks opened this issue Jul 13, 2023 · 17 comments
Closed
3 tasks done

Prisma Integration Incompatible with 5.0.0 #8532

mwillbanks opened this issue Jul 13, 2023 · 17 comments

Comments

@mwillbanks
Copy link

mwillbanks commented Jul 13, 2023

Is there an existing issue for this?

How do you use Sentry?

Self-hosted/on-premise

Which SDK are you using?

@sentry/serverless

SDK Version

7.58.1

Framework Version

No response

Link to Sentry event

No response

SDK Setup

import * as Sentry from '@sentry/serverless';
import { PrismaClient } from '@prisma/client';

const prisma = new PrismaClient();
Sentry.AWSLambda.init({
  dsn: process.env.SENTRY_DSN,
  environment: process.env.STAGE,
  tracesSampleRate: 1.0,
  release: process.env.VERSION,
  integrations: [new Sentry.AWSLambda.Integrations.Prisma({ client: prisma })],
});

Steps to Reproduce

  1. Use @vendia/serverless-express
  2. Use serverless-offline
  3. yarn start
  4. Navigate to any route

Expected Result

Route would load as previous.

Actual Result

✖ Unhandled exception in handler 'api'.
✖ TypeError: Converting circular structure to JSON
      --> starting at object with constructor 'zr'
      |     property 'client' -> object with constructor 't'
      --- property '_fetcher' closes the circle
      at JSON.stringify (<anonymous>)
      at new Prisma (/Volumes/Projects/xxxxx-api/node_modules/@sentry-internal/tracing/cjs/node/integrations/prisma.js:51:93)
      at init (/Volumes/Projects/xxxxx-api/.build/helper/sentry.js:39:24)
      at configureExpress (/Volumes/Projects/xxxxx-api/.build/helper/sentry.js:51:22)
      at Object.<anonymous> (/Volumes/Projects/xxxxx-api/.build/api/app.js:28:31)
      at Module._compile (node:internal/modules/cjs/loader:1196:14)
      at Object.Module._extensions..js (node:internal/modules/cjs/loader:1250:10)
      at Module.load (node:internal/modules/cjs/loader:1074:32)
      at Function.Module._load (node:internal/modules/cjs/loader:909:12)
      at Module.require (node:internal/modules/cjs/loader:1098:19)
✖ Converting circular structure to JSON
      --> starting at object with constructor 'zr'
      |     property 'client' -> object with constructor 't'
      --- property '_fetcher' closes the circle
@mydea
Copy link
Member

mydea commented Jul 14, 2023

Hello,

thank you for writing in!

I think we may fix this in the process of #8534 - we are currently starting to work on revamping the whole underlying performance tracing functionality of the Node SDK, which will allow us to just leverage e.g. https://www.npmjs.com/package/@prisma/instrumentation, which should hopefully also work with v5.

@janpio
Copy link

janpio commented Jul 17, 2023

(Prisma here)

Prisma 5.0.0 has not fundamentally changed anything, beyond that we adapted the internal protocol that Prisma Client talks to the internal Query Engine. Before it was GraphQL-like, now it is a custom Json. (This actually existed since Prisma 4.11.0 with the jsonProtocol preview feature, but was made the default with the major release.)

Not sure if this is what causing the problem here though. The error above seems to be from a JSON.stringify which I can only find one in https://github.com/getsentry/sentry-javascript/blob/564af01aeb60706dce0694c662df8a34932c4176/packages/tracing-internal/src/node/integrations/prisma.ts#L86C88-L86C102 - is that the one causing the problem? If so, why doesn't it like the Client any more?

If someone creates a small reproduction example of a working Prisma integration with Prisma 4.16.2, I am happy to take a look at what could be going on when upgrading to Prisma 5.0.0 (or enabling the preview feature with 4.16.2 itself).

@tylerexpa
Copy link

tylerexpa commented Jul 21, 2023

I'm also seeing this with Prisma 4.14.1 and @sentry/nextjs 7.59.3

@janpio
Copy link

janpio commented Jul 21, 2023

Do you have previewFeatures = ["jsonProtocol"] enabled @tylerexpa or not?

@tylerexpa
Copy link

@janpio I have previewFeatures = ["fullTextSearch", "postgresqlExtensions"] set

@janpio
Copy link

janpio commented Jul 22, 2023

That would go against my theory above.

Still would appreciate a minimal reproduction of the problem so I can play with it. I know nothing about Sentry, so creating one will not be trivial for me.

@FaresKi
Copy link

FaresKi commented Jul 28, 2023

Hi!
Sorry to add salt to the wound but I'm running into similar issues with Next.js.
I'm trying to follow Prisma best practices when it comes to Prisma instantiation as stated here:

import { PrismaClient } from '@prisma/client'

const globalForPrisma = globalThis as unknown as {
  prisma: PrismaClient | undefined
}

export const prisma = globalForPrisma.prisma ?? new PrismaClient()

if (process.env.NODE_ENV !== 'production') globalForPrisma.prisma = prisma

I've tried combining that with Sentry's integration, so it would look something like this:

import { PrismaClient } from '@prisma/client'
import * as Sentry from '@sentry/node'

const globalForPrisma = globalThis as unknown as {
	prisma: PrismaClient | undefined
}

const prisma = globalForPrisma.prisma ?? new PrismaClient()

if (process.env.NODE_ENV !== 'production') globalForPrisma.prisma = prisma

export default prisma

Sentry.init({
	dsn: 'my-sentry-dsn',
	release: '1.0',
	tracesSampleRate: 1.0,
	integrations: [new Sentry.Integrations.Prisma({ client: prisma })],
})

Unfortunately, it made me run into the same issues as stated above, ie the circular error.
Looking forward to a solution!
Thanks for your help :)

@janpio
Copy link

janpio commented Jul 30, 2023

Thanks. Can you @FaresKi maybe put a minimal Next.js project with just this needed code in a Github repo? As I said before, happy to try this out - but no experience with Sentry, so hesitant to set something up myself. Thanks.

@FaresKi
Copy link

FaresKi commented Jul 30, 2023

Thanks. Can you @FaresKi maybe put a minimal Next.js project with just this needed code in a Github repo? As I said before, happy to try this out - but no experience with Sentry, so hesitant to set something up myself. Thanks.

Glad to help!
How do you want to proceed ?
I provide the code and you configure Sentry via the CLI? They have a great integration process.
Unfortunately I can't provide you access to my Sentry.

@janpio
Copy link

janpio commented Jul 31, 2023

Yes, I have an account available - might just need some pointers to know what to do exactly. So some code, and some light instructions should be enough for me to figure out the rest.

@FaresKi
Copy link

FaresKi commented Aug 2, 2023

While I was preparing the reproducible, I've noticed the problem being solved when upgrading Prisma to the latest release (same as Sentry).
@mwillbanks, maybe you should do the same?

@FaresKi
Copy link

FaresKi commented Aug 5, 2023

Hi again guys!
I come with good & bad news.
Bad news: the upgrade actually didn't fix anything so, I was a bit bummed but determined to understand where the problem came from.

I realized in my project that I had a double initialisation of Sentry! One without the integration and one with!
I ended up removing the former and moving the initialisation of sentry + prisma integration into the sentry server config.

Multiple instances and/or incorrect initialisation localisation may cause unexpected JSON stringifications. Just something to keep in mind.

The good news is that it now definitely works.

lforst added a commit that referenced this issue Aug 7, 2023
`JSON.stringify` will throw when called on circular structures. This is an unnecessary risk in the prisma integration so we just log it plainly.

Ref: #8532
@lforst
Copy link
Member

lforst commented Aug 7, 2023

I am gonna remove the JSON.stringify call. It's unnecessarily dangerous in this context since it will be evaluated before we even decide if we wanna log or not - not to mention the performance impact.

#8745

@recurrence
Copy link

lforst 's fix resolves this issue for me (the other ideas did not).

@tyteen4a03
Copy link

Any idea when this will be released?

@lforst
Copy link
Member

lforst commented Aug 9, 2023

@tyteen4a03 I'd assume in the upcoming days.

@lforst
Copy link
Member

lforst commented Aug 17, 2023

Fix has been released.

@lforst lforst closed this as completed Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests