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

NextJS source map upload config is too narrow #3896

Closed
twavv opened this issue Aug 17, 2021 · 8 comments · Fixed by #4705
Closed

NextJS source map upload config is too narrow #3896

twavv opened this issue Aug 17, 2021 · 8 comments · Fixed by #4705
Labels

Comments

@twavv
Copy link
Contributor

twavv commented Aug 17, 2021

Package + Version

Was introduced in @sentry/[email protected], specifically 8ef39cc and these lines:
8ef39cc#diff-55396f0a8f12ba2eb7ce27f3cf886f946c29c2aa93b5d2f993dadbd5a9f3cc75R256

Downgrading to 6.10.0 works again.

Description

Sentry config only uploads .next/static/chunks/pages instead of .next/static/chunks. The latter contains all things that are not a page (i.e., anything and everything that is codesplit, which constitutes the majority of at least my personal codebase).

twavv referenced this issue Aug 17, 2021
…les (#3845)

Though sourcemap uploading using `SentryWebpackPlugin` is generally quite fast, even when there are a lot of files[1], there are situations in which it can slow things down a lot[2].

[1] #3769 (comment)
[2] vercel/next.js#26588 (comment)

There are a number of reasons for that, but one of them is that, in the current state, `@sentry/nextjs`'s use of the plugin is pretty indiscriminate - it just uploads anything and everything in the `.next` folder (which is where nextjs puts all of its built files), during both client and server builds. The lack of specificity leads us to upload files we don't need, and the fact that we don't distinguish between client and server builds means that whichever one happens second not only uploads its own unnecessary files, it also uploads all of the files from the other build (which have already been uploaded), both necessary and unnecessary. More details in #3769.

This change makes it so that we're much more specific in terms of what we upload, in order to fix both the specificity and the duplication problems.

Notes:

- There's discussion in the linked issue about which non-user sources/maps to upload (things like code from webpack, nextjs itself, etc). In the end, I chose to restrict it to user code (almost - see below), for two reasons. First, non-user code is unlikely to change much (or often) between releases, so if third-party code were included, we'd be uploading lots and lots of copies of the same files. Second, though it's occasionally helpful to be able to see such code in the stacktrace, the vast majority of the time the problem lies in user code. For both reasons, then, including third-party files didn't feel worth it , either in terms of time spent uploading them or space spent storing them.

  (I say it's "almost" restricted to user code because, among other files, the server build generates bundles which are labeled only by numbers (`1226.js` and such), some of which are user code and some of which are third-party code, and - in this PR at least - I didn't include a way to filter out the latter while making sure to still include the former. This is potentially still a worthwhile thing to do, but the fact that it would mean actually examining the contents of files (rather than just their paths) makes it a more complicated change, which will have to wait for a future PR.)

-  In that issue, the topic of HMR (hot module reloading) files also came up. For the moment I also chose to skip those (even though they contain user code), as a) the plugin isn't meant for use in a dev environment, where every change triggers a new build, potentially leading to hundreds of sets of release files being uploaded, and b) we'd face a similar issue of needing to examine more than just the path in order to know what to upload (to avoid uploading all previous iterations of the code at each rebuild).

- Another small issue I fixed, while I was in the relevant code, was to prevent the webpack plugin from injecting the release into bundles which don't need it (like third-party code, or any bundle with no `Sentry.init()` call). Since this lines up exactly with the files into which we need to inject `sentry.server.config.js` or `sentry.client.config.js`, I pulled it out into a function, `shouldAddSentryToEntryPoint()`.

Fixes #3769
Fixes vercel/next.js#26588
@nickbabcock
Copy link

As a new sentry user, I thought I was going crazy that I couldn't see any source maps for my next.js app. Turns out this was the issue and the solution for my statically exported site (YMMV) was to update next.config.js to include the missing chunks (an alternative to downgrading).

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

const yourNextConfig = {};
module.exports = withSentryConfig(yourNextConfig, {
  include: [
    { paths: [".next/static/chunks"], urlPrefix: "~/_next/static/chunks" },
  ],
});

@Dazix
Copy link

Dazix commented Oct 11, 2021

As a new sentry user, I thought I was going crazy that I couldn't see any source maps for my next.js app. Turns out this was the issue and the solution for my statically exported site (YMMV) was to update next.config.js to include the missing chunks (an alternative to downgrading).

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

const yourNextConfig = {};
module.exports = withSentryConfig(yourNextConfig, {
  include: [
    { paths: [".next/static/chunks"], urlPrefix: "~/_next/static/chunks" },
  ],
});

This works only if you don't use any serverside logic because this tmp fix disables upload of .next/server/* folder.

@meotimdihia
Copy link

meotimdihia commented Nov 27, 2021

please review this problem. I am getting errors like this:
msedge_2021-11-27_09-02-39

@hixus
Copy link

hixus commented Feb 7, 2022

Got hit this today. I think the @Dazix fix should be merged.

@chentsulin
Copy link

@AbhiPrasad @lobsterkatie Is there any chance to review the patch? This issue blocks the adoption of new versions.

@AbhiPrasad
Copy link
Member

Hey @chentsulin, we are discussing this internally and will provide an update soon!

@saminnet
Copy link

saminnet commented Mar 7, 2022

@AbhiPrasad Any update on this?

@lobsterkatie
Copy link
Member

Hi, all.

Sorry for the delay. I've just pushed #4705, which adds an option, documented in getsentry/sentry-docs#4827, to widen the scope of the upload.

It should be released in 6.18.3.

Cheers!

lobsterkatie added a commit that referenced this issue Mar 14, 2022
When nextjs builds an app, compiled versions of each page end up in `.next/static/chunks/pages`. In some circumstances, however, user-generated code ends up outside of `pages/`, and in those cases sourcemaps are broken because the relevant files aren't being uploaded.

This fixes that by optionally widening the scope of the upload, while excluding files known to contain only nextjs and webpack code. (Every build will generate other files which should be excluded, but there's no way to tell in advance what they're going to be called.) In order to reduce the number of irrelevant files being uploaded to Sentry, this behavior is off by default, but can be turned on with a new option `widenClientFileUpload`:

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

const moduleExports = {
  sentry: {
    widenClientFileUpload: true,
  },
};

const sentryWebpackPluginOptions = {
  // ...
};

module.exports = withSentryConfig(moduleExports, sentryWebpackPluginOptions);
```

This change is documented in getsentry/sentry-docs#4827.

Fixes #3896
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants