From df67de1ceb90b5297bd5ae362ee92999f6fa2ab4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 8 Jan 2025 14:45:18 +0100 Subject: [PATCH] fix(sveltekit): Ensure source maps deletion is called after source maps upload --- packages/sveltekit/src/vite/sourceMaps.ts | 89 ++++++++++- .../test/vite/sentrySvelteKitPlugins.test.ts | 10 +- .../sveltekit/test/vite/sourceMaps.test.ts | 146 ++++++++++++++++-- 3 files changed, 220 insertions(+), 25 deletions(-) diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index b664e2d23db5..a59e9af19260 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -76,6 +76,14 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug plugin => plugin.name === 'sentry-vite-debug-id-upload-plugin', ); + const sentryViteFileDeletionPlugin = sentryPlugins.find(plugin => plugin.name === 'sentry-file-deletion-plugin'); + + const sentryViteReleaseManagementPlugin = sentryPlugins.find( + // sentry-debug-id-upload-plugin was the old (misleading) name of the plugin + // sentry-release-management-plugin is the new name + plugin => plugin.name === 'sentry-debug-id-upload-plugin' || plugin.name === 'sentry-release-management-plugin', + ); + if (!sentryViteDebugIdUploadPlugin) { debug && // eslint-disable-next-line no-console @@ -85,7 +93,33 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug return sentryPlugins; } - const restOfSentryVitePlugins = sentryPlugins.filter(plugin => plugin.name !== 'sentry-vite-debug-id-upload-plugin'); + if (!sentryViteFileDeletionPlugin) { + debug && + // eslint-disable-next-line no-console + console.warn( + 'sentry-file-deletion-plugin not found in sentryPlugins! Cannot modify plugin - returning default Sentry Vite plugins', + ); + return sentryPlugins; + } + + if (!sentryViteReleaseManagementPlugin) { + debug && + // eslint-disable-next-line no-console + console.warn( + 'sentry-release-management-plugin not found in sentryPlugins! Cannot modify plugin - returning default Sentry Vite plugins', + ); + return sentryPlugins; + } + + const unchangedSentryVitePlugins = sentryPlugins.filter( + plugin => + ![ + 'sentry-vite-debug-id-upload-plugin', + 'sentry-file-deletion-plugin', + 'sentry-release-management-plugin', // new name of release management plugin + 'sentry-debug-id-upload-plugin', // old name of release management plugin + ].includes(plugin.name), + ); let isSSRBuild = true; @@ -95,8 +129,8 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug __sentry_sveltekit_output_dir: outputDir, }; - const customPlugin: Plugin = { - name: 'sentry-upload-sveltekit-source-maps', + const customDebugIdUploadPlugin: Plugin = { + name: 'sentry-sveltekit-debug-id-upload-plugin', apply: 'build', // only apply this plugin at build time enforce: 'post', // this needs to be set to post, otherwise we don't pick up the output from the SvelteKit adapter @@ -248,7 +282,54 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug }, }; - return [...restOfSentryVitePlugins, customPlugin]; + // The file deletion plugin is originally called in `writeBundle`. + // We need to call it in `closeBundle` though, because we also postpone + // the upload step to `closeBundle` + const customFileDeletionPlugin: Plugin = { + name: 'sentry-sveltekit-file-deletion-plugin', + apply: 'build', // only apply this plugin at build time + enforce: 'post', + closeBundle: async () => { + if (!isSSRBuild) { + return; + } + + const writeBundleFn = sentryViteFileDeletionPlugin?.writeBundle; + if (typeof writeBundleFn === 'function') { + // This is fine though, because the original method doesn't consume any arguments in its `writeBundle` callback. + const outDir = path.resolve(process.cwd(), outputDir); + try { + // @ts-expect-error - the writeBundle hook expects two args we can't pass in here (they're only available in `writeBundle`) + await writeBundleFn({ dir: outDir }); + } catch (e) { + // eslint-disable-next-line no-console + console.warn('Failed to delete source maps:', e); + } + } + }, + }; + + const customReleaseManagementPlugin: Plugin = { + name: 'sentry-sveltekit-release-management-plugin', + apply: 'build', // only apply this plugin at build time + enforce: 'post', + closeBundle: async () => { + try { + // @ts-expect-error - this hook exists on the plugin! + await sentryViteReleaseManagementPlugin.writeBundle(); + } catch (e) { + // eslint-disable-next-line no-console + console.warn('[Source Maps Plugin] Failed to upload release data:', e); + } + }, + }; + + return [ + ...unchangedSentryVitePlugins, + customReleaseManagementPlugin, + customDebugIdUploadPlugin, + customFileDeletionPlugin, + ]; } function getFiles(dir: string): string[] { diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index 29dc1b09fb34..f5fa7327fe49 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -55,11 +55,13 @@ describe('sentrySvelteKit()', () => { // default source maps plugins: 'sentry-telemetry-plugin', 'sentry-vite-release-injection-plugin', - 'sentry-debug-id-upload-plugin', 'sentry-vite-debug-id-injection-plugin', - 'sentry-file-deletion-plugin', + // custom release plugin: + 'sentry-sveltekit-release-management-plugin', // custom source maps plugin: - 'sentry-upload-sveltekit-source-maps', + 'sentry-sveltekit-debug-id-upload-plugin', + // custom deletion plugin + 'sentry-sveltekit-file-deletion-plugin', ]); }); @@ -76,7 +78,7 @@ describe('sentrySvelteKit()', () => { const instrumentPlugin = plugins[0]; expect(plugins).toHaveLength(1); - expect(instrumentPlugin.name).toEqual('sentry-auto-instrumentation'); + expect(instrumentPlugin?.name).toEqual('sentry-auto-instrumentation'); process.env.NODE_ENV = previousEnv; }); diff --git a/packages/sveltekit/test/vite/sourceMaps.test.ts b/packages/sveltekit/test/vite/sourceMaps.test.ts index 2d12c9835b58..9837067ec643 100644 --- a/packages/sveltekit/test/vite/sourceMaps.test.ts +++ b/packages/sveltekit/test/vite/sourceMaps.test.ts @@ -3,17 +3,31 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { Plugin } from 'vite'; import { makeCustomSentryVitePlugins } from '../../src/vite/sourceMaps'; -const mockedSentryVitePlugin = { +const mockedViteDebugIdUploadPlugin = { name: 'sentry-vite-debug-id-upload-plugin', writeBundle: vi.fn(), }; +const mockedViteReleaseManagementPlugin = { + name: 'sentry-release-management-plugin', + writeBundle: vi.fn(), +}; + +const mockedFileDeletionPlugin = { + name: 'sentry-file-deletion-plugin', + writeBundle: vi.fn(), +}; + vi.mock('@sentry/vite-plugin', async () => { const original = (await vi.importActual('@sentry/vite-plugin')) as any; return { ...original, - sentryVitePlugin: () => [mockedSentryVitePlugin], + sentryVitePlugin: () => [ + mockedViteReleaseManagementPlugin, + mockedViteDebugIdUploadPlugin, + mockedFileDeletionPlugin, + ], }; }); @@ -30,20 +44,22 @@ beforeEach(() => { vi.clearAllMocks(); }); -async function getCustomSentryViteUploadSourcemapsPlugin(): Promise { +async function getSentryViteSubPlugin(name: string): Promise { const plugins = await makeCustomSentryVitePlugins({ authToken: 'token', org: 'org', project: 'project', adapter: 'other', }); - return plugins.find(plugin => plugin.name === 'sentry-upload-sveltekit-source-maps'); + + return plugins.find(plugin => plugin.name === name); } describe('makeCustomSentryVitePlugin()', () => { it('returns the custom sentry source maps plugin', async () => { - const plugin = await getCustomSentryViteUploadSourcemapsPlugin(); - expect(plugin?.name).toEqual('sentry-upload-sveltekit-source-maps'); + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); + + expect(plugin?.name).toEqual('sentry-sveltekit-debug-id-upload-plugin'); expect(plugin?.apply).toEqual('build'); expect(plugin?.enforce).toEqual('post'); @@ -58,9 +74,9 @@ describe('makeCustomSentryVitePlugin()', () => { expect(plugin?.writeBundle).toBeUndefined(); }); - describe('Custom sentry vite plugin', () => { + describe('Custom debug id source maps plugin plugin', () => { it('enables source map generation', async () => { - const plugin = await getCustomSentryViteUploadSourcemapsPlugin(); + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); // @ts-expect-error this function exists! const sentrifiedConfig = plugin.config({ build: { foo: {} }, test: {} }); expect(sentrifiedConfig).toEqual({ @@ -73,7 +89,7 @@ describe('makeCustomSentryVitePlugin()', () => { }); it('injects the output dir into the server hooks file', async () => { - const plugin = await getCustomSentryViteUploadSourcemapsPlugin(); + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); // @ts-expect-error this function exists! const transformOutput = await plugin.transform('foo', '/src/hooks.server.ts'); const transformedCode = transformOutput.code; @@ -84,34 +100,34 @@ describe('makeCustomSentryVitePlugin()', () => { }); it('uploads source maps during the SSR build', async () => { - const plugin = await getCustomSentryViteUploadSourcemapsPlugin(); + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); // @ts-expect-error this function exists! plugin.configResolved({ build: { ssr: true } }); // @ts-expect-error this function exists! await plugin.closeBundle(); - expect(mockedSentryVitePlugin.writeBundle).toHaveBeenCalledTimes(1); + expect(mockedViteDebugIdUploadPlugin.writeBundle).toHaveBeenCalledTimes(1); }); it("doesn't upload source maps during the non-SSR builds", async () => { - const plugin = await getCustomSentryViteUploadSourcemapsPlugin(); + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); // @ts-expect-error this function exists! plugin.configResolved({ build: { ssr: false } }); // @ts-expect-error this function exists! await plugin.closeBundle(); - expect(mockedSentryVitePlugin.writeBundle).not.toHaveBeenCalled(); + expect(mockedViteDebugIdUploadPlugin.writeBundle).not.toHaveBeenCalled(); }); }); it('catches errors while uploading source maps', async () => { - mockedSentryVitePlugin.writeBundle.mockImplementationOnce(() => { + mockedViteDebugIdUploadPlugin.writeBundle.mockImplementationOnce(() => { throw new Error('test error'); }); - const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementationOnce(() => {}); + const consoleLogSpy = vi.spyOn(console, 'log').mockImplementationOnce(() => {}); - const plugin = await getCustomSentryViteUploadSourcemapsPlugin(); + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); // @ts-expect-error this function exists! expect(plugin.closeBundle).not.toThrow(); @@ -124,4 +140,100 @@ describe('makeCustomSentryVitePlugin()', () => { expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('Failed to upload source maps')); expect(consoleLogSpy).toHaveBeenCalled(); }); + + describe('Custom release management plugin', () => { + it('has the expected hooks and properties', async () => { + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-release-management-plugin'); + + expect(plugin).toEqual({ + name: 'sentry-sveltekit-release-management-plugin', + apply: 'build', + enforce: 'post', + closeBundle: expect.any(Function), + }); + }); + + it('calls the original release management plugin to start the release creation pipeline', async () => { + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-release-management-plugin'); + // @ts-expect-error this function exists! + await plugin.closeBundle(); + expect(mockedViteReleaseManagementPlugin.writeBundle).toHaveBeenCalledTimes(1); + }); + + it('catches errors during release creation', async () => { + mockedViteReleaseManagementPlugin.writeBundle.mockImplementationOnce(() => { + throw new Error('test error'); + }); + + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementationOnce(() => {}); + + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-release-management-plugin'); + + // @ts-expect-error this function exists! + expect(plugin.closeBundle).not.toThrow(); + + // @ts-expect-error this function exists! + await plugin.closeBundle(); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Failed to upload release data'), + expect.any(Error), + ); + }); + + it('also works correctly if the original release management plugin has its old name', async () => { + const currentName = mockedViteReleaseManagementPlugin.name; + mockedViteReleaseManagementPlugin.name = 'sentry-debug-id-upload-plugin'; + + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-release-management-plugin'); + + // @ts-expect-error this function exists! + await plugin.closeBundle(); + + expect(mockedViteReleaseManagementPlugin.writeBundle).toHaveBeenCalledTimes(1); + + mockedViteReleaseManagementPlugin.name = currentName; + }); + }); + + describe('Custom file deletion plugin', () => { + it('has the expected hooks and properties', async () => { + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-file-deletion-plugin'); + + expect(plugin).toEqual({ + name: 'sentry-sveltekit-file-deletion-plugin', + apply: 'build', + enforce: 'post', + closeBundle: expect.any(Function), + }); + }); + + it('calls the original file deletion plugin to delete files', async () => { + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-file-deletion-plugin'); + // @ts-expect-error this function exists! + await plugin.closeBundle(); + expect(mockedFileDeletionPlugin.writeBundle).toHaveBeenCalledTimes(1); + }); + + it('catches errors during file deletion', async () => { + mockedFileDeletionPlugin.writeBundle.mockImplementationOnce(() => { + throw new Error('test error'); + }); + + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementationOnce(() => {}); + + const plugin = await getSentryViteSubPlugin('sentry-sveltekit-file-deletion-plugin'); + + // @ts-expect-error this function exists! + expect(plugin.closeBundle).not.toThrow(); + + // @ts-expect-error this function exists! + await plugin.closeBundle(); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Failed to delete source maps'), + expect.any(Error), + ); + }); + }); });