SvelteKit SDK Roadmap - Githubissues

smeubank commented 1 year ago

Discussed in https://github.com/getsentry/sentry-javascript/discussions/5838

Problem Statement

There is already support for the Svelte JavaScript framework, and now that SvelteKit is also in a 1.0 stable release we should extend the support.

See: https://svelte.dev/ SvelteKit: https://kit.svelte.dev/

If you are interested in this, please react to this issue and/or leave a comment below!

Solution Brainstorm

An SDK similar to what we have for NextJS, providing support for client- and server-side error and performance monitoring.

Initial Work

Initially, we'll focus on these tasks to release the first version of the SvelteKit SDK. While this version will still require manual instrumentation, it should be enough to release and receive early feedback from the community.

The following high-level tasks will be extracted to separate issues each (if applicable) to track them individually on a more fine-grained level:

#### Initial Tasks
- [ ] https://github.com/getsentry/sentry-javascript/issues/7348
- [ ] https://github.com/getsentry/sentry-javascript/issues/7402
- [ ] https://github.com/getsentry/sentry-javascript/issues/7403
- [ ] https://github.com/getsentry/sentry-javascript/issues/7489
- [ ] https://github.com/getsentry/sentry-javascript/issues/7526
- [ ] https://github.com/getsentry/sentry-javascript/issues/7413
- [ ] https://github.com/getsentry/sentry-javascript/issues/7669
- [ ] https://github.com/getsentry/sentry-javascript/pull/7881

Goal: Users can add and configure the SvelteKit SDK to their project and initialize the SDK for errors, performance and Replay. At this point, fully using the SDK will require manual wrapping of data fetchers and handlers. This SDK will work with the Node SvelteKit Adapter.

Advanced Features

Once we have a basic SDK, we start looking at more advanced features. These differ from stretch goal in the sense that they have immediate high value. For example: They make usage significantly easier or increase the supported platforms in which the SDK can be used.

- [ ] https://github.com/getsentry/sentry-wizard/issues/244
- [ ] https://github.com/getsentry/sentry-javascript/issues/7940
- [ ] https://github.com/getsentry/sentry-javascript/issues/8085
- [ ] Support Vercel Edge runtime

Goal: Users no longer need to manually add Sentry wrappers to their data fetchers and handlers. Instead, this will be done automatically at build time. Additionally, users can use the SDK not just on Node servers but also on Vercel.

Stretch Goals

If we have enough resources and time, there are a couple of cool stretch goals to take a look at: We will likely not be able to implement all of these features from the start. That's okay. We can extract them to separate issues or implement them by demand at a later time.

