Outdated `@sentry/vite-plugin` used in SvelteKit package

benmccann commented 1 year 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/sveltekit

SDK Version

^7

Framework Version

SvelteKit 1.20.0

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

From https://github.com/sveltejs/svelte/issues/8682:

git clone git@github.com:cesarnml/portfolio-sveltekit.git
cd portfolio-sveltekit
pnpm i -D svelte@next

Expected Result

The latest version should be used here: https://github.com/getsentry/sentry-javascript/blob/9ed5cfdefeb012c6523ca04d0ea697d1829fc004/packages/sveltekit/package.json#L30

It's reported to fix the issue: https://github.com/sveltejs/svelte/issues/8682#issuecomment-1572211488

Actual Result

➜  portfolio-sveltekit git:(cesar/sc-251_update-all-portfolio-dependencies) ✗ pn i -D svelte@next              
 WARN  deprecated intl-messageformat-parser@3.6.4: We've written a new parser that's 6x faster and is backwards compatible. Please use @formatjs/icu-messageformat-parser
 WARN  deprecated @formatjs/intl-unified-numberformat@3.3.7: We have renamed the package to @formatjs/intl-numberformat
 WARN  deprecated @formatjs/intl-utils@2.3.0: the package is rather renamed to @formatjs/ecma-abstract with some changes in functionality (primarily selectUnit is removed and we don't plan to make any further changes to this package
Packages: +22 -16
++++++++++++++++++++++----------------
Progress: resolved 916, reused 895, downloaded 0, added 0, done
node_modules/.pnpm/@sveltejs+kit@1.20.0_svelte@4.0.0-next.0_vite@4.3.9/node_modules/@sveltejs/kit: Running postinstall script, failed in 773ms
.../node_modules/@sveltejs/kit postinstall$ node postinstall.js
│ Error while trying to sync SvelteKit config
│ Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in /Users/cesar/Documents/Code/LearnCode/Svelte/portfolio-sveltekit/node_modules/.pnpm/@sentry+svelte@
│     at new NodeError (node:internal/errors:399:5)
│     at exportsNotFound (node:internal/modules/esm/resolve:361:10)
│     at packageExportsResolve (node:internal/modules/esm/resolve:641:13)
│     at resolveExports (node:internal/modules/cjs/loader:565:36)
│     at Module._findPath (node:internal/modules/cjs/loader:634:31)
│     at Module._resolveFilename (node:internal/modules/cjs/loader:1061:27)
│     at Module._load (node:internal/modules/cjs/loader:920:27)
│     at Module.require (node:internal/modules/cjs/loader:1141:19)
│     at require (node:internal/modules/cjs/helpers:110:18)
│     at Object.<anonymous> (/Users/cesar/Documents/Code/LearnCode/Svelte/portfolio-sveltekit/node_modules/.pnpm/@sentry+svelte@7.53.1_svelte@4.0.0-next.0/node_modules/@
│ node:internal/process/promises:288
│             triggerUncaughtException(err, true /* fromPromise */);
│             ^
│ Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in /Users/cesar/Documents/Code/LearnCode/Svelte/portfolio-sveltekit/node_modules/.pnpm/@sentry+svelte@
│     at new NodeError (node:internal/errors:399:5)
│     at exportsNotFound (node:internal/modules/esm/resolve:361:10)
│     at packageExportsResolve (node:internal/modules/esm/resolve:641:13)
│     at resolveExports (node:internal/modules/cjs/loader:565:36)
│     at Module._findPath (node:internal/modules/cjs/loader:634:31)
│     at Module._resolveFilename (node:internal/modules/cjs/loader:1061:27)
│     at Module._load (node:internal/modules/cjs/loader:920:27)
│     at Module.require (node:internal/modules/cjs/loader:1141:19)
│     at require (node:internal/modules/cjs/helpers:110:18)
│     at Object.<anonymous> (/Users/cesar/Documents/Code/LearnCode/Svelte/portfolio-sveltekit/node_modules/.pnpm/@sentry+svelte@7.53.1_svelte@4.0.0-next.0/node_modules/@
│   code: 'ERR_PACKAGE_PATH_NOT_EXPORTED'
│ }
│ Node.js v18.16.0
└─ Failed in 773ms at /Users/cesar/Documents/Code/LearnCode/Svelte/portfolio-sveltekit/node_modules/.pnpm/@sveltejs+kit@1.20.0_svelte@4.0.0-next.0_vite@4.3.9/node_modules/@sveltejs/kit
 ELIFECYCLE  Command failed with exit code 1.

Lms24 commented 1 year ago

Hi @benmccann thanks for opening this issue,

I took a quick look at the repo from @cesarnml you mentioned and I found these Sentry dependencies registered as devDependencies. There are a couple of issues:

Sentry SDK packages should be installed as dependencies. We've had multiple reports already saying that the SDK causes build errors if it's installed as a dev dependency but it works when it's a dependency. I suppose this has something to do with how Vite handles dev and application dependencies. Also maybe with the fact that we have to retain CJS support in our packages.
The repo uses the Node and the Svelte SDK. We recently released a dedicated SvelteKit SDK which adds quite a few things around these two SDKs so that Sentry works correctly with SvelteKit. I'd recommend removing the three Sentry packages and switching to this SDK. The SvelteKit SDK also includes the Sentry Vite plugin but it adds a lot of custom logic around it.

I played around with Svelte 4 today and for me, a simple SvelteKit+Svelte4 app works well with our SvelteKit SDK. Here's my test repo: https://github.com/Lms24/sentry-sveltekit-svelte4. There could definitely be problems with Svelte 4 but at least my simple example worked.

Let us know if this solves the problems. I'm ooo for the next two weeks, so someone else from our team will pick this up if there's still something to fix. Cheers!

benmccann commented 1 year ago

Yeah, I'll trust that project in the issue that was filed was not setup correctly and that it could be improved thus fixing the issue.

It might still be worth leaving this open to track upgrading to the latest @sentry/vite-plugin in @sentry/sveltekit though. It seems surprising to me that it would be using an older version

cesarnml commented 1 year ago

Just chiming in here to confirm that upgrading to the Sentry's SvelteKit SDK worked with both svelte3 and svelte4 in the repo that originally sparked this discussion.

Thank you both for looking into this issue.

lforst commented 1 year ago

@benmccann We might leave this open to track upgrading the vite plugin, however bumping to v2 will likely require a major bump of the SDK so it might take a while.

Please note that the jump from 0.6.0 to 2.2.0 looks way larger than it is. We just skipped version 1 because of technicalities.

getsentry / sentry-javascript