getsentry / sentry-javascript

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

Cannot set properties of undefined (setting '_sentryTraceData') #14571

Open Grohden opened 9 hours ago

Grohden commented 9 hours ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/nextjs

SDK Version

8.35.0

Framework Version

Next 14.2.11

Link to Sentry event

https://trusted-health.sentry.io/issues/5833907467/events/651aec0e738f4a8592762072abecec34/

Reproduction Example/SDK Setup

We've been having this issue for a while AFAIK, so here's what we've investigated

From the linked stack trace, these are the lines:

https://github.com/getsentry/sentry-javascript/blob/20c267e1eba82d0a07db6e7182e769d4d6590cc5/packages/nextjs/src/common/pages-router-instrumentation/wrapGetInitialPropsWithSentry.ts#L37-L48

If (await tracedGetInitialProps.apply(thisArg, args)) results in undefined we get a {} default

But then all these:

const {
          data: initialProps, // undefined
          baggage, // undefined
          sentryTrace, // undefined
} = {}

become undefined, so we're executing this:

        // The Next.js serializer throws on undefined values so we need to guard for it (#12102)
        if (sentryTrace) {
          undefined._sentryTraceData = sentryTrace;
        }

Which will result in those undefined access problems...

You guys also have some other places where there's similar code that might be worth checking https://github.com/search?q=repo%3Agetsentry%2Fsentry-javascript+tracedGetInitialProps.apply%28thisArg%2C+args%29%29&type=code

It also seems sentry had a similar issue in the past

Steps to Reproduce

Don't know how to simulate

Expected Result

No throw

Actual Result

Throw

Grohden commented 9 hours ago

btw I guess it just needs the proper ts cast there.. data is typed as non optional

lforst commented 7 hours ago

Thanks for writing in! So technically Next.js doesn't allow returning undefined from getInitialProps, so your app will crash regardless. I'll still add a safeguard so that people don't think it is Sentry causing issues.

Grohden commented 6 hours ago

@lforst oh well, then it should crash, but it doesn't :/, but yeah, at least not crash on sentry traces so we don't bother you all with these

Our robots.txt page work fine without returning something Image

We're adding the {} return there but all seemed to work :/