#### Stretch Goals
- [x] Consider issue grouping rules for SvelteKit
- [ ] Resolve source maps for errors in dev mode (analogously to NextJS)
- [ ] Turn around trace-parent order in page load trace (i.e. client txn becomes parent of server txn)
- [ ] Add `tunnelRoute` option (analogously to NextJS)
- [ ] Store Navigation event (https://kit.svelte.dev/docs/types#public-types-navigationevent) in client side `handleError` call
- [ ] Store Request event (https://kit.svelte.dev/docs/types#public-types-requestevent) in server side `handleError` call
- [ ] Support more JS Runtimes/adapters (Cloudflare, Netlify)

Goal: The moon 🚀

Dog-fooding opportunities:

JS SDK Bundle Size measurement app?

┆Issue is synchronized with this Jira Improvement by Unito

Lms24 commented 1 year ago

@Lms24 i think there was still discussion if this would be an own package

I think we definitely need to make this its own package, just like the NextJS/Remix SDKs.

Lms24 commented 1 year ago

Copying a few of the challenges for a SvelteKit SDK I identified so far from the GH discussion:

How to automatically instrument/wrap load functions, both on the client and the server? (Proxy loader approach from Next.JS?)
- Similarly, how to wrap API routes?
- Similarly, can we auto-wrap the handle hook on the server?
~How to parameterize client routes (pageloads, navigations)?~
- I already have a quite functional routing instrumentation so this should be quite easily do-able.
How to make SDK setup as simple as possible
- Can we inject the Sentry.init calls into the hooks files?
- Automatically upload source maps (Similarly to Next.JS)
- Wrap vite/SvelteKit config so that we can add our stuff to it (e.g. preprocessors)?
Will the SDK be compatible with Vercel edge functions, if an SK app is deployed to Vercel and edge functions are used? Similarly for other runtimes (e.g cloudflare workers)? See https://github.com/getsentry/sentry-javascript/issues/5611

EDIT: Also refined what we need to instrument on client and server in the original post

Lms24 commented 1 year ago

For future reference: For the routing instrumentation, I currently have a version running that uses the navigating store. we might want to look into the navigation lifecycle hooks before/afterNavigate

jycouet commented 1 year ago

I would like to have SvelteKit support too. 👍

//From Svienna, Sentry talk.

🎉🎉🎉

KayoticSully commented 1 year ago

I would be very interested in this as well. Specifically a cloudflare workers compatible implementation. Thanks for considering a SvelteKit integration though!

smeubank commented 1 year ago

I would be very interested in this as well. Specifically a cloudflare workers compatible implementation. Thanks for considering a SvelteKit integration though!

are you already working with toucan-js? We have not discussed to start with native support for cloudflare to start. If we ever get around to it, best done in a way that should not be specific to any particular framework.

But we are happy to gather feedback about what is and is not working, with our SDKs there and learnings from using the toucan-js package

Lms24 commented 1 year ago

Hello everyone, we just released version 7.44.0 with the first alpha version of @sentry/sveltekit. Please take a look at the README to learn more about how to set up the SDK and what to expect from this first alpha release.

We'd really appreciate feedback, be it bug reports, UX/DX opinions, or of course thanks and praises 😄 Feel free to reach out in this thread, or in case of a bug report, please open a new issue. Thanks!

jycouet commented 1 year ago

Hi @Lms24, and thank you for this first version.

I just had a look, and I have a few questions/comments (in random order).

1/ What is withSentryViteConfig doing? Isn't it easier to add a sentryVitePlugin in the plugin section?

2/ sentry.client.config.ts & sentry.server.config.ts doesn't have access to env Today, I use the Svelte integration and I put it in a +layout.svelte Like this, I can make use of

import { PUBLIC_SENTRY_DSN } from '$env/static/public'

3/ sentry.client.config.ts & sentry.server.config.ts doesn't have access to dev To use:

import { dev } from '$app/environment'

4/ Sentry.init doesn't have access to data I use it to send the release version like this: My +layout.svelte 👇

if (PUBLIC_SENTRY_DSN) {
        Sentry.init({
            dsn: PUBLIC_SENTRY_DSN,
            tracesSampleRate: 0.5,
            replaysSessionSampleRate: 0.05,
            replaysOnErrorSampleRate: 1.0,
            release: `${dev ? 'dev' : 'prod'} - v${data.releaseCreatedAtUtc.toISOString()}`,
            environment: dev ? 'development' : 'production',
            integrations: [new Sentry.Replay(), new BrowserTracing()],
            initialScope: (scope) => {
                return scope.setExtra('DEPARTMENT', PUBLIC_DEPARTMENT)
            }
        })
    }

5/ Limitation: manually add our wrappers to all your load I'm wondering how you will achieve this. In Houdini (GraphQL Client lib), we generate some load function (and +page.js) at buildtime, so our plugin probably needs to be here first for you to inject after this wrapper. (but then, files are virtual, maybe you would be interested in: https://github.com/sveltejs/kit/discussions/6708 to improve this management?)

Thx again & happy coding

Lms24 commented 1 year ago

Hey @jycouet thanks for your feedback, really appreciate you taking a look!

Let me answer your points:

1/ What is withSentryViteConfig doing?

It applies additional properties to the users' vite config, like adding a new plugin to inject our SDK init calls into the client and server bundles.

Isn't it easier to add a sentryVitePlugin in the plugin section?

Our idea with wrapping the entire config is that it's easier for us to introduce and for our SDK users to consume changes. For instance, if we introduce more plugins (which we'll do once we upload source maps automatically) or add other properties to the vite config, users don't need to make these changes themselves but our wrapper takes care of this.

2/ sentry.client.config.ts & sentry.server.config.ts doesn't have access to env 3/ sentry.client.config.ts & sentry.server.config.ts doesn't have access to dev

This is a good point, thanks for raising it. We might need to revisit the DX around intializing the SDK then. Perhaps, we just instruct folks to add the init call in hooks.(client|server).(ts|js). From local testing and a few PoCs before we started working on the SDK I found the hooks files to be good entry points for the SDK. What do you think, would this make more sense from a SvelteKit PoV?

4/ Sentry.init doesn't have access to data

Hmm I guess this is tricky because I'm pretty sure that hooks files don't have access to that either 😅 For this I'm wondering if it is actually a good idea to use. Once we include our release creation and source maps plugin to the SvelteKit SDK, we'll be able to inject the release into the code at build time, which means you don't need to set it manually at all when initializing the SDK. So at this point I'd de-prioritize this use case if that makes sense.

5/ Limitation: manually add our wrappers to all your load I'm wondering how you will achieve this.

So our rough idea here is to take a similar approach to what we did for NextJS. There, at build time, we load a template that imports the users' getServerSideProps function, wrap our wrapping function around it, bundle the whole thing with rollup and replace the result with the users' file. It sounds very complicated but it proved to be more reliable than for instance modifying the AST to wrap our wrapper around the user's function. So far the theory. We'll still need to evaluate how we do this exactly for SvelteKit.

In Houdini (GraphQL Client lib), we generate some load function (and +page.js) at buildtime, so our plugin probably needs to be here first for you to inject after this wrapper

That's good to know and something we'll need to consider. Right now, we make sure that our plugin runs first but we can of course adjust this.

A general note: We're trying to keep DX for our full stack framework SDKs as similar as possible. For example, in the NextJS SDK, we also use dedicated sentry.(client|server).config.(ts|js) files to init and configure the SDK, just like we have a withSentryConfig wrapper around the NextJS app config object. That being said, if this means that DX will suffer (like points 2 and 3 you mentioned), we should revisit these decisions and chose a more SvelteKit-native way.

jycouet commented 1 year ago

Great reading all this 👍

1/ What is withSentryViteConfig doing?

Today, I feel that it's "We take the control". But, with the proper documentation explaining what it's doing and why (like you did), I would be more relaxed. Especially since my config is way bigger than the example, I was a bit worried that some parts of my config will not be handled. So I guess it's mainly documentation, thx for sharing

2/ sentry.client.config.ts & sentry.server.config.ts doesn't have access to env 3/ sentry.client.config.ts & sentry.server.config.ts doesn't have access to dev

In Houdini, we also have a houdini.config.js and we have the need of env as well. So we give env to the function. Maybe it's something worth exploring for the setup?
Yes, I'm a big fan of hooks too, I think that it's a great way to do things. (with examples using sequence, because you will not be the only hook of an app.)
sentry.(client&server).config.(ts|js) & hooks.(client&server).(ts|js) feels like a lot! I think that the more SvelteKit way would be hooks.

4/ Sentry.init doesn't have access to data

I agree that what I'm doing is probably not the "recommended way", and would be happy to switch to a more handled way like you described. I would like to benefit from the "release" feature from the server & client side. I think that the kit version should be able to do this.

5/ Limitation: manually add our wrappers to all your load

Yes, we do the AST also in Houdini, that's why I want to keep an eye on it. (plugin order, sentry & Houdini working 🤞😅)

Happy to share my thoughts 🎉 Speak to you soon

Lms24 commented 1 year ago

Hi folks,

we've just released 7.46.0 with a lot of updates for the SvelteKit SDK. Check out this post for a summary.

Lms24 commented 1 year ago

Version 7.48.0 of the SDK adds automatic upload of source maps. Also, we had to make some changes how to set up the SDK in vite.config.js. Please take a look at the updated README for further details!

wtachau commented 1 year ago

Very exciting! Looking forward to trying it out.

Is there an estimate for when the SDK will be stable?

Lms24 commented 1 year ago

@wtachau at the moment we're trying to simplify the SDK setup experience by adding SvelteKit support to our SDK setup Wizard. After this is done, we'll promote the SDK to a stable state. So hopefully not too far in the future :)

