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

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

Closed
3 tasks done
ArnauKokoro opened this issue Jan 10, 2024 · 19 comments

Comments

@ArnauKokoro
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/vue

SDK Version

7.91.0

Framework Version

Vue 3.2.0

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

  1. Add sentryVitePlugin and VitePWA plugins on vite.config
  2. 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
Copy link
Member

lforst commented Jan 10, 2024

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
Copy link

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

  1. create an vue3 project and set up sentry vite plugin
  2. create env.js file in project dir, with contentwindow.appEnv={apiHost: "http://localhost:8100"}
  3. 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>
  1. npm run dev and check the browser, BANG!

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Jan 23, 2024
@lforst
Copy link
Member

lforst commented Jan 25, 2024

@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
Copy link

@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
Copy link

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
Copy link
Member

lforst commented Apr 26, 2024

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
Copy link

@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
Copy link

userquin commented Apr 30, 2024

@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.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Apr 30, 2024
@mydea
Copy link
Member

mydea commented Apr 30, 2024

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

@userquin
Copy link

userquin commented Apr 30, 2024

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

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Apr 30, 2024
@mydea
Copy link
Member

mydea commented Apr 30, 2024

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
Copy link
Member

lforst commented May 2, 2024

@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
Copy link

userquin commented May 2, 2024

@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).

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 May 2, 2024
@lforst
Copy link
Member

lforst commented May 2, 2024

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
Copy link

acaldas commented Sep 4, 2024

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.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Sep 4, 2024
@Lms24
Copy link
Member

Lms24 commented Sep 5, 2024

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 :)

@AbhiPrasad
Copy link
Member

Closing for clean-up. Please re-open the issue if this still applies. Thanks!

@AbhiPrasad AbhiPrasad closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Archived in project
Development

No branches or pull requests

9 participants