getsentry / sentry-javascript

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

[nextjs] Improve webpack plugin enablement and warnings #5619

Closed lobsterkatie closed 1 year ago

lobsterkatie commented 1 year ago

Current as of version 7.11.1

This is a continuation of the conversation started in https://github.com/getsentry/sentry/pull/37649. TL;DR: If, in the nextjs SDK, we enable the webpack plugin in a situation where the user is unlikely to have set up sentry-cli, we're setting them up for errors.

That issue was specifically about the preview and development environments on Vercel, and that case has been handled in https://github.com/getsentry/sentry-javascript/pull/5603, which will be released in version 7.12. As discussed in the original PR, though, there's more that could be done, which I'm recording here so that the PR can be closed. Specifically:

Merott commented 1 year ago

Needing some clarification regarding the remaining issues.

Are releases currently being generated for Vercel preview deployments using the official integration?

In Sentry, I see releases for preview deployments, albeit without source maps. I expected not to see those releases at all. Is this normal, a known issue within the scope of this one, or potentially a misconfiguration on my end?

(I'm sorry if this is unrelated. I'll open a separate discussion if that's the case.)

CleanShot 2022-12-12 at 23 05 06@2x

CleanShot 2022-12-12 at 23 04 51@2x
IGassmann commented 1 year ago

Needing some clarification regarding the remaining issues.

Are releases currently being generated for Vercel preview deployments using the official integration?

In Sentry, I see releases for preview deployments, albeit without source maps. I expected not to see those releases at all. Is this normal, a known issue within the scope of this one, or potentially a misconfiguration on my end?

(I'm sorry if this is unrelated. I'll open a separate discussion if that's the case.)

CleanShot 2022-12-12 at 23 05 06@2x

CleanShot 2022-12-12 at 23 04 51@2x

I think those releases might be auto-created by events tagged with the release value. @lobsterkatie could that be?

lforst commented 1 year ago

I think those releases might be auto-created by events tagged with the release value. @lobsterkatie could that be?

Yup! That can happen.

alvarlagerlof commented 1 year ago

I think those releases might be auto-created by events tagged with the release value. @lobsterkatie could that be?

Yup! That can happen.

This has confused me so much. For months I've been trying to understand why there are lots of issues in releases without source maps. Sometimes there's a release for a commit, sometimes there's none.

I've been trying to debug the Sentry setup using preview environments, which is how we change everything else in Vercel in my org. This seems to have been a non-working approach all along, since source maps are apparently didabled for previews. How can I enable them? I don't like to test config changes in production.

lforst commented 1 year ago

@alvarlagerlof what version of the SDK are you on? We changed this behaviour to auto-upload in dev and preview envs not long ago because it was indeed confusing. See https://github.com/getsentry/sentry-javascript/pull/7436. The earliest version that has this change is 7.43.0. So you probably just need to update the SDK.


Some trivia: We're currently investing a lot of time into a process that will allow us to unminify stack traces regardless of releases:

The change in Sentry is already live, we just need to update the consuming tooling to also use this new process. In the case of the Next.js SDK this requires our webpack plugin to be updated first. You can check progress over at https://github.com/getsentry/sentry-javascript-bundler-plugins.

lforst commented 1 year ago

Let me know if upgrading resolves this issue for you. I might close this issue because we kinda addressed it in #7436 and #7434.

alvarlagerlof commented 1 year ago

@alvarlagerlof what version of the SDK are you on? We changed this behaviour to auto-upload in dev and preview envs not long ago because it was indeed confusing.

No sorry, that is the opposite of what I meant. I think that auto.uploads in previews is to be expected. What I've come to expect from Vercel, and how everything I run on there works, is that Preview is s replica of production. It's the same, except for a different set of environment variables to the extend that you change them. Not much at all should differ from production unless you specify so. This is a really good thing, and is why I use Vercel to begin with.

This is also what caught me. Everyone in my org is in the habit of testing new integrations/tools/libraries using preview deploys. We never test in production, because that is risky in comparison. This includes config setup and changes. Now our Sentry setup was already misconfigured (separate issue, our fault). But changing it is something I thought I could test using preview environments. Turns out I never could. The fact that this wasn't documented anywhere is unfortunate.

My expectation has always been that preview is extremely close to production. Local dev is something else, and yes, I agree there probably shouldn't be any uploaded source maps there. But I don't want issues from local dev either.

See #7436.

Now I'm confused, that PR looks like you are uploading source maps for preview environments currently.

The earliest version that has this change is 7.43.0. So you probably just need to update the SDK.

We use a bot called Renovate, so the packages are very up to date. The current installed version appears to be 7.48.0.

Some trivia: We're currently investing a lot of time into a process that will allow us to unminify stack traces regardless of releases:

The change in Sentry is already live, we just need to update the consuming tooling to also use this new process. In the case of the Next.js SDK this requires our webpack plugin to be updated first. You can check progress over at https://github.com/getsentry/sentry-javascript-bundler-plugins.

This is great to hear. Sounds like what I want.


Let me know if upgrading resolves this issue for you. I might close this issue because we kinda addressed it in https://github.com/getsentry/sentry-javascript/pull/7436 and https://github.com/getsentry/sentry-javascript/pull/7434.

Resolved for me would be being able to debug and fix my setup in preview, not production. Is that currently possible or not?

This is what I get most of the time on Vercel preview deploys.

error - No Sentry auth token configured. Source maps will not be uploaded.

To reproduce that, all you need to do is npx create-next-app@latest, npx @sentry/wizard -s -i nextjs and add the Vercel integration in Sentry.

lforst commented 1 year ago

@alvarlagerlof what version of the SDK are you on? We changed this behaviour to auto-upload in dev and preview envs not long ago because it was indeed confusing.

No sorry, that is the opposite of what I meant.

Sorry, I might have fucked up this sentence 🤣 What I meant is that we changed the SDK from not-uploading to uploading sourcemaps for these environments.

We use a bot called Renovate, so the packages are very up to date. The current installed version appears to be 7.48.0.

Good to hear that you are on the latest version, stuff shouldn't be too hard to fix then.


I think I found the culprit. While we changed the behaviour of our SDK to upload for development and preview environments we didn't update our Vercel integration to set the required environment variables for these deployment targets. I opened a PR to fix this: https://github.com/getsentry/sentry/pull/47441

When this PR is merged and released, I think we can resolve your problem by simply reinstalling the Sentry integration on Vercel.

If you do not want to wait until then, you can populate the SENTRY_AUTH_TOKEN environment variable in your Vercel project with an auth token: https://docs.sentry.io/api/auth/

Thanks for bearing with me and thoroughly explaining the problem.

alvarlagerlof commented 1 year ago
image

Yeah I was just about to post this as well :D. I'm going to widen these manually and see if that works.

alvarlagerlof commented 1 year ago

It uploads source maps for releases in preview correctly now!

But new issues are not correctly attributed to those releases. It's stuck on an old release on main from several commits ago. I keep making new commits on a branch but it's still that one commit from main.

alvarlagerlof commented 1 year ago
image

To demonstrate, I've made several commits (more than 3) between these errors. Seems like this might be this issue, which also looks unresolved.

lforst commented 1 year ago

@alvarlagerlof yeah that one is on my radar but I haven't found a solid fix yet. We somehow need to prevent Vercel from caching the release value. Lemme look into this. Maybe I have an epiphany.

alvarlagerlof commented 1 year ago

@alvarlagerlof yeah that one is on my radar but I haven't found a solid fix yet. We somehow need to prevent Vercel from caching the release value. Lemme look into this. Maybe I have an epiphany.

See https://github.com/getsentry/sentry-javascript/issues/5734#issuecomment-1510362749.

lforst commented 1 year ago

This is deployed now. Removing and adding the Sentry integration in vercel should now set the env vars for the right deployment environments.