BTW, we'll soon move to beta: #7874

wtachau commented 1 year ago

Amazing, thanks!

colinhowells commented 1 year ago

Thanks for prioritizing SvelteKit, great to see!

Question about await in wrapServerLoadWithSentry – right now my IDE complains, because it expects async (even though wrapServerLoadWithSentry is async), what's the syntax I should be using?

/** @type {import('./$types').PageServerLoad} */
export const load = wrapServerLoadWithSentry((event) => {
  const someData = await getSomeDataFromElsewhere(); // 'await' here is flagged as an error
  return { someData };
});

colinhowells commented 1 year ago

I suppose one way to do it is to split the functions:

/** @type {import('./$types').PageServerLoad} */
export const load = wrapServerLoadWithSentry(getSomeData);

const getSomeData = async (event) => {
  const someData = await getSomeDataFromElsewhere();
  return { someData };
}

madeleineostoja commented 1 year ago

Amazing to hear! Will the support for vercel / edge networks come before a stable release? If not is there a rough eta for those items under ‘advanced features’?

wtachau commented 1 year ago

+1 for vercel adapter

jsanagustin commented 1 year ago

Thanks for prioritizing SvelteKit, great to see!

Question about await in wrapServerLoadWithSentry – right now my IDE complains, because it expects async (even though wrapServerLoadWithSentry is async), what's the syntax I should be using?
/** @type {import('./$types').PageServerLoad} */
export const load = wrapServerLoadWithSentry((event) => {
  const someData = await getSomeDataFromElsewhere(); // 'await' here is flagged as an error
  return { someData };
});

