Service Worker "import sentry-release-injection" error when sourcemapping for Sentry (Vite plugin)

ArnauKokoro commented 8 months ago

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 SDK are you using?

@sentry/vue

SDK Version

7.91.0

Framework Version

Link to Sentry event

No response

SDK Setup

sentryVitePlugin({
        org: 'xxxxx',
        project: 'xxxxxx',
        release: {
          name: 'xxxxxx'
        }
}),
VitePWA({
        registerType: 'prompt',
         devOptions: {
            enabled: true
          },
          workbox: {
            globPatterns: ['**/*.{js,css,html,ico,png,jpg,jpeg,svg}']
          },
          manifest: {}
  })
})

Steps to Reproduce

Add sentryVitePlugin and VitePWA plugins on vite.config
yarn dev

Expected Result

Not having an import sourcemap error

Actual Result

Console: Uncaught SyntaxError: Cannot use import statement outside a module (at dev-sw.js?dev-sw:99:2)

File error:

workbox.precache({})
//# sourceMappingURL=sw.js.map

;import "/@id/__x00__sentry-release-injection-file";
//# sourceMappingURL=xxx

lforst commented 8 months ago

Hi, this is probably a bug somewhere in workbox or VitePWA, would you mind opening an issue in their respective repositories?

You can point them to this comment. My suspicion here is that they are trying to resolve or rewrite the virtual module which they should generally not do since it starts with a null bye \0. This is a convention in rollup/vite that everybody building plugins should adhere to.

Ideally you also share your build logs in case there are any warnings or similar!

FlyingRadish commented 7 months ago

I also encountered this problem and here is a simple reproduction:

create an vue3 project and set up sentry vite plugin
create env.js file in project dir, with contentwindow.appEnv={apiHost: "http://localhost:8100"}

reference this js file in head of index.html

<head>
...
<script type="text/javascript">
document.write("<script src='/env.js?v=" + new Date().getTime() + "'><\/script>")
</script>
</head>

npm run dev and check the browser, BANG!

lforst commented 7 months ago

@houxg What do you mean exactly by create a vue3 project? Would you mind giving a bit more concrete instructions? Then we can debug more effectively. Thank you!

FlyingRadish commented 7 months ago

@houxg What do you mean exactly by create a vue3 project? Would you mind giving a bit more concrete instructions? Then we can debug more effectively. Thank you!

create a simple vue3 project by vite, and setup sentry-vite-plugin.

MaxWeisen commented 4 months ago

I am also experiencing this same error. Here are the packages and versions I am working with:

nuxt: 3.10.3
vue: 3.4.19
@vite-pwa/nuxt: 0.6.0
@sentry/vue: 5.110.0
@sentry/vite-plugin: 2.16.1

Any tips on how to fix this or is Sentry and vite-pwa not able to work together at this time?

lforst commented 4 months ago

I haven't tried out vite-pwa in combination with the plugin. If someone provides a minimal reproduction example I can take a look. Otherwise we will look at this when we have some spare cycles which may take a while.

MaxWeisen commented 4 months ago

@lforst Here is a minimal reproduction. I have some instructions on what I did to reproduce the issue in the readme.

https://github.com/MaxWeisen/vite-pwa-sentry-plugin-issue

Thank you!

userquin commented 4 months ago

This is a problem in both, workbox-build and sentry when using generateSW strategy:

you cannot use type module in non chromium based browsers when registering the sw
generateSW in workbox-build node module will use importScripts when generating the service worker

We cannot use vite-pwa-plugin and sentry in dev server even when registering the service worker with type module.

The workaround is using a custom service worker: in this case, vite-plugin-pwa will use custom Vite build to build the service worker: in dev server, Vite will just remove ts annotations; in build mode, we don't have any problem since vite-plugin-pwa will not use the plugins in the main build and so sentry will not be in the sw build (you can include sentry plugin in the custom build, Vite will inline sentry import).

Check from this comment and next ones: https://github.com/vite-pwa/nuxt/issues/132#issuecomment-2079951086.

Here a working example in dev server and build : https://github.com/userquin/vite-pwa-sentry-plugin-issue-120/tree/inject-manifest (inject-manifest branch)

mydea commented 4 months ago

I am not an expert in this overall, but could a solution also be to just not run the sentryVitePlugin in dev mode at all? IMHO you probably don't need it in dev mode?

userquin commented 4 months ago

@mydea the problem is about testing the sw and the pwa web manifest, sentry should have a way to exclude some assets, rn any asset with js/ts/jsx/tsx/mjs extension being intercepted by the sentry plugin (transform hook), the pwa plugin serving dev sw using dev-sw.js?dev-sw:

EDIT: or just exclude the sentry plugin in dev, it is easy, just adding apply: 'build' when exposing the unplugin plugin for vite.

mydea commented 4 months ago

Hmm, I guess we could add some kind of ignore config to skip certain file globs - would that fix your problem? 🤔

userquin commented 4 months ago

EDIT: or just exclude the sentry plugin in dev, it is easy, just adding apply: 'build' when exposing the unplugin plugin for vite.

mydea commented 4 months ago

I think we are unlikely to do this in the plugi

EDIT: or just exclude the sentry plugin in dev, it is easy, just adding apply: 'build' when exposing the unplugin plugin for vite.

I think we are unlikely to do that, if you want this you can simply skip adding the plugin in dev yourself, I guess? Or am I missing something there?

I think having an option to ignore certain files for sourcemaps injection is not unreasonable 🤔 I'll wait for @Lms24 & @lforst to chime in as they know more about the deeper internals of the bundler plugins, though.

lforst commented 4 months ago

@userquin I still firmly believe this is an issue with workbox or VitePWA. I think they are externalizing our injected module, which I don't think is correct of them to do. This requires an upstream fix. Would you mind opening an issue in their repositories? Feel free to ping us there and we can provide more context and info to the maintainers.

userquin commented 4 months ago

@lforst workbox-build::generateSW will build the sw using importScripts and we cannot use ESM import syntax when registering the sw using type: 'classic' (rn only chromium based browsers support type: 'module' sw registration).

We cannot also mix importScripts and ESM imports: I tried registering the sw using type: 'module' and Chrome/Chromium/Edge throwing error.

This is a limitation on how sw can be registered: rn the workaround is changing the strategy and use custom sw.

Since sentry is adding that import to any asset with js/ts/jsx/tsx/mjs extensions we can only remove that import once build and before returning the content: the fix will not be quite easy since we need to register another plugin for final transformation after sentry plugin (sentry adding the import after pwa plugin building the sw).

lforst commented 4 months ago

If the Sentry vite plugin causes issues you could always use Sentry CLI to inject debug IDs and upload sourcemaps: https://docs.sentry.io/platforms/javascript/sourcemaps/uploading/cli/

Currently, resolving this issue is not a priority for us, since Sentry CLI should work fine as a workaround. If you are down for it you could open a PR with a fix (if you have one in mind). Otherwise, I will backlog this for now.

acaldas commented 2 weeks ago

I'm using @sentry/vite-plugin and it was adding this import to my service worker. Fixed it by setting release.inject to false. Will this cause any issues in the error reporting?

I noticed that all it does is set SENTRY_RELEASE in the window object, I do it in my app code instead.

Would be nice to have an option to configure how the import is injected.

Lms24 commented 1 week ago

Hi, if you set release in your Sentry.init anyway, injecting the release is 1) not necessary and 2) has no effect since the release option takes precedence.

So,

Will this cause any issues in the error reporting?

Nope, all good :)

getsentry / sentry-javascript-bundler-plugins