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

SvelteKit SDK Roadmap #6692

Closed
12 of 19 tasks
smeubank opened this issue Jan 9, 2023 Discussed in #5838 · 60 comments
Closed
12 of 19 tasks

SvelteKit SDK Roadmap #6692

smeubank opened this issue Jan 9, 2023 Discussed in #5838 · 60 comments
Assignees
Labels
Package: sveltekit Issues related to the Sentry SvelteKit SDK Type: Improvement

Comments

@smeubank
Copy link
Member

smeubank commented Jan 9, 2023

Discussed in #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

  1. 4 of 4
    Package: sveltekit Type: Feature
    Lms24
  2. 3 of 3
    Feature: Errors Package: sveltekit Type: Feature
    AbhiPrasad
  3. 3 of 3
    Feature: Errors Package: sveltekit Type: Feature
    AbhiPrasad
  4. 8 of 8
    Package: sveltekit
    Lms24
  5. 7 of 7
    Package: sveltekit Status: Backlog Type: Feature
    AbhiPrasad
  6. 5 of 5
    Feature: Performance Package: sveltekit Type: Feature
    Lms24
  7. 11 of 11
    Feature: Source Maps Package: sveltekit Type: Feature
    Lms24
  8. AbhiPrasad

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.

Tasks

  1. 10 of 10
    Lms24
  2. Package: sveltekit Status: Backlog
    Lms24
  3. 8 of 8
    Package: sveltekit
    Lms24

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

Goal: The moon 🚀

Dog-fooding opportunities:

  • JS SDK Bundle Size measurement app?

┆Issue is synchronized with this Jira Improvement by Unito

@smeubank smeubank added Type: Improvement Package: svelte Issues related to the Sentry Svelte SDK labels Jan 9, 2023
@AbhiPrasad AbhiPrasad pinned this issue Jan 9, 2023
@Lms24
Copy link
Member

Lms24 commented Jan 10, 2023

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

Lms24 commented Jan 10, 2023

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 Support different JS runtimes #5611

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

@Lms24
Copy link
Member

Lms24 commented Jan 10, 2023

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

@HazAT HazAT added Team: Web Frontend Sync: Jira apply to auto-create a Jira shadow ticket and removed Sync: Jira apply to auto-create a Jira shadow ticket labels Feb 3, 2023
@jycouet
Copy link

jycouet commented Feb 20, 2023

I would like to have SvelteKit support too. 👍

//From Svienna, Sentry talk.

🎉🎉🎉

@KayoticSully
Copy link

KayoticSully commented Feb 26, 2023

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

@Lms24 Lms24 changed the title SvelteKit Support SvelteKit Support Roadmap Mar 6, 2023
@Lms24 Lms24 added Package: sveltekit Issues related to the Sentry SvelteKit SDK Status: In Progress and removed Package: svelte Issues related to the Sentry Svelte SDK Status: Backlog labels Mar 6, 2023
@Lms24 Lms24 changed the title SvelteKit Support Roadmap SvelteKit SDK Roadmap Mar 6, 2023
@smeubank
Copy link
Member Author

smeubank commented Mar 7, 2023

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

Lms24 commented Mar 20, 2023

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

jycouet commented Mar 21, 2023

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: sveltejs/kit#6708 to improve this management?)


Thx again & happy coding

@Lms24
Copy link
Member

Lms24 commented Mar 22, 2023

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

jycouet commented Mar 22, 2023

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?
    image
  • 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

@jycouet
Copy link

jycouet commented May 22, 2023

Also, would be nice to bump some dep as I get deprecated [email protected] on pnpm i

dependencies:
@sentry/sveltekit 7.52.1
└─┬ @sentry/svelte 7.52.1
  └─┬ magic-string 0.26.7
    └── sourcemap-codec 1.4.8

Thx

@Lms24
Copy link
Member

Lms24 commented May 24, 2023

RE GET null server transactions
EDIT: Extracted this to a separate issue in #8199. Will link to your replies but let's please continue over there.

@Lms24
Copy link
Member

Lms24 commented May 24, 2023

To everyone following this thread: If you come across bugs or have specific requests, please open a new issue instead of replying here, thanks! This helps us triaging and keeping a better overview, thanks :)

@skylar
Copy link

skylar commented Jun 10, 2023

Updating form 7.49.0 -> 7.50.0 (or any thereafter) results in this error when I run vite dev:

This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason:
TypeError: response.headers is not iterable
    at setResponse (file:///project/node_modules/.pnpm/@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/exports/node/index.js:119:38)
    at file:///project/node_modules/.pnpm/@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/exports/vite/dev/index.js:520:6
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

It also fails when run in production mode (e.g. Vercel). I'm puzzled no one else has seen this as my project isn't very different than most simple SvelteKit projects. I updated to the latest versions of Vite (4.3.0) and SvelteKit (1.20.2) but to no avail.

@Lms24
Copy link
Member

Lms24 commented Jun 19, 2023

@skylar (sorry for getting back to you only now, I was ooo) would you mind opening an issue with a minimal reproduction for this? It's also the first time that I'm hearing about this.

@isaacharrisholt
Copy link

Is there a timeline for supporting the Vercel edge runtime? Currently getting this error when I try to build:

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

    ../../node_modules/.pnpm/@[email protected]_@[email protected][email protected]/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/.pnpm/@[email protected]_@[email protected][email protected]/node_modules/@sentry/sveltekit/esm/client/router.js:3:33: ERROR: Could not resolve "$app/stores"
    at failureErrorWithLog (<REPO PATH>/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1639:15)
    at <REPO PATH>/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1051:25
    at <REPO PATH>/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:996:52
    at buildResponseToResult (<REPO PATH>/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1049:7)
    at <REPO PATH>/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1078:16
    at responseCallbacks.<computed> (<REPO PATH>/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:700:9)
    at handleIncomingPacket (<REPO PATH>/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:755:9)
    at Socket.readFromStdout (<REPO PATH>/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:676:7)
    at Socket.emit (node:events:513:28)
    at Socket.emit (node:domain:489:12)
 ELIFECYCLE  Command failed with exit code 1.
ERROR: command finished with error: command (<REPO PATH>/apps/resident) pnpm run build exited (1)

@Lms24
Copy link
Member

Lms24 commented Aug 3, 2023

Hi folks, I'm going to close this issue for the time being. We're currently not actively working on new SvelteKit SDK features other than fixing bugs and adding general SDK improvements. As you can see, there are still some open points in the roadmap but we have to prioritize things and currently, other tasks have higher priorities.

Does this mean we won't do any of them? No! I'd love to get some stuff done once we have some time for it.

There's one exception to this, which is the (Vercel) Edge runtime support. We're working on getting there. However, to work on the necessary SvelteKit-specific part, we first have to refactor some general SDK internals first. This is being tracked in #8693.

Please feel free to open issues with specific bug reports or feature requests! thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: sveltekit Issues related to the Sentry SvelteKit SDK Type: Improvement
Projects
Archived in project
Status: Done
Development

No branches or pull requests