@colinhowells Pretty sure you just need to add async before (event):

/** @type {import('./$types').PageServerLoad} */
export const load = wrapServerLoadWithSentry(async (event) => {
  const someData = await getSomeDataFromElsewhere(); // 'await' here is flagged as an error
  return { someData };
});

colinhowells commented 1 year ago

ah, yeah, I thought I missed something obvious and did – thanks

Lms24 commented 1 year ago

Will the support for vercel / edge networks come before a stable release?

At this time, our plan is to go stable before supporting Vercel. This doesn't mean though that it's not gonna happen. "Stable" for us primarily means no more breaking changes and a consistent setup experience, including docs and everything that needs to happen around Sentry supporting a new platform/framework. We'll definitely add more features after the SDK is stable and Vercel/Edge is among the most important ones.

colinhowells commented 1 year ago

Just in case anyone was wondering, Cloudflare Pages dies on build (this is an esbuild error) – there might be some Vite configuration piece I'm missing though I've tried prebundling:

✘ [ERROR] Could not resolve "$app/stores"

node_modules/@sentry/sveltekit/esm/client/router.js:3:33:
3 │ import { page, navigating } from '$app/stores';
  ╵                                  ~~~~~~~~~~~~~

You can mark the path "$app/stores" as external to exclude it from the bundle, which will remove this error.

madeleineostoja commented 1 year ago

I'm having trouble with the sourcemap uploading in the new vite plugin, not too sure where to go next with debugging.

Throwing this stacktrace on build:

> Rewriting sources
> Rewriting sources
> Rewriting sources
> 
> 
> Rewriting completed in 0s
> Adding source map references
> Adding source map references
> Adding source map references
> Nothing to upload
> Nothing to upload
> Nothing to upload
[sentry-vite-plugin] Info: Successfully uploaded source maps.
[sentry-vite-plugin] Info: Successfully finalized release.
[commonjs--resolver] Unexpected character '�' (Note that you need plugins to import files that are not JavaScript)
file: {MY_PROJECT}/node_modules/.pnpm/chokidar@3.5.3/node_modules/chokidar/index.js:1:0
1: ����@�
         @��...
   ^
2: 
3: 
   *
    h���/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFound...

janvotava commented 1 year ago

I'm having trouble with the sourcemap uploading in the new vite plugin, not too sure where to go next with debugging.

Throwing this stacktrace on build:

> Rewriting sources
> Rewriting sources
> Rewriting sources
> 
> 
> Rewriting completed in 0s
> Adding source map references
> Adding source map references
> Adding source map references
> Nothing to upload
> Nothing to upload
> Nothing to upload
[sentry-vite-plugin] Info: Successfully uploaded source maps.
[sentry-vite-plugin] Info: Successfully finalized release.
[commonjs--resolver] Unexpected character '�' (Note that you need plugins to import files that are not JavaScript)
file: {MY_PROJECT}/node_modules/.pnpm/chokidar@3.5.3/node_modules/chokidar/index.js:1:0
1: ����@�
         @��...
   ^
