From e66910d2e0d4d7d60da275daed523a9f5e9e88b6 Mon Sep 17 00:00:00 2001 From: Paris Outen Date: Fri, 12 Jul 2024 19:06:09 +0800 Subject: [PATCH] fix: Do not delete files before all upload tasks executed (#572) Co-authored-by: Luca Forstner --- .../src/debug-id-upload.ts | 13 +++- packages/bundler-plugin-core/src/index.ts | 11 +-- .../src/plugins/release-management.ts | 13 +++- .../src/plugins/sourcemap-deletion.ts | 74 +++++++++---------- 4 files changed, 60 insertions(+), 51 deletions(-) diff --git a/packages/bundler-plugin-core/src/debug-id-upload.ts b/packages/bundler-plugin-core/src/debug-id-upload.ts index 7ff8f226..22f4b666 100644 --- a/packages/bundler-plugin-core/src/debug-id-upload.ts +++ b/packages/bundler-plugin-core/src/debug-id-upload.ts @@ -33,7 +33,7 @@ interface DebugIdUploadPluginOptions { silent: boolean; headers?: Record; }; - freeDependencyOnSourcemapFiles: () => void; + createDependencyOnSourcemapFiles: () => () => void; } export function createDebugIdUploadFunction({ @@ -47,8 +47,10 @@ 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", @@ -56,6 +58,10 @@ export function createDebugIdUploadFunction({ 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 --git a/packages/bundler-plugin-core/src/index.ts b/packages/bundler-plugin-core/src/index.ts index d7dc2585..794b67c9 100644 --- a/packages/bundler-plugin-core/src/index.ts +++ b/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((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 --git a/packages/bundler-plugin-core/src/plugins/release-management.ts b/packages/bundler-plugin-core/src/plugins/release-management.ts index d984cc4e..f088ea04 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; }; - 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 --git a/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts b/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts index 6ea38fb2..97d743f2 100644 --- a/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts +++ b/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; + waitUntilSourcemapFileDependenciesAreFreed: () => Promise; 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, }; }