Skip to content

feat(sveltekit): Auto-instrument universal and server load functions #7969

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

Closed
wants to merge 2 commits into from

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Apr 26, 2023

This PR adds auto-wrapping of load functions in page(.server).(ts|js) files to the SvelteKit SDK.

API

API wise, this is not a breaking change for users, as internally, we just add an additional plugin which is returned by the sentrySvelteKit plugin factory function. However, users can add new options to the factory functions, to control auto-wrapping:

sentrySvelteKit({
  autoInstrument: true, // false to disable entirely
  // or
  autoInstrument: {
    load: true,
    serverLoad: false,
  },
  // other options...
  autoUploadSourceMaps: false,
  debug: true,
})

Methodology

After trying a lot of options to bundle the original page module with a Sentry wrapper template (i.e. proxy loader in NextJS), I pivoted to modify the AST directly. The reason is that every time we touched source map generation, the resulting server source maps would no longer point to the original +page.ts/js files, causing broken source maps.
I believe this is a bug in the SvelteKit build process but I didn't find a way to work around this. Therefore, I suggest we go with the AST-based approach for now and revisit later if necessary.

I'm not too happy with the current approach because the fact that we can't touch the source maps also means, our AST wrapping can't modify the file content arbitrarily. We have to preserve the original location (at least the line) where the declaration of the load function was.

This PR also adds various tests for different kinds of load exports to ensure we handle them correctly.

closes #7940

@Lms24
Copy link
Member Author

Lms24 commented Apr 26, 2023

Draft, as I just had an idea. Will mark as ready for review once I verified it.

@Lms24 Lms24 force-pushed the lms/sveltekit-autowrap-load branch from cbf2cfc to 4058c73 Compare April 26, 2023 11:28
@Lms24 Lms24 force-pushed the lms/sveltekit-autowrap-load branch from 4058c73 to a74d6e6 Compare April 26, 2023 11:29
@Lms24
Copy link
Member Author

Lms24 commented Apr 28, 2023

Closing in favour of #7994

@Lms24 Lms24 closed this Apr 28, 2023
@Lms24 Lms24 deleted the lms/sveltekit-autowrap-load branch December 3, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-instrument SvelteKit load functions
1 participant