getsentry / sentry-javascript

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

Sentry Webpack Plugin Ignores Configuration Files with "sideEffects": false #9787

Open Angelodaniel opened 7 months ago

Angelodaniel commented 7 months 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

latest

Framework Version

next latest

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

It looks like sth. related to tree shaking in Webpack (https://webpack.js.org/guides/tree-shaking/)

It can be easily reproduced using even the latest next and @sentry/nextjs versions - e.g. via project created using npx create-next-app@latest; npx @Sentry/wizard@latest -i nextjs. With such a sample project created, all works as expected.

Interesting things start to happen, when you add "sideEffects": false to the package.json: Both sentry.edge.config.(js|ts) and sentry.server.config.(js|ts) seem to be completely ignored - the content of both could be only throw new Error(); - these won't even be loaded. However, if you remove any, the Webpack plugin will complain about the missing file(s).

In my initial message I have mentioned that the API route errors are reported to Sentry - it looks like, however, it was due to the Next's Error Overlay being displayed and @sentry/browser jumping in.

I had the "sideEffects": false entry in my project's package.json for some long time already - it was actually resulting in my app client bundle files size being smaller by ~10kb due to a better tree shaking on my project's dependencies. I do not, however, see the same in the sample project created above. It's not affecting Sentry running in the browser - here all works as expected. The 'solution' to the issue is to remove "sideEffects": false from package.json, or at least change it to the following:

"sideEffects": [
  "./sentry.client.config.*s",
  "./sentry.edge.config.*s",
  "./sentry.server.config.*s"
]

I would, however, expect the Sentry Webpack plugin does it automatically (i.e. prevents these files being shaken out). Or, at least, Sentry documentation mentions this trap...

Expected Result

I would, however, expect the Sentry Webpack plugin does it automatically (i.e. prevents these files being shaken out).

Actual Result

package.json includes "sideEffects": false, the Sentry Webpack plugin appears to ignore certain configuration files (sentry.edge.config.(js|ts) and sentry.server.config.(js|ts)). Even if the content of these files is set to something simple like throw new Error();, they are not loaded, and removing any of these files results in a Webpack plugin complaint about missing files.

Additionally, the report notes that API route errors are reported to Sentry, possibly due to Next.js Error Overlay being displayed and @sentry/browser jumping in. Despite these issues, the Sentry Webpack plugin seems to work as expected in the browser, indicating a specific problem when sideEffects is set to false in the project's configuration.

┆Issue is synchronized with this Jira Improvement by Unito

lforst commented 7 months ago

I don't want to fully dismiss this but I also want to say that setting sideEffects: false is wrong when you use the Sentry SDK, as the SDK is essentially one big side-effect. The Sentry.init() calls being tree-shaken out is probably even correct behaviour when sideEffects is set to false.

I am not aware of anything we can do here on a webpack level - if anybody has suggestions, I am fully open to them. Setting sideEffects: false is not the default and setting it should be thoroughly thought through. IMO it is unusual to set it inside an app that is not consumed by anything else and I wouldn't recommend it because it can introduce weird behaviours (like this one).

If anything, we could add docs for this.

jpulec commented 6 months ago

I'd just like to add that I lost days of my life trying to figure out why our app wasn't reporting to Sentry, but only in the server and edge runtimes, until I found this issue. Maybe add some docs around this to the troubleshooting section?

lforst commented 6 months ago

@jpulec Fair suggestion. Where would you have put the docs exactly so you would have seen it? Also, if you don't mind me asking, why did you add sideEffect: false in the first place?

jpulec commented 6 months ago

The first place I went looking when I was having problems was the Troubleshooting Section here. I was hoping that this might have been some sort of more common case, since it seemed so obviously broken to me.

Re: sideEffects, I have it set to sideEffects: ['*.css', '*.scss'], This is done to prune files from our bundle if they are unused. We use a lot of barrel files, which means that without this, we'd basically include all source files, even if they are unused, since they're probably imported somewhere in our dependency graph. Probably not an ideal project structure, but it's where we are today.

lforst commented 6 months ago

@jpulec Thank you. I'll make sure it will get added to the docs after our Next.js migration this week!