Add support for `handleLoad` hook on client and server hooks

[Lms24]

Describe the problem

TL;DR: We (Sentry) propose handleLoad hooks for client and server to have a central hook to wrap users' load functions with common functionality.

Hi, Sentry SDK dev again 👋

As briefly mentioned in #9530, we're working on a dedicated Sentry SvelteKit SDK. Among a lot of other features, we want to instrument users' load functions to automatically capture performance data and errors during data loading operations. Our SDK currently provides a wrapper for load that users can use in the following way:

// +page(.server).js

import {wrapLoadWithSentry} from '@sentry/sveltekit'

export const load = wrapLoadWithSentry((event) => {
  // whatever users do here
  return {message: 'Hi'}
});

while in theory this works quite well, in practice, it is a rather cumbersome job for our users to wrap every load function with this wrapper to integrate Sentry with their Kit apps.

Describe the proposed solution

Therefore, we propose a new hook called handleLoad for both hooks.client.js as well as hooks.server.js. We came up with an API that's similar to the server-side handle hook:

// hooks.(server|client).js

export const handleLoad = ({event, resolve}) => {
  // Do things before `load` is invoked
  const result = resolve(event);
  // Do things after `load` is invoked
  return result;
}

This hook would run whenever a universal load function is invoked, be it on the server or on the client.

To support server-only loads, we propose an additional handleServerLoad hook which would obviously only run on the server. API-wise, it would be the same as handleLoad just with the ServerLoad typings.

Optionally (if you think this is a good idea), users could specify the sequence function just like for handle to chain multiple handlers

This way, we could instruct our users to just add a Sentry handler to this hook once, instead of going through all their load functions and wrapping them with our wrapper:

// hooks.(server|client).js
import {handleLoadWithSentry} from '@sentry/sveltekit'

export const handleLoad = handleLoadWithSentry()

// only in hooks.server.js
export const handleServerLoad = handleServerLoadWithSentry()

We'd be happy to contribute this hook ourselves or help you all out in any way we can!

Alternatives considered

As we think that instructing our users to manually wrap all load function is too cumbersome, there's only one alternative that doesn't require changes in SvelteKit:

We create a Vite plugin which at build time, would try to wrap load functions with our wrapper automatically. We're currently doing this in our NextJS SDK (with Webpack) to auto-wrap NextJS data fetchers but it proved to be quite brittle and error-prone. We started out by modifying the AST, which we quickly abandoned and moved to a more sophisticated solution. However, it still depends a lot on users' project setups and the framework being stable itself, making it tricky to maintain on our end. Also, this whole process is rather opaque to our users as they don't see what's happening in their source code. Furthermore, we're aware that certain Vite plugins for SvelteKit (e.g. Houdini) already generate load functions at build time, meaning that things like plugin order or other interference might become an issue. For the mentioned reasons we would like to avoid such a build-time solution if possible at all.

Generally speaking, creating the SDK for SvelteKit is a very enjoyable process, especially thanks to the already provided hooks. In the end we would love to be able to have everything runtime-Sentry-related in the two hooks files which would be very clean in our view and super easy to set up for our users. Leveraging hooks feels like the most SvelteKit-native approach to us and we'd love to see handleLoad becoming reality.

Importance

would make Sentry users' life much easier (and admittedly Sentry devs' lives as well ;) )

Additional Information

I already took a stab at implementing this myself in SvelteKit and I opened a PoC PR (#9543). I know I'm probably missing cases and the whole thing needs a proper review and tests but maybe it's a place to start. I'm happy to collaborate and continue working on this PR if you think we could add this hook to SvelteKit!

Also, if you have another idea how we could easily instrument load functions in one place, we're definitely up for suggestions :)

Thank you for taking this issue into consideration!

[dummdidumm]

@AlecAivazis @jycouet would something like this also be useful for Houdini? For example, instead of faking a +page.js on the file system you could write out a load function somewhere in .houdini and reference those files inside your also provided handleLoad hook. Does this sounds sensible? If yes, what things would you need from handleLoad to reference the correct generated houdini load function?

[AlecAivazis]

Ohh neat! This would save us from having to create the fake +page.js files for loads but it wouldn't save us from having to create fake files for our authentication story (which relies on .svelte files). I think as long as we can figure out a good story for async importing each load implementation to keep bundle size in check (and prefetching still works) it would be a great improvement to our overall stability (although probably something we'd wait to include with more breaking changes).

To answer your question: I don't think we would need much more than the url but it would probably be good to also provide a reference to the load function that the user defined.

[dummdidumm]

The thing is that (from how I understand the feature request) handleLoad would be invoked multiple times per route, if there is for example a load function in a +layout.js file as well as a +page.js file. So the URL is probably not enough to determine which load function exactly is invoked - how would you determine that, or is it not relevant to you anyway?

[Lms24]

Actually, that's a good point. Knowing which file in the route is responsible for the invocation would also be beneficial for Sentry as we could add this information to our load span.

[AlecAivazis]

Ahh good point - I had forgotten about layouts. That's what I get for responding while out to dinner.

We do need to be able to distinguish between +page and +layout files so we would need the full file path for where the load would be coming from. However, this handle being called at every possible depth (to see if we need to "fake" a +layout in every directory) seems a bit... much. I'm not sure if there's a better answer but it was kind of nice that the static transformation approach avoided any unnecessary runtime cost.

[Lms24]

I'd like to bump this as it's still important to us. However, I want to add another thing I recently learned:

SvelteKit proxies or listens to accesses to various properties in the load functions' event objects to decide when to invalidate a route or not. This is problematic for us because our current wrap(Server)LoadWithSentry wrappers (example) our SDK exposes access for instance event.route.id and in rare cases event.url, causing data invalidations that are completely opaque to our users.

• [Sveltekit] Sveltekit auto instrumentation in vite plugin causes load functions to invalidate on every route change getsentry/sentry-javascript#8610
• Using the SvelteKit SDK breaks SSR functionality of TanStack Query getsentry/sentry-javascript#8818

While we were able to work around this in our server load wrapper, there's no way to do this on the client-side because of the missing Proxy. I'll gladly open a PR to add a proper proxy for this on the client side but I'm wondering if we need a different API to safely access things like the route id, without degrading the behaviour of our users' apps.

So the bottom line is: If we implement a handleLoad hook, it would only be useful for us if we can safely access the event (or at least) parts of it. Not sure, how this would work exactly internally but I'm wondering about the SvelteKit team's thoughts here in general.

IOW, how can SvelteKit permit Telemetry tools (like Sentry, but there are others) to safely observe such values without interfering with the app's behaviour but while maintaining data integrity (i.e. invalidate whenever necessary) at the same time?