2: 
3: 
   *
    h���/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFound...

This happens when installed as a development dependency.

madeleineostoja commented 1 year ago

It's still happening to me when installed as a regular dependency. Might be worth noting I'm using it in a monorepo with shared config. Do we know what the underlying issue that causes this is?

Lms24 commented 1 year ago

@madeleineostoja any chance that you're trying to bundle the Sentry vite plugin? This is a dependency of @sentry/sveltekit but it shouldn't end up in server or client bundles. it's only relevant for the build time.

madeleineostoja commented 1 year ago

@Lms24 okay yep looks like consuming vite config from a shared package in a pnpm monorepo is the cause. Is there any workaround for monorepo usage? Would flagging the offending dep as external under config.rollupOptions work?

EDIT: Turns out all I needed to do was install @senty/sveltekit as a dep in the app itself, so that pnpm didn't try bundle it from my config (where vite plugin lives) and/or utils (where initialisation and hook helpers live) packages.

Idk whether it's worth documenting that it can't be bundled, for monorepos etc, or if I'm just an egg

madeleineostoja commented 1 year ago

One final question on roadmap — is there any further progress that could be surfaced on automatically instrumenting the load tracking? I've seen the issue upstream for a handleLoad hook in sveltekit, but it sounds like it might not be happening. Any other paths the Sentry team is considering for it?

I'm trying to evaluate whether it's worth manually wrapping every client and server load across my monorepo now (mainly for apm), which would be non-trivial given the size of it, or if another solution is on the near horizon.

Lms24 commented 1 year ago

Idk whether it's worth documenting that it can't be bundled, for monorepos etc, or if I'm just an egg

Yes, I think when we get to creating docs (aiming for the coming week), this is def worth mentioning. Monorepos are always a little complex with dependency management, hoisting, etc :)

is there any further progress that could be surfaced on automatically instrumenting the load tracking? I've seen the issue upstream for a handleLoad hook in sveltekit, but it sounds like it might not be happening. Any other paths the Sentry team is considering for it?

Yeah I was hoping this would go be resolved with a definitive answer faster but based on some internal conversations I know that the Svelte team is still considering it. They're just not prioritising SvelteKit features at the moment. While I still think that handleLoad would be the cleanest solution, I think we need to come up with an intermediate solution until then.

Actually, here's a question for you and for everyone following this thread:

To apply some sort of automation here, we have two options and I'm curious which you'd prefer:

Actually modifying source code with the Sentry wizard. You need to run npx @sentry/wizard -i sveltekit which would go into your page(.server).js files and actually wrap your load functions with our handle(Sever)LoadWithSentry wrapper. The advantage is that this is very transparent, as you know what Sentry is doing. The disadvantage is that this happens only when you run the wizard. Not at every build for instance. So there still is some maintenance required on your end. Please note that we're already working on the wizard to add the Sentry code in hooks files and vite config when setting up the SDK for the first time. The question is rather, if we should extend this behaviour to potentially touch a lot more files.
Auto-wrap the load functions at build time. This approach would add a Vite plugin which automatically applies our wrapper at build time. The advantage is that you don't have to do anything and everything just works. The disadvantage is that it might feel a bit intransparent to you, as no changes to your source code will be made but you runtime code will have the wrappers.

I'm currently leaning towards option 2 (mainly due to maintenance concerns for our users) but I'm curious for feedback :)

98mux commented 1 year ago

@Lms24 Option 2 sounds like the best one to me! Maybe if the name of the vite plugin sort of explained it like: "wrapSentryAroundLoad" for example, then it wouldn't feel as intransparent. Btw thanks for your amazing job so far! :D The sveltekit sentry is working like a charm for me

hrueger commented 1 year ago

I like both options, they both have their pros and cons. However, when you go with #1, I find it important to also have an "undo"-feature in the wizard. If in the future the handleLoad hook is available in Svelte Kit, unwrapping the load functions manually would be lots of work.

wtachau commented 1 year ago

2 is nice in that updates can be made through the vite plugin rather than asking users to update their code. It would be helpful for that option to have plenty of config (and an option to manually wrap the load functions) as everything magical is great until it doesn't work as expected :)

madeleineostoja commented 1 year ago

I agree that if we’re going to run a potentially brittle codemod that might be temporary anyway, I’d prefer to run it on build where I can just disable it and rebuild if it gets weird or a cleaner solution comes from upstream in future.

The transparency issue is a matter of docs and having the behaviour opt-in in the vite plugin with a well named param imo (something like wrapLoad: boolean maybe).

The other code mods the wizard will do are different because:

They’re finalised API
They sound far less brittle
They’re much less intrusive across a whole codebase

Lms24 commented 1 year ago

Thank you for the feedback RE wrapping load functions. We'll go with option 2, applying a build-time solution. I opened an issue to track this feature: #7940.

risalfajar commented 1 year ago

Is there any plan for adapter/static?

Lms24 commented 1 year ago

@risalfajar I haven't tested it out yet and so far it has not been on our radar. Looking into it, it might be possible to support it. In fact, as long as you didn't specify custom output directories (i.e. use the default build dir), it might just work ootb. Feel free to give it a try and let us know how it worked out.

happysalada commented 1 year ago

with the latest version of everything (@sentry/sveltekit 7.50.0 , sveltekit 1.15.9, svelte 3.58) I get the following error when following the steps in https://www.npmjs.com/package/@sentry/sveltekit

> Using @sveltejs/adapter-cloudflare
✘ [ERROR] Could not resolve "$app/stores"

    node_modules/.pnpm/@sentry+sveltekit@7.50.0_@sveltejs+kit@1.15.9_svelte@3.58.0/node_modules/@sentry/sveltekit/esm/client/router.js:3:33:
      3 │ import { page, navigating } from '$app/stores';

  You can mark the path "$app/stores" as external to exclude it from the bundle, which will remove
  this error.

did I miss anything ?

Lms24 commented 1 year ago

@happysalada hmm nothing should have changed with this. We're marking $app/stores as external when we transpile the SDK but this shouldn't affect you. Just to double-check:

Did this start happening after you updated the Sentry SDK or SvelteKit?
Did you add @sentry/sveltekit as a dependency? People have mistakenly added it as a devDependency in the past which caused bundling problems
Technically, the cloudflare adapter isn't supported so I'm not sure if this perhaps is only a problem with this adapter? Have you tried reproducing this with the Node adapter?

happysalada commented 1 year ago

I started installing this today, so I have never tried this before.

here are my dependencies

"dependencies": {
    "@sentry/sveltekit": "^7.50.0",
    "decimal.js": "^10.4.3",
    "surrealdb.js": "^0.6.0",
    "svelte-headlessui": "^0.0.15",
    "svelte-transition": "^0.0.7"
}

I'm not using node to deploy this unfortunately. No worries if this is not supported. Feel free to ping me when it is.

Swepool commented 1 year ago

@Lms24 seems to be the cloudflare adapter :/

matindow commented 1 year ago

with the latest version of everything (@sentry/sveltekit 7.50.0 , sveltekit 1.15.9, svelte 3.58) I get the following error when following the steps in https://www.npmjs.com/package/@sentry/sveltekit
> Using @sveltejs/adapter-cloudflare
✘ [ERROR] Could not resolve "$app/stores"

    node_modules/.pnpm/@sentry+sveltekit@7.50.0_@sveltejs+kit@1.15.9_svelte@3.58.0/node_modules/@sentry/sveltekit/esm/client/router.js:3:33:
      3 │ import { page, navigating } from '$app/stores';

  You can mark the path "$app/stores" as external to exclude it from the bundle, which will remove
  this error.
did I miss anything ?

i'm getting this when using adapter-cloudflare-workers with @sentry/sveltekit@7.51.0, no error when using @sentry/sveltekit@7.50.0.

> Using @sveltejs/adapter-cloudflare-workers
X [ERROR] Could not resolve "$app/stores"

    node_modules/@sentry/sveltekit/esm/client/router.js:3:33:
      3 │ import { page, navigating } from '$app/stores';
        ╵                                  ~~~~~~~~~~~~~

  You can mark the path "$app/stores" as external to exclude it from the bundle, which will remove
  this error.

error during build:
Error: Build failed with 1 error:
node_modules/@sentry/sveltekit/esm/client/router.js:3:33: ERROR: Could not resolve "$app/stores"
    at failureErrorWithLog (C:\x\node_modules\esbuild\lib\main.js:1636:15)
    at C:\x\node_modules\esbuild\lib\main.js:1048:25
    at C:\x\node_modules\esbuild\lib\main.js:993:52
    at buildResponseToResult (C:\x\node_modules\esbuild\lib\main.js:1046:7)
    at C:\x\node_modules\esbuild\lib\main.js:1075:16
    at responseCallbacks.<computed> (C:\x\node_modules\esbuild\lib\main.js:697:9)
    at handleIncomingPacket (C:\x\node_modules\esbuild\lib\main.js:752:9)
    at Socket.readFromStdout (C:\x\node_modules\esbuild\lib\main.js:673:7)
    at Socket.emit (node:events:513:28)
    at Socket.emit (node:domain:489:12)

Lms24 commented 1 year ago

@matindow, does the error still persist if you disable auto-instrumentation?

// vite.config.ts
export default defineConfig({
  plugins: [
    sentrySvelteKit({
      autoInstrument: false,
    }),
    sveltekit()
  ],
});

This is the only thing we changed in the 7.51.0 release, so my bet would be that something in this change triggered this error in the cloudflare adapter.

maximedupre commented 1 year ago

Thanks for this wonderful package team :)

I'm looking into adding user context/identifying users with my errors - does @sentry/sveltekit support this?

If it does, can anyone point me toward the right "technique" to achieve this?

I used to simply pass the user context like Sentry.captureException(..., { user }), but now everything is happening under the hood.

If it is at all possible, I suppose it will involve using scopes and hubs, but it's still unclear to me how it fits in the whole "hooks architecture" 🤔

Thanks

colinhowells commented 1 year ago

@Lms24 That's been a problem for quite a while

Lms24 commented 1 year ago

@maximedupre you can identify users (link to our docs) with Sentry.setUser.

I'd recommend to set this as early as possible so that errors and transactions/spans can be associated with the correct user.

@colinhowells right, I remember that cloudflare workers were always problematic. So I think this isn't related to the latest update. I know this isn't good news but for the time being, we don't have the capacity to look at cloudflare worker compatibility.

maximedupre commented 1 year ago

@Lms24 Thanks for the answer.

I'm currently using Sentry.setUser in hooks.server.ts in a sequence, right after Sentry.sentryHandle(), so that should be as soon as it can be.

My concern is that Sentry.setUser sets the user "globally" so there might be concurrency issues with parallel requests. Please let me know if this is a valid concern.

As for setting the user client-side, I'm also using Sentry.setUser in +layout.svelte when I set my $user store. It is necessary to set the Sentry user both on the client-side and the server-side, right?

Lms24 commented 1 year ago

@maximedupre

My concern is that Sentry.setUser sets the user "globally" so there might be concurrency issues with parallel requests. Please let me know if this is a valid concern.

This shouldn't be an issue as we're creating an async context for each incoming request to isolate Sentry scopes (which is where the user is set). If you notice leakage between users (or maybe also between tags), please open an issue so that we can take a look at it.

It is necessary to set the Sentry user both on the client-side and the server-side, right?

Yes, this is required

Apart from that, I think the places where you set your users look good to me.

matindow commented 1 year ago

@matindow, does the error still persist if you disable auto-instrumentation?
// vite.config.ts
export default defineConfig({
  plugins: [
    sentrySvelteKit({
      autoInstrument: false,
    }),
    sveltekit()
  ],
});
This is the only thing we changed in the 7.51.0 release, so my bet would be that something in this change triggered this error in the cloudflare adapter.

with autoInstrument: false it builds without error.

happysalada commented 1 year ago

I just tried that for cloudflare pages just in case, and the build still fails.

getsentry / sentry-javascript

SvelteKit SDK Roadmap #6692

Discussed in https://github.com/getsentry/sentry-javascript/discussions/5838

Problem Statement

Solution Brainstorm

Initial Work

Advanced Features

Stretch Goals

Dog-fooding opportunities: