fix(v8/sveltekit): Ensure source maps deletion is called after source ma… (#14963)

Lms24 · web-flow · commit af00c8f8a6f8 · 2025-01-09T17:26:43.000+01:00
Backport of #14942
diff --git 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
@@ -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
@@ -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<Plugin | undefined> {
+async function getSentryViteSubPlugin(name: string): Promise<Plugin | undefined> {
   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),
+      );
+    });
+  });
 });
