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

fix(sveltekit): Avoid invalidating data on route changes in wrapServerLoadWithSentry #8801

Merged
merged 3 commits into from
Aug 16, 2023

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Aug 11, 2023

As reported in #8610, our SvelteKit SDK caused server load data to be invalidated, resulting in said functions being called on every route change. I initially thought this was related to our Kit-specific client fetch instrumentation which turned out not to be causing this. Instead, the culprit is our wrapServerLoadWithSentry wrapper:

In the wrapper, we access event.route.id to determine the route of the load function for the span description. Internally, SvelteKit puts a proxy on certain event properties, such as event.route. In case any property of event.route was accessed, SvelteKit marks this internally and send along a flag to the client. On a route change, the client would check this flag and mark the route as invalidated, thereby causing a call to the load function on each navigation.

Took me quite a while to find the cause but thanks to @kamilogorek we found a fix to access the route id without invoking the proxy.

closes #8610

@github-actions
Copy link
Contributor

github-actions bot commented Aug 11, 2023

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 75.12 KB (+0.36% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 31.14 KB (+0.09% 🔺)
@sentry/browser - Webpack (gzipped) 21.81 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 69.72 KB (+0.27% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 28.18 KB (+0.1% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped) 20.18 KB (+0.05% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 219.89 KB (+0.38% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 84.78 KB (+0.13% 🔺)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 59.86 KB (+0.04% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 31.04 KB (+0.09% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 65.3 KB (+0.39% 🔺)
@sentry/react - Webpack (gzipped) 21.84 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 92.94 KB (+0.32% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 50.66 KB (+0.1% 🔺)

@Lms24 Lms24 changed the title fix(sveltekit): Avoid invalidating data o route changes in wrapServerLoadWithSentry fix(sveltekit): Avoid invalidating data on route changes in wrapServerLoadWithSentry Aug 11, 2023
@Lms24 Lms24 marked this pull request as ready for review August 11, 2023 13:50
@Lms24 Lms24 requested review from a team, AbhiPrasad, ale-cota and kamilogorek and removed request for a team and ale-cota August 11, 2023 13:50
Co-authored-by: Kamil Ogórek <kamil.ogorek@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants