fix: Always delete files when sourcemaps.filesToDeleteAfterUpload is set (#547)

Author: lforst

Diff for: packages/bundler-plugin-core/src/debug-id-upload.ts

@@ -24,7 +24,7 @@ interface DebugIdUploadPluginOptions {
   handleRecoverableError: (error: unknown) => void;
   sentryHub: Hub;
   sentryClient: NodeClient;
-  filesToDeleteAfterUpload?: string | string[];
+  deleteFilesUpForDeletion: () => Promise<void>;
   sentryCliOptions: {
     url: string;
     authToken: string;
@@ -47,7 +47,7 @@ export function createDebugIdUploadFunction({
   sentryClient,
   sentryCliOptions,
   rewriteSourcesHook,
-  filesToDeleteAfterUpload,
+  deleteFilesUpForDeletion,
 }: DebugIdUploadPluginOptions) {
   return async (buildArtifactPaths: string[]) => {
     const artifactBundleUploadTransaction = sentryHub.startTransaction({
@@ -180,36 +180,7 @@ export function createDebugIdUploadFunction({
         logger.info("Successfully uploaded source maps to Sentry");
       }
 
-      if (filesToDeleteAfterUpload) {
-        const deleteGlobSpan = artifactBundleUploadTransaction.startChild({
-          description: "delete-glob",
-        });
-        const filePathsToDelete = await glob(filesToDeleteAfterUpload, {
-          absolute: true,
-          nodir: true,
-        });
-        deleteGlobSpan.finish();
-
-        filePathsToDelete.forEach((filePathToDelete) => {
-          logger.debug(`Deleting asset after upload: ${filePathToDelete}`);
-        });
-
-        const deleteSpan = artifactBundleUploadTransaction.startChild({
-          description: "delete-files-after-upload",
-        });
-        await Promise.all(
-          filePathsToDelete.map((filePathToDelete) =>
-            fs.promises.rm(filePathToDelete, { force: true }).catch((e) => {
-              // This is allowed to fail - we just don't do anything
-              logger.debug(
-                `An error occured while attempting to delete asset: ${filePathToDelete}`,
-                e
-              );
-            })
-          )
-        );
-        deleteSpan.finish();
-      }
+      await deleteFilesUpForDeletion();
     } catch (e) {
       sentryHub.withScope((scope) => {
         scope.setSpan(artifactBundleUploadTransaction);

Diff for: packages/bundler-plugin-core/src/index.ts

@@ -25,6 +25,7 @@ import {
 import * as dotenv from "dotenv";
 import { glob } from "glob";
 import { logger } from "@sentry/utils";
+import { fileDeletionPlugin } from "./plugins/sourcemap-deletion";
 
 interface SentryUnpluginFactoryOptions {
   releaseInjectionPlugin: (injectionCode: string) => UnpluginOptions;
@@ -184,6 +185,34 @@ export function sentryUnpluginFactory({
       })
     );
 
+    async function deleteFilesUpForDeletion() {
+      const filesToDeleteAfterUpload =
+        options.sourcemaps?.filesToDeleteAfterUpload ?? options.sourcemaps?.deleteFilesAfterUpload;
+
+      if (filesToDeleteAfterUpload) {
+        const filePathsToDelete = await glob(filesToDeleteAfterUpload, {
+          absolute: true,
+          nodir: true,
+        });
+
+        filePathsToDelete.forEach((filePathToDelete) => {
+          logger.debug(`Deleting asset after upload: ${filePathToDelete}`);
+        });
+
+        await Promise.all(
+          filePathsToDelete.map((filePathToDelete) =>
+            fs.promises.rm(filePathToDelete, { force: true }).catch((e) => {
+              // This is allowed to fail - we just don't do anything
+              logger.debug(
+                `An error occurred while attempting to delete asset: ${filePathToDelete}`,
+                e
+              );
+            })
+          )
+        );
+      }
+    }
+
     if (options.bundleSizeOptimizations) {
       const { bundleSizeOptimizations } = options;
       const replacementValues: SentrySDKBuildFlags = {};
@@ -297,12 +326,22 @@ export function sentryUnpluginFactory({
             vcsRemote: options.release.vcsRemote,
             headers: options.headers,
           },
+          deleteFilesUpForDeletion,
         })
       );
     }
 
     plugins.push(debugIdInjectionPlugin(logger));
 
+    plugins.push(
+      fileDeletionPlugin({
+        deleteFilesUpForDeletion,
+        handleRecoverableError,
+        sentryHub,
+        sentryClient,
+      })
+    );
+
     if (!options.authToken) {
       logger.warn(
         "No auth token provided. Will not upload source maps. Please set the `authToken` option. You can find information on how to generate a Sentry auth token here: https://docs.sentry.io/api/auth/"
@@ -321,9 +360,7 @@ export function sentryUnpluginFactory({
           createDebugIdUploadFunction({
             assets: options.sourcemaps?.assets,
             ignore: options.sourcemaps?.ignore,
-            filesToDeleteAfterUpload:
-              options.sourcemaps?.filesToDeleteAfterUpload ??
-              options.sourcemaps?.deleteFilesAfterUpload,
+            deleteFilesUpForDeletion,
             dist: options.release.dist,
             releaseName: options.release.name,
             logger: logger,

Diff for: packages/bundler-plugin-core/src/plugins/release-management.ts

@@ -27,6 +27,7 @@ interface ReleaseManagementPluginOptions {
     silent: boolean;
     headers?: Record<string, string>;
   };
+  deleteFilesUpForDeletion: () => Promise<void>;
 }
 
 export function releaseManagementPlugin({
@@ -41,6 +42,7 @@ export function releaseManagementPlugin({
   sentryHub,
   sentryClient,
   sentryCliOptions,
+  deleteFilesUpForDeletion,
 }: ReleaseManagementPluginOptions): UnpluginOptions {
   return {
     name: "sentry-debug-id-upload-plugin",
@@ -83,6 +85,8 @@ export function releaseManagementPlugin({
         if (deployOptions) {
           await cliInstance.releases.newDeploy(releaseName, deployOptions);
         }
+
+        await deleteFilesUpForDeletion();
       } catch (e) {
         sentryHub.captureException('Error in "releaseManagementPlugin" writeBundle hook');
         await safeFlushTelemetry(sentryClient);

Diff for: packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts

@@ -0,0 +1,30 @@
+import { Hub, NodeClient } from "@sentry/node";
+import { UnpluginOptions } from "unplugin";
+import { safeFlushTelemetry } from "../sentry/telemetry";
+
+interface FileDeletionPlugin {
+  handleRecoverableError: (error: unknown) => void;
+  deleteFilesUpForDeletion: () => Promise<void>;
+  sentryHub: Hub;
+  sentryClient: NodeClient;
+}
+
+export function fileDeletionPlugin({
+  handleRecoverableError,
+  sentryHub,
+  sentryClient,
+  deleteFilesUpForDeletion,
+}: FileDeletionPlugin): UnpluginOptions {
+  return {
+    name: "sentry-file-deletion-plugin",
+    async writeBundle() {
+      try {
+        await deleteFilesUpForDeletion();
+      } catch (e) {
+        sentryHub.captureException('Error in "sentry-file-deletion-plugin" buildEnd hook');
+        await safeFlushTelemetry(sentryClient);
+        handleRecoverableError(e);
+      }
+    },
+  };
+}

Diff for: packages/integration-tests/fixtures/after-upload-deletion/after-upload-deletion.test.ts

@@ -0,0 +1,27 @@
+/* eslint-disable jest/no-standalone-expect */
+/* eslint-disable jest/expect-expect */
+import path from "path";
+import fs from "fs";
+import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";
+
+describe("Deletes with `filesToDeleteAfterUpload` even without uploading anything", () => {
+  testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
+    expect(fs.existsSync(path.join(__dirname, "out", "webpack4", "bundle.js.map"))).toBe(false);
+  });
+
+  test("webpack 5 bundle", () => {
+    expect(fs.existsSync(path.join(__dirname, "out", "webpack5", "bundle.js.map"))).toBe(false);
+  });
+
+  test("esbuild bundle", () => {
+    expect(fs.existsSync(path.join(__dirname, "out", "esbuild", "bundle.js.map"))).toBe(false);
+  });
+
+  test("rollup bundle", () => {
+    expect(fs.existsSync(path.join(__dirname, "out", "rollup", "bundle.js.map"))).toBe(false);
+  });
+
+  test("vite bundle", () => {
+    expect(fs.existsSync(path.join(__dirname, "out", "vite", "bundle.js.map"))).toBe(false);
+  });
+});

Diff for: packages/integration-tests/fixtures/after-upload-deletion/input/bundle.js

@@ -0,0 +1,2 @@
+// eslint-disable-next-line no-console
+console.log("whatever");

Diff for: packages/integration-tests/fixtures/after-upload-deletion/setup.ts

@@ -0,0 +1,19 @@
+import * as path from "path";
+import { createCjsBundles } from "../../utils/create-cjs-bundles";
+
+const outputDir = path.resolve(__dirname, "out");
+
+["webpack4", "webpack5", "esbuild", "rollup", "vite"].forEach((bundler) => {
+  createCjsBundles(
+    {
+      bundle: path.resolve(__dirname, "input", "bundle.js"),
+    },
+    outputDir,
+    {
+      sourcemaps: {
+        filesToDeleteAfterUpload: [path.join(__dirname, "out", bundler, "bundle.js.map")],
+      },
+    },
+    [bundler]
+  );
+});

Diff for: packages/integration-tests/utils/create-cjs-bundles.ts

@@ -23,6 +23,7 @@ export function createCjsBundles(
     void vite.build({
       clearScreen: false,
       build: {
+        sourcemap: true,
         outDir: path.join(outFolder, "vite"),
         rollupOptions: {
           input: entrypoints,
@@ -43,6 +44,7 @@ export function createCjsBundles(
       })
       .then((bundle) =>
         bundle.write({
+          sourcemap: true,
           dir: path.join(outFolder, "rollup"),
           format: "cjs",
           exports: "named",
@@ -52,6 +54,7 @@ export function createCjsBundles(
 
   if (plugins.length === 0 || plugins.includes("esbuild")) {
     void esbuild.build({
+      sourcemap: true,
       entryPoints: entrypoints,
       outdir: path.join(outFolder, "esbuild"),
       plugins: [sentryEsbuildPlugin(sentryUnpluginOptions)],
@@ -65,6 +68,7 @@ export function createCjsBundles(
   if (parseInt(nodejsMajorversion) < 18 && (plugins.length === 0 || plugins.includes("webpack4"))) {
     webpack4(
       {
+        devtool: "source-map",
         mode: "production",
         entry: entrypoints,
         cache: false,
@@ -86,6 +90,7 @@ export function createCjsBundles(
   if (plugins.length === 0 || plugins.includes("webpack5")) {
     webpack5(
       {
+        devtool: "source-map",
         cache: false,
         entry: entrypoints,
         output: {