getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.99k stars 1.57k forks source link

Prisma Integration Incompatible with 5.0.0 #8532

Closed mwillbanks closed 1 year ago

mwillbanks commented 1 year ago

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 commented 1 year ago

Hello,

thank you for writing in!

I think we may fix this in the process of https://github.com/getsentry/sentry-javascript/issues/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 commented 1 year ago

(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 commented 1 year ago

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

janpio commented 1 year ago

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

tylerexpa commented 1 year ago

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

janpio commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

tyteen4a03 commented 1 year ago

Any idea when this will be released?

lforst commented 1 year ago

@tyteen4a03 I'd assume in the upcoming days.

lforst commented 1 year ago

Fix has been released.