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

Respect user-provided sourcemap generation settings #13993

Open
7 tasks
Lms24 opened this issue Oct 16, 2024 · 1 comment
Open
7 tasks

Respect user-provided sourcemap generation settings #13993

Lms24 opened this issue Oct 16, 2024 · 1 comment

Comments

@Lms24
Copy link
Member

Lms24 commented Oct 16, 2024

Description

Currently, most (all?) of our meta framework SDKs enable source map generation even if users explicitly disabled sourcemap generation in their build config. While this improves the onboarding experience because Sentry "just works", it is intrusive and overrides a quite critical user decision. In most frameworks we emit build logs that we do this but it's nevertheless questionable behaviour.

Therefore, we decided to change the behaviour with the next upcoming major for all stable meta framework SDKs:

  • If users explicitly disabled sourcemaps (e.g. build.sourcemaps: false in Vite), we respect this and emit a warning that errors won't be unminified in Sentry. We will not upload anything.
  • If users set source map generation to any other value that effectively enables them (true, hidden, inline, etc). We respect this and don't do anything (i.e. deletion) besides uploading. Uploading will still emit build logs as today.
  • If users don't explicitly enable/disable set source maps generation, we
    • enable hidden source maps generation
    • configure filesToDeleteAfterUpload to delete all .map files
    • emit a log that this is what we're doing for transparency

For stable SDKs, this change is behaviour-breaking with significant user impact. So we will only do this in the upcoming major.

Stable SDKs

For SDKs currently in beta, we can change this right now, which is also a good opportunity to trial the new behaviour

SDKs in Beta

  1. andreiborza

Considered Trade-offs

  • If users explicitly disable source maps, errors will no longer appear unminified in Sentry (since we don't upload anything). This is a UX decrease in Sentry but it is compensated by being a good citizen on the SDK side. We will emit clear warnings that this is problematic and ask users to enable and delete or unset their source map generation settings.
  • Theoretically, other bundler plugins could modify the source map generation setting prior to our plugin coming in and checking the previously set value. We cannot detect this. For now we'll live with this trade-off and evaluate how much of a problem this actually is.
  • Some frameworks (like Nuxt) recognize during build if source maps are emitted and might change the build output (for example, list the source maps as routes in a routing manifst/table). If we delete source maps afterwards (case 3), this might cause build or runtime issues/warnings/logs. We'll need to find framework-specific solutions how to address this. For instance, we could try to modify the route manifest file and remove the entries again.
@Lms24 Lms24 added this to the 9.0.0 milestone Oct 16, 2024
s1gr1d added a commit that referenced this issue Oct 18, 2024
)

Nuxt implementation for:
#13993
Fixes: #13997

In Nuxt, there are 3 places to set source maps (and all need to be
enabled):
- `sourcemap`
- `nitro.rollupConfig.output.sourcemap`
- `vite.build.sourcemap`

As Nuxt sets `sourcemap.client: false` ([docs
here](https://nuxt.com/docs/api/nuxt-config#sourcemap)), it's not
possible to determine whether this setting was set by the user or the
framework. Users have to set this manually like this:
```js
export default defineNuxtConfig({
  sourcemap: {
    client: true,
  },
})
```

With this PR, all source maps are set to `'hidden'` if they were
undefined before and keep the setting otherwise. This is done in 3
separate functions (one for vite, rollup and nuxt) to better distinguish
between the settings.
@s1gr1d
Copy link
Member

s1gr1d commented Oct 18, 2024

Maybe something we should keep an eye on: Nuxt sets sourcemaps.client: false per default (docs here) and users have to explicitly enable this when the SDK respects a disabled sourcemap setting. We might make an exception here and always enable with "hidden".

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

No branches or pull requests

2 participants