sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

Sentry sending the wrong release when capturing an exception

Open jonathanhuang13 opened this issue 3 years ago • 7 comments

Is there an existing issue for this?

  • [X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
  • [X] I have reviewed the documentation https://docs.sentry.io/
  • [X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/nextjs

SDK Version

7.11.1

Framework Version

Next 12.2.5

Link to Sentry event

https://sentry.io/organizations/maven-com/issues/3576614941/events/c36e908b7bb1426b8fd78b96f8659d71/?environment=production&project=5632569&statsPeriod=14d

Steps to Reproduce

  1. Deploy to Vercel, which is connected to Sentry
  2. Verify that there is a release in Sentry with the commit SHA of what was just deployed
  3. Go to deployed site
  4. Trigger an error
  5. See in Sentry what release that error logs

Expected Result

The release should be the newest commit SHA

Actual Result

The release is stuck on some old commit SHA. As you can see in the screenshot, we have a page that just shows the commit SHA of what's deployed. We expect that SHA to be the same as the release in the network request but it's not. In this particular case, we've deployed to production 4 times between release 8ff5 and 52ee meaning Sentry isn't just stuck on the last release.

CleanShot 2022-09-12 at 12 48 34@2x

We've also verified that the 52ee release was indeed released on Sentry.

What I think is the error

I think Next.js's build cache (webpack under-the-hood) is causing the error. When I redeploy in Vercel without using the cache, the Sentry release updates. I think my configs are setup correctly though:

sentry.client.config

// This file configures the initialization of Sentry on the browser.
// The config you add here will be used whenever a page is visited.
// https://docs.sentry.io/platforms/javascript/guides/nextjs/
import * as Sentry from '@sentry/nextjs';

const SENTRY_DSN = process.env.SENTRY_DSN || process.env.NEXT_PUBLIC_SENTRY_DSN;

Sentry.init({
  enabled: process.env.NODE_ENV === 'production',
  environment: process.env.NEXT_PUBLIC_ENV === 'PROD' ? 'production' : 'development',
  dsn: SENTRY_DSN,
  // Adjust this value in production, or use tracesSampler for greater control
  tracesSampleRate: 1.0,
  // ...
  // Note: if you want to override the automatic release value, do not set a
  // `release` value here - use the environment variable `SENTRY_RELEASE`, so
  // that it will also get attached to your source maps
});

next.config.js

const { withSentryConfig } = require('@sentry/nextjs');

const sentryWebpackPluginOptions = {
  // Additional config options for the Sentry Webpack plugin. Keep in mind that
  // the following options are set automatically, and overriding them is not
  // recommended:
  //   release, url, org, project, authToken, configFile, stripPrefix,
  //   urlPrefix, include, ignore

  // dryRun is true only when we are in the test or dev env
  dryRun: process.env.NODE_ENV === 'development' || (process.env.NEXT_PUBLIC_ENV ?? '').toLowerCase() === 'test',

  silent: true, // Suppresses all logs
  // For all available options, see:
  // https://github.com/getsentry/sentry-webpack-plugin#options.
};


const moduleExports = {
  sentry: {
    hideSourceMaps: true,
  },
  webpack: (config, { webpack }) => {
    config.plugins.push(
      new webpack.DefinePlugin({
        // Tree shake Sentry, taken from https://docs.sentry.io/platforms/javascript/configuration/tree-shaking/
        __SENTRY_DEBUG__: false,
        __SENTRY_TRACING__: false,
      }),
    );

    // return the modified config
    return config;
  },
  ...
}

module.exports = withSentryConfig(moduleExports, sentryWebpackPluginOptions);

This might be related to https://github.com/getsentry/sentry-javascript/issues/4373

jonathanhuang13 avatar Sep 13 '22 15:09 jonathanhuang13

Hi, Thanks for writing in and providing such a detailed issue description!

It might very well be that Vercel is caching the files where we inject the release. Given that I have only little experience with Vercel, this is the first time I've actually noticed that they're caching stuff (so here are some links for posterity: 1, 2).

Do you have any experience around the Vercel build cache and have suggestions on how we could go about fixing this?

lforst avatar Sep 14 '22 07:09 lforst

Hi @lforst, thanks for the response. Here's one more link that Next.js provides (although it's not very detailed). I'm also not very familiar with Vercel build cache, but I think it uses Webpack 5 under-the-hood so I wonder if a Webpack config could fix the issue

jonathanhuang13 avatar Sep 14 '22 17:09 jonathanhuang13

I wonder if there's a way to selectively bust the cache for the files where we are injecting the release - but I think asking the nextjs folks will help point you in the right direction.

One thing that might work is to explicitly set the release everywhere. So you would take and use the VERCEL_GITHUB_COMMIT_SHA everywhere, instead of making the SDK and webpack plugin infer it.

AbhiPrasad avatar Sep 15 '22 08:09 AbhiPrasad

Okay I started a discussion here in Next.js.

I just tried setting the SENTRY_RELEASE but had no luck. Just to confirm, I only set the environment variable at build time like so:

  • vercel --build-env SENTRY_RELEASE=foo - Vercel uses the Next.js cache and the result is that Sentry uses the old release value
  • vercel --build-env SENTRY_RELEASE=foo --force - Vercel doesn't use the Next.js cache (because of --force) and the result is that Sentry's release value is now foo

jonathanhuang13 avatar Sep 15 '22 18:09 jonathanhuang13

One way I think we could fix this would be to change how the release is injected. Right now, @sentry/webpack-plugin creates a file which it then adds to the webpack entry property of all files specified in the plugin's include option, and it sounds like that file is what's being cached. There's no reason we couldn't just use our prefixLoader, though. All it would take is grabbing the boilerplate which gets injected into that file and adding it to our template instead.

lobsterkatie avatar Sep 16 '22 01:09 lobsterkatie

Thanks for the response @lobsterkatie. Can you say more though? I'm not sure if I fully understand. What would my webpack config look like now?

jonathanhuang13 avatar Sep 16 '22 17:09 jonathanhuang13

Thanks for the response @lobsterkatie. Can you say more though? I'm not sure if I fully understand. What would my webpack config look like now?

There wouldn't be any change on your end. The question is, how are we internally using the webpack plugin? And should we make the change just in the nextjs SDK, or should we do it directly in the webpack plugin?

I'm going to add this to our backlog, and chat with the team about the best way forward.

UPDATE: We decided to just fix this in nextjs to start, as a proof of concept, and then if it works, we'll think about porting the change over to the webpack plugin. (Since other kinds of apps besides just nextjs apps can be deployed on Vercel, eventually it would be good to fix this everywhere if we can.)

lobsterkatie avatar Sep 19 '22 16:09 lobsterkatie

As a workaround for Next.js, setting the release explicitly using Vercel's built-in NEXT_PUBLIC_VERCEL_GIT_COMMIT_SHA var seems to work:

Sentry.init({
  // ...
  release: process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_SHA,
});

zacharyliu avatar Sep 28 '22 21:09 zacharyliu

Reopening this because we have to revert https://github.com/getsentry/sentry-javascript/pull/6404 because it introduced a bug (https://github.com/getsentry/sentry-javascript/issues/6504).

lforst avatar Dec 13 '22 12:12 lforst

have the same issue here over at github.com/dotabod/frontend

noticed hard refreshing my cache shows the right release, at least once when i tried. not sure about other times

going to try setting it explicitly like @zacharyliu suggested

Geczy avatar Jan 26 '23 23:01 Geczy

can confirm setting the release specifically has been tracking correctly now. previously it was stuck on an old old release in sentry.io dashboard releases page (with adoption rates)

Geczy avatar Jan 27 '23 00:01 Geczy

nvm its still not setting, the latest commit is 583e7498e00b01aa31684b50f9537b4153228e23

but sentry is showing 468897b0478e50300bdd57aaaeda2ea0c61eb24b and so is the source on dotabod.com

from https://github.com/dotabod/frontend/commits/master

image image

Geczy avatar Jan 27 '23 00:01 Geczy

Same issue here. I can also confirm that redeploying on Vercel without the cache fixes the issue.

IGassmann avatar Feb 01 '23 19:02 IGassmann

Ok it seems like auto release injection is a bit inconsistent on Vercel. I will get in touch with them and ask if we can bust the cache somehow. At this point, to get around this, I would probably recommend using environment variables and explicitly setting the release in Sentry.init() and the withSentryConfig Webpack plugin options.

lforst avatar Feb 01 '23 21:02 lforst

@lforst but thats what im doing and it doesn't workaround it :(

Geczy avatar Feb 01 '23 22:02 Geczy

In the mean time, I've been disabling the build cache altogether by setting the environment variable VERCEL_FORCE_NO_BUILD_CACHE to 1. However, this naturally makes our builds considerably longer.

IGassmann avatar Feb 02 '23 06:02 IGassmann

@Geczy Okay, looking at your repo I see the issue that you're not setting the release option in the webpack plugin options with withSentryConfig. Can you try doing that?

lforst avatar Feb 02 '23 09:02 lforst

thanks ill set it ! should i remove it from the sentry {} object in module exports?

Geczy avatar Feb 02 '23 16:02 Geczy

Hi, was just looking at this. A very simple edit (mentioned above) fixes the release for Next on Vercel seemingly completely.

Sentry.init({
  // options generated by `npx @sentry/wizard -i nextjs`
  release: process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_SHA, // <- This line is new
});

I think this should either be made default when generated the sentry.*.config.js via npx @sentry/wizard -i nextjs or built-in (with an ability to still override it).

The current default behaviour when just following the Next.js guide is broken and won't be working out for your users. This issue took a lot of time to find. I shouldn't have to.

Edit: Perhaps it should be something like:

release: process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_SHA ?? "development"

alvarlagerlof avatar Apr 16 '23 12:04 alvarlagerlof

@alvarlagerlof We won't be putting Vercel specific instructions into the snippet because not everybody is deploying to vercel. Can you elaborate what exactly is broken in the Next.js guide you linked?

lforst avatar Apr 16 '23 15:04 lforst

@lforst There seems to be no mention that you need to put a release: in the config for releases to not be broken when deploying (at least on Vercel, which is what I am doing myself).

The guide doesn't have to include Vercel-specific details for everyone, but it could mention this issue somewhere, could it not? The other option that I see is if the @sentry/nextjs package infers that it needs to add a release option automatically when it finds itself running on Vercel.

Either needs to change because right now the docs is guiding people to a broken setup if they use Vercel at least.

alvarlagerlof avatar Apr 16 '23 15:04 alvarlagerlof

@alvarlagerlof I think it's just broken for Vercel. It's related to build caches. I have a PR open to fix this.

lforst avatar Apr 16 '23 15:04 lforst

@alvarlagerlof I think it's just broken for Vercel. It's related to build caches. I have a PR open to fix this.

Ah, that works 👍

Maybe that will fix the issue where some releases have like 20 commits?

alvarlagerlof avatar Apr 16 '23 15:04 alvarlagerlof

I believe this should be fixed in recent versions. Feel free to ping me if this is not the case.

lforst avatar Aug 17 '23 12:08 lforst