getsentry / sentry-javascript

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

disable for sveltekit component tracking #10276

Closed JonathonRP closed 7 months ago

JonathonRP 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/sveltekit

SDK Version

latest

Framework Version

svelte 5, sveltekit 2

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

follow install guide using svelte 5 and sveltekit 2, follow guide to config component tracking for svelte run dev

Expected Result

can run dev vite server without error proper import and use should be documented, as can not find in docs or anywhere that does not result in error.

Actual Result

get error about not function if using @sentry/sveltekit, get error about cjs if using @sentry/svelte, all attempts to import results in error.

AbhiPrasad commented 7 months ago

Hi @JonathonRP - could you share your SDK setup (what you configured to add Sentry), and copy paste the error logs for us to look at? Otherwise this is hard for us to start investigating.

JonathonRP commented 7 months ago

@AbhiPrasad error logs -

Error [ERR_REQUIRE_ESM]: require() of ES Module /workspaces/Finanzen/node_modules/.pnpm/svelte@5.0.0-next.36/node_modules/svelte/src/main/main-server.js from /workspaces/Finanzen/node_modules/.pnpm/@sentry+svelte@7.94.1_svelte@5.0.0-next.36/node_modules/@sentry/svelte/cjs/performance.js not supported.
Instead change the require of main-server.js in /workspaces/Finanzen/node_modules/.pnpm/@sentry+svelte@7.94.1_svelte@5.0.0-next.36/node_modules/@sentry/svelte/cjs/performance.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/workspaces/Finanzen/node_modules/.pnpm/@sentry+svelte@7.94.1_svelte@5.0.0-next.36/node_modules/@sentry/svelte/cjs/performance.js:4:16)
    at Object.<anonymous> (/workspaces/Finanzen/node_modules/.pnpm/@sentry+svelte@7.94.1_svelte@5.0.0-next.36/node_modules/@sentry/svelte/cjs/index.js:6:21)
TypeError: withSentryConfig is not a function
    at file:///workspaces/Finanzen/svelte.config.js?ts=1705683833309:35:16
error when starting dev server:
file:///workspaces/Finanzen/svelte.config.js?ts=1705683711184:3
import { withSentryConfig } from '@sentry/sveltekit';
         ^^^^^^^^^^^^^^^^
SyntaxError: Named export 'withSentryConfig' not found. The requested module '@sentry/sveltekit' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@sentry/sveltekit';
const { withSentryConfig } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:190:5)
// hooks.client.ts
Sentry.init({
    dsn: "",
    tracesSampleRate: 1,
    replaysSessionSampleRate: 0.1,
    replaysOnErrorSampleRate: 1,
    integrations: [new Sentry.Replay(), new Sentry.BrowserTracing(), new Sentry.BrowserProfilingIntegration()]
})
// hooks.server.ts
Sentry.init({
    dsn: "",
    tracesSampleRate: 1.0,
    spotlight: dev
})
// config
sentrySvelteKit({
  sourceMapsUploadOptions: {
      org: 'self-qw6',
      project: 'finanzen',
  }
})
Lms24 commented 7 months ago

Hi @JonathonRP thanks for writing in and apologies, this issue slipped through my radar.

So to confirm, the errors you're getting only appear when you add the withSentryConfig function to your svelte config? Otherwise, using the SDK works for you?

I must admit, we kinda neglected this feature in combination with SvelteKit (we added it to our browser-only Svelte SDK). So chances are high this doesn't work correctly. Ideally, we rid of the withSentryConfig wrapper because it's not really a pattern I like using anymore in our Svelte SDKs. Furthermore, Svelte 4 contains some changes to preprocessor logic and I'm not yet sure how Svelte 5 is going to work with component tracking at all. I suspect it should work but didn't try it yet.

So I think we should revisit this in our major version and see if we can make this feature work in a better way. Can't promise you an ETA but we'll look into it.

JonathonRP commented 7 months ago

@Lms24 I appreciate that but it is blocking me on my project as app is completely not working from using sentry, the tracking doesn't work in svelte5 the internal current_component isn't exported but they have an extend that works for custom web components I think. I tried the svelte browser and got errors with that too. I think a minor step could be to have tracking off by default instead...

Lms24 commented 7 months ago

it is blocking me on my project

Component tracking is not enabled by default and it's not required for Sentry to generally work. Also, it is not documented for SvelteKit, only for browser-only Svelte apps. It just adds a couple of more details around components to our performance monitoring product.

I don't fully understand what's going on here then. I was under the impression that you wanted to add component tracking and this was giving you errors.

If your app generally crashes after installing @sentry/sveltekit (either with the wizard or following the manual setup) we should figure out why that's happening. Are you able to provide a minimal reproduction in the form of e.g. a GitHub repo? I can't reproduce this myself and I need some clues to debug this. Thank you!

JonathonRP commented 7 months ago

@Lms24 sure I'll try to create repo in mean time the confusion could be caused by me, as I thought it was on by default because of the 'import { current_component } from "svelte/internal"' which I thought was tracking feature, I opened this because I thought disabling tracking would be a work around for my other issue. Now, though I think it's the performance feature.

JonathonRP commented 7 months ago

@Lms24 here is a Repo https://stackblitz.com/edit/sveltejs-kit-template-default-2lc4he?file=package.json

Lms24 commented 7 months ago

I'm going to close this because I believe the reported error has the same cause as in #10275. Please let me know if you think this is not the case and if this should be reopened.