fix: Do not delete files before all upload tasks executed (#572)

Co-authored-by: Luca Forstner <[email protected]>

Author: tyouzu1

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

@@ -33,7 +33,7 @@ interface DebugIdUploadPluginOptions {
     silent: boolean;
     headers?: Record<string, string>;
   };
-  freeDependencyOnSourcemapFiles: () => void;
+  createDependencyOnSourcemapFiles: () => () => void;
 }
 
 export function createDebugIdUploadFunction({
@@ -47,15 +47,21 @@ export function createDebugIdUploadFunction({
   sentryClient,
   sentryCliOptions,
   rewriteSourcesHook,
-  freeDependencyOnSourcemapFiles,
+  createDependencyOnSourcemapFiles,
 }: DebugIdUploadPluginOptions) {
+  const freeGlobalDependencyOnSourcemapFiles = createDependencyOnSourcemapFiles();
+
   return async (buildArtifactPaths: string[]) => {
     const artifactBundleUploadTransaction = sentryHub.startTransaction({
       name: "debug-id-sourcemap-upload",
     });
 
     let folderToCleanUp: string | undefined;
 
+    // It is possible that this writeBundle hook (which calls this function) is called multiple times in one build (for example when reusing the plugin, or when using build tooling like `@vitejs/plugin-legacy`)
+    // Therefore we need to actually register the execution of this hook as dependency on the sourcemap files.
+    const freeUploadDependencyOnSourcemapFiles = createDependencyOnSourcemapFiles();
+
     try {
       const mkdtempSpan = artifactBundleUploadTransaction.startChild({ description: "mkdtemp" });
       const tmpUploadFolder = await fs.promises.mkdtemp(
@@ -194,7 +200,8 @@ export function createDebugIdUploadFunction({
         cleanupSpan.finish();
       }
       artifactBundleUploadTransaction.finish();
-      freeDependencyOnSourcemapFiles();
+      freeGlobalDependencyOnSourcemapFiles();
+      freeUploadDependencyOnSourcemapFiles();
       await safeFlushTelemetry(sentryClient);
     }
   };

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

@@ -212,6 +212,9 @@ export function sentryUnpluginFactory({
 
     /**
      * Returns a Promise that resolves when all the currently active dependencies are freed again.
+     *
+     * It is very important that this function is called as late as possible before wanting to await the Promise to give
+     * the dependency producers as much time as possible to register themselves.
      */
     function waitUntilSourcemapFileDependenciesAreFreed() {
       return new Promise<void>((resolve) => {
@@ -342,7 +345,7 @@ export function sentryUnpluginFactory({
             vcsRemote: options.release.vcsRemote,
             headers: options.headers,
           },
-          freeDependencyOnSourcemapFiles: createDependencyOnSourcemapFiles(),
+          createDependencyOnSourcemapFiles,
         })
       );
     }
@@ -367,7 +370,7 @@ export function sentryUnpluginFactory({
           createDebugIdUploadFunction({
             assets: options.sourcemaps?.assets,
             ignore: options.sourcemaps?.ignore,
-            freeDependencyOnSourcemapFiles: createDependencyOnSourcemapFiles(),
+            createDependencyOnSourcemapFiles,
             dist: options.release.dist,
             releaseName: options.release.name,
             logger: logger,
@@ -405,9 +408,7 @@ export function sentryUnpluginFactory({
 
     plugins.push(
       fileDeletionPlugin({
-        // It is very important that this is only called after all other dependencies have been created with `createDependencyOnSourcemapFiles`.
-        // Ideally, we always register this plugin last.
-        dependenciesAreFreedPromise: waitUntilSourcemapFileDependenciesAreFreed(),
+        waitUntilSourcemapFileDependenciesAreFreed,
         filesToDeleteAfterUpload:
           options.sourcemaps?.filesToDeleteAfterUpload ??
           options.sourcemaps?.deleteFilesAfterUpload,

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

@@ -27,7 +27,7 @@ interface ReleaseManagementPluginOptions {
     silent: boolean;
     headers?: Record<string, string>;
   };
-  freeDependencyOnSourcemapFiles: () => void;
+  createDependencyOnSourcemapFiles: () => () => void;
 }
 
 export function releaseManagementPlugin({
@@ -42,11 +42,17 @@ export function releaseManagementPlugin({
   sentryHub,
   sentryClient,
   sentryCliOptions,
-  freeDependencyOnSourcemapFiles,
+  createDependencyOnSourcemapFiles,
 }: ReleaseManagementPluginOptions): UnpluginOptions {
+  const freeGlobalDependencyOnSourcemapFiles = createDependencyOnSourcemapFiles();
   return {
     name: "sentry-debug-id-upload-plugin",
     async writeBundle() {
+      // It is possible that this writeBundle hook is called multiple times in one build (for example when reusing the plugin, or when using build tooling like `@vitejs/plugin-legacy`)
+      // Therefore we need to actually register the execution of this hook as dependency on the sourcemap files.
+      const freeWriteBundleInvocationDependencyOnSourcemapFiles =
+        createDependencyOnSourcemapFiles();
+
       try {
         const cliInstance = new SentryCli(null, sentryCliOptions);
 
@@ -90,7 +96,8 @@ export function releaseManagementPlugin({
         await safeFlushTelemetry(sentryClient);
         handleRecoverableError(e);
       } finally {
-        freeDependencyOnSourcemapFiles();
+        freeGlobalDependencyOnSourcemapFiles();
+        freeWriteBundleInvocationDependencyOnSourcemapFiles();
       }
     },
   };

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

@@ -7,7 +7,7 @@ import fs from "fs";
 
 interface FileDeletionPlugin {
   handleRecoverableError: (error: unknown) => void;
-  dependenciesAreFreedPromise: Promise<void>;
+  waitUntilSourcemapFileDependenciesAreFreed: () => Promise<void>;
   sentryHub: Hub;
   sentryClient: NodeClient;
   filesToDeleteAfterUpload: string | string[] | undefined;
@@ -19,52 +19,46 @@ export function fileDeletionPlugin({
   sentryHub,
   sentryClient,
   filesToDeleteAfterUpload,
-  dependenciesAreFreedPromise,
+  waitUntilSourcemapFileDependenciesAreFreed,
   logger,
 }: FileDeletionPlugin): UnpluginOptions {
-  const writeBundle = async () => {
-    try {
-      if (filesToDeleteAfterUpload !== undefined) {
-        const filePathsToDelete = await glob(filesToDeleteAfterUpload, {
-          absolute: true,
-          nodir: true,
-        });
+  return {
+    name: "sentry-file-deletion-plugin",
+    async writeBundle() {
+      try {
+        if (filesToDeleteAfterUpload !== undefined) {
+          const filePathsToDelete = await glob(filesToDeleteAfterUpload, {
+            absolute: true,
+            nodir: true,
+          });
 
-        logger.debug("Waiting for dependencies on generated files to be freed before deleting...");
+          logger.debug(
+            "Waiting for dependencies on generated files to be freed before deleting..."
+          );
 
-        await dependenciesAreFreedPromise;
+          await waitUntilSourcemapFileDependenciesAreFreed();
 
-        filePathsToDelete.forEach((filePathToDelete) => {
-          logger.debug(`Deleting asset after upload: ${filePathToDelete}`);
-        });
+          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
-              );
-            })
-          )
-        );
+          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
+                );
+              })
+            )
+          );
+        }
+      } catch (e) {
+        sentryHub.captureException('Error in "sentry-file-deletion-plugin" buildEnd hook');
+        await safeFlushTelemetry(sentryClient);
+        handleRecoverableError(e);
       }
-    } catch (e) {
-      sentryHub.captureException('Error in "sentry-file-deletion-plugin" buildEnd hook');
-      await safeFlushTelemetry(sentryClient);
-      handleRecoverableError(e);
-    }
-  };
-  return {
-    name: "sentry-file-deletion-plugin",
-    vite: {
-      writeBundle: {
-        sequential: true,
-        order: "post",
-        handler: writeBundle,
-      },
     },
-    writeBundle,
   };
 }