diff --git a/packages/bundler-plugin-core/src/debug-id-upload.ts b/packages/bundler-plugin-core/src/debug-id-upload.ts index 9f38a371..7ff8f226 100644 --- a/packages/bundler-plugin-core/src/debug-id-upload.ts +++ b/packages/bundler-plugin-core/src/debug-id-upload.ts @@ -24,7 +24,6 @@ interface DebugIdUploadPluginOptions { handleRecoverableError: (error: unknown) => void; sentryHub: Hub; sentryClient: NodeClient; - deleteFilesUpForDeletion: () => Promise; sentryCliOptions: { url: string; authToken: string; @@ -34,6 +33,7 @@ interface DebugIdUploadPluginOptions { silent: boolean; headers?: Record; }; + freeDependencyOnSourcemapFiles: () => void; } export function createDebugIdUploadFunction({ @@ -47,7 +47,7 @@ export function createDebugIdUploadFunction({ sentryClient, sentryCliOptions, rewriteSourcesHook, - deleteFilesUpForDeletion, + freeDependencyOnSourcemapFiles, }: DebugIdUploadPluginOptions) { return async (buildArtifactPaths: string[]) => { const artifactBundleUploadTransaction = sentryHub.startTransaction({ @@ -179,8 +179,6 @@ export function createDebugIdUploadFunction({ uploadSpan.finish(); logger.info("Successfully uploaded source maps to Sentry"); } - - await deleteFilesUpForDeletion(); } catch (e) { sentryHub.withScope((scope) => { scope.setSpan(artifactBundleUploadTransaction); @@ -196,6 +194,7 @@ export function createDebugIdUploadFunction({ cleanupSpan.finish(); } artifactBundleUploadTransaction.finish(); + freeDependencyOnSourcemapFiles(); await safeFlushTelemetry(sentryClient); } }; diff --git a/packages/bundler-plugin-core/src/index.ts b/packages/bundler-plugin-core/src/index.ts index 2780524c..8f91ced2 100644 --- a/packages/bundler-plugin-core/src/index.ts +++ b/packages/bundler-plugin-core/src/index.ts @@ -185,32 +185,46 @@ 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, - }); + // We have multiple plugins depending on generated source map files. (debug ID upload, legacy upload) + // Additionally, we also want to have the functionality to delete files after uploading sourcemaps. + // All of these plugins and the delete functionality need to run in the same hook (`writeBundle`). + // Since the plugins among themselves are not aware of when they run and finish, we need a system to + // track their dependencies on the generated files, so that we can initiate the file deletion only after + // nothing depends on the files anymore. + const dependenciesOnSourcemapFiles = new Set(); + const sourcemapFileDependencySubscribers: (() => void)[] = []; + + function notifySourcemapFileDependencySubscribers() { + sourcemapFileDependencySubscribers.forEach((subscriber) => { + subscriber(); + }); + } + + function createDependencyOnSourcemapFiles() { + const dependencyIdentifier = Symbol(); + dependenciesOnSourcemapFiles.add(dependencyIdentifier); - filePathsToDelete.forEach((filePathToDelete) => { - logger.debug(`Deleting asset after upload: ${filePathToDelete}`); + return function freeDependencyOnSourcemapFiles() { + dependenciesOnSourcemapFiles.delete(dependencyIdentifier); + notifySourcemapFileDependencySubscribers(); + }; + } + + /** + * Returns a Promise that resolves when all the currently active dependencies are freed again. + */ + function waitUntilSourcemapFileDependenciesAreFreed() { + return new Promise((resolve) => { + sourcemapFileDependencySubscribers.push(() => { + if (dependenciesOnSourcemapFiles.size === 0) { + resolve(); + } }); - 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 (dependenciesOnSourcemapFiles.size === 0) { + resolve(); + } + }); } if (options.bundleSizeOptimizations) { @@ -326,22 +340,13 @@ export function sentryUnpluginFactory({ vcsRemote: options.release.vcsRemote, headers: options.headers, }, - deleteFilesUpForDeletion, + freeDependencyOnSourcemapFiles: createDependencyOnSourcemapFiles(), }) ); } 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/" @@ -360,7 +365,7 @@ export function sentryUnpluginFactory({ createDebugIdUploadFunction({ assets: options.sourcemaps?.assets, ignore: options.sourcemaps?.ignore, - deleteFilesUpForDeletion, + freeDependencyOnSourcemapFiles: createDependencyOnSourcemapFiles(), dist: options.release.dist, releaseName: options.release.name, logger: logger, @@ -396,6 +401,21 @@ 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(), + filesToDeleteAfterUpload: + options.sourcemaps?.filesToDeleteAfterUpload ?? + options.sourcemaps?.deleteFilesAfterUpload, + logger, + handleRecoverableError, + sentryHub, + sentryClient, + }) + ); + return plugins; }); } diff --git a/packages/bundler-plugin-core/src/plugins/release-management.ts b/packages/bundler-plugin-core/src/plugins/release-management.ts index acf73870..d984cc4e 100644 --- a/packages/bundler-plugin-core/src/plugins/release-management.ts +++ b/packages/bundler-plugin-core/src/plugins/release-management.ts @@ -27,7 +27,7 @@ interface ReleaseManagementPluginOptions { silent: boolean; headers?: Record; }; - deleteFilesUpForDeletion: () => Promise; + freeDependencyOnSourcemapFiles: () => void; } export function releaseManagementPlugin({ @@ -42,7 +42,7 @@ export function releaseManagementPlugin({ sentryHub, sentryClient, sentryCliOptions, - deleteFilesUpForDeletion, + freeDependencyOnSourcemapFiles, }: ReleaseManagementPluginOptions): UnpluginOptions { return { name: "sentry-debug-id-upload-plugin", @@ -85,12 +85,12 @@ 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); handleRecoverableError(e); + } finally { + freeDependencyOnSourcemapFiles(); } }, }; diff --git a/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts b/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts index 032deade..53043df0 100644 --- a/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts +++ b/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts @@ -1,25 +1,59 @@ import { Hub, NodeClient } from "@sentry/node"; +import { glob } from "glob"; import { UnpluginOptions } from "unplugin"; +import { Logger } from "../sentry/logger"; import { safeFlushTelemetry } from "../sentry/telemetry"; +import fs from "fs"; interface FileDeletionPlugin { handleRecoverableError: (error: unknown) => void; - deleteFilesUpForDeletion: () => Promise; + dependenciesAreFreedPromise: Promise; sentryHub: Hub; sentryClient: NodeClient; + filesToDeleteAfterUpload: string | string[] | undefined; + logger: Logger; } export function fileDeletionPlugin({ handleRecoverableError, sentryHub, sentryClient, - deleteFilesUpForDeletion, + filesToDeleteAfterUpload, + dependenciesAreFreedPromise, + logger, }: FileDeletionPlugin): UnpluginOptions { return { name: "sentry-file-deletion-plugin", async writeBundle() { try { - await deleteFilesUpForDeletion(); + 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..." + ); + + await dependenciesAreFreedPromise; + + 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 + ); + }) + ) + ); + } } catch (e) { sentryHub.captureException('Error in "sentry-file-deletion-plugin" buildEnd hook'); await safeFlushTelemetry(sentryClient);