Skip to content

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Jan 8, 2025

This PR fixes a source maps deletion bug in the Sentry SvelteKit SDK reported in #14131.

After debugging this extensively, the root cause turned out to be a timing issue. For SvelteKit, we have to move all invocations of the writeBundle hook in the sub-plugins of the original Sentry Vite plugin to the closeBundle hook, to ensure that the second build made when the SvelteKit adapter is invoked is also awaited correctly. We did this correctly for the debug id source maps upload plugin but did not do it for the release management and file deletion plugins.

The changed order of things caused a deadlock situations due to our deletion plugin waiting on upload and release plugins to finish. Since the deletion plugin was started earlier than the upload plugin, it was awaiting something that wasn't even started yet.

This PR now:

  • replaces the original release management plugin with a custom one where we start release creation on closeBundle
  • replaces the original file deletion plugin with a custom one where we start deletion on closeBundle
  • in both cases, further ensures that the plugin is only invoked for build and as late as possible (enforce: 'post'). All of these three things ensure now that the deadlock situation is resolved.
  • searches for the release management plugin by its old and new name introduced in ref(core): Rename release management plugin name sentry-javascript-bundler-plugins#647
  • slightly renames the custom source maps uploading plugin for better convention; as well as some variables for better readability
  • adds unit tests for the new custom sub plugins

closes #14131
fixes #12660

@Lms24 Lms24 self-assigned this Jan 8, 2025
@Lms24 Lms24 requested review from lforst and s1gr1d January 8, 2025 13:54
Copy link
Member

@s1gr1d s1gr1d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for debugging and solving this 🙌

@Lms24 Lms24 merged commit 6779dfe into develop Jan 9, 2025
39 checks passed
@Lms24 Lms24 deleted the lms/fix-sveltekit-sourcemap-deletion-ii branch January 9, 2025 12:55
Lms24 added a commit that referenced this pull request Jan 9, 2025
…ps upload (#14942)

- replace the original release management plugin with a custom one
where we start release creation on `closeBundle`
- replace the original file deletion plugin with a custom one where we
start deletion on `closeBundle`
- in both cases, further ensuresthat the plugin is only invoked for
build and as late as possible (`enforce: 'post'`). 
- searche for the release management plugin by its old and new name
introduced in
getsentry/sentry-javascript-bundler-plugins#647
- slightly rename the custom source maps uploading plugin for better
convention; as well as some variables for better readability
- add unit tests for the new custom sub plugins
Lms24 added a commit that referenced this pull request Jan 9, 2025
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.

Sveltekit build fails with sourcemap deletion enabled Sourcemaps not deleted after upload for Sveltekit deployed to Vercel

2 participants