From 4621e22af97fa935614f708e8797bc8391221eb9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 11 Jun 2024 13:42:50 +0200 Subject: [PATCH 1/9] fix: Always delete files when `sourcemaps.filesToDeleteAfterUpload` is set --- .../src/debug-id-upload.ts | 35 ++------------- packages/bundler-plugin-core/src/index.ts | 43 +++++++++++++++++-- .../src/plugins/release-management.ts | 4 ++ .../src/plugins/sourcemap-deletion.ts | 30 +++++++++++++ 4 files changed, 77 insertions(+), 35 deletions(-) create mode 100644 packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts diff --git a/packages/bundler-plugin-core/src/debug-id-upload.ts b/packages/bundler-plugin-core/src/debug-id-upload.ts index 1e7b4822..9f38a371 100644 --- a/packages/bundler-plugin-core/src/debug-id-upload.ts +++ b/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; 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 --git a/packages/bundler-plugin-core/src/index.ts b/packages/bundler-plugin-core/src/index.ts index a04b03fa..70362cb0 100644 --- a/packages/bundler-plugin-core/src/index.ts +++ b/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; @@ -180,6 +181,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 = {}; @@ -293,12 +322,22 @@ export function sentryUnpluginFactory({ vcsRemote: options.release.vcsRemote, headers: options.headers, }, + deleteFilesUpForDeletion, }) ); } plugins.push(debugIdInjectionPlugin(logger)); + plugins.push( + fileDeletionPlugin({ + deleteFilesUpForDeletion, + handleRecoverableError: 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/" @@ -317,9 +356,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 --git a/packages/bundler-plugin-core/src/plugins/release-management.ts b/packages/bundler-plugin-core/src/plugins/release-management.ts index dd905acb..acf73870 100644 --- a/packages/bundler-plugin-core/src/plugins/release-management.ts +++ b/packages/bundler-plugin-core/src/plugins/release-management.ts @@ -27,6 +27,7 @@ interface ReleaseManagementPluginOptions { silent: boolean; headers?: Record; }; + deleteFilesUpForDeletion: () => Promise; } 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 --git a/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts b/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts new file mode 100644 index 00000000..6fd1e625 --- /dev/null +++ b/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; + sentryHub: Hub; + sentryClient: NodeClient; +} + +export function fileDeletionPlugin({ + handleRecoverableError, + sentryHub, + sentryClient, + deleteFilesUpForDeletion, +}: FileDeletionPlugin): UnpluginOptions { + return { + name: "sentry-file-deletion-plugin", + async buildEnd() { + try { + await deleteFilesUpForDeletion(); + } catch (e) { + sentryHub.captureException('Error in "sentry-file-deletion-plugin" buildEnd hook'); + await safeFlushTelemetry(sentryClient); + handleRecoverableError(e); + } + }, + }; +} From 9eb04e169b957059fa99e58f0ea58add30a6e6c6 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 17 Jun 2024 14:12:05 +0200 Subject: [PATCH 2/9] Add test --- .../after-upload-deletion.test.ts | 27 +++++++++++++++++++ .../after-upload-deletion/input/bundle.js | 2 ++ .../fixtures/after-upload-deletion/setup.ts | 19 +++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 packages/integration-tests/fixtures/after-upload-deletion/after-upload-deletion.test.ts create mode 100644 packages/integration-tests/fixtures/after-upload-deletion/input/bundle.js create mode 100644 packages/integration-tests/fixtures/after-upload-deletion/setup.ts diff --git a/packages/integration-tests/fixtures/after-upload-deletion/after-upload-deletion.test.ts b/packages/integration-tests/fixtures/after-upload-deletion/after-upload-deletion.test.ts new file mode 100644 index 00000000..cf095502 --- /dev/null +++ b/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 --git a/packages/integration-tests/fixtures/after-upload-deletion/input/bundle.js b/packages/integration-tests/fixtures/after-upload-deletion/input/bundle.js new file mode 100644 index 00000000..aa70f660 --- /dev/null +++ b/packages/integration-tests/fixtures/after-upload-deletion/input/bundle.js @@ -0,0 +1,2 @@ +// eslint-disable-next-line no-console +console.log("whatever"); diff --git a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts new file mode 100644 index 00000000..76ef7bb0 --- /dev/null +++ b/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, "*.map"), + }, + }, + [bundler] + ); +}); From ae807c6ba8c51fb9450071c306a40662156395cd Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 17 Jun 2024 14:13:43 +0200 Subject: [PATCH 3/9] sourcemaps --- packages/integration-tests/utils/create-cjs-bundles.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/integration-tests/utils/create-cjs-bundles.ts b/packages/integration-tests/utils/create-cjs-bundles.ts index 1569b3ea..a03916cf 100644 --- a/packages/integration-tests/utils/create-cjs-bundles.ts +++ b/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: { From bc377f2ab9dc76191338a38d548242cd9c7f0e88 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 17 Jun 2024 14:21:51 +0200 Subject: [PATCH 4/9] Fix by using writeBundle --- packages/bundler-plugin-core/src/index.ts | 2 +- packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/bundler-plugin-core/src/index.ts b/packages/bundler-plugin-core/src/index.ts index 70362cb0..3e43dfe8 100644 --- a/packages/bundler-plugin-core/src/index.ts +++ b/packages/bundler-plugin-core/src/index.ts @@ -332,7 +332,7 @@ export function sentryUnpluginFactory({ plugins.push( fileDeletionPlugin({ deleteFilesUpForDeletion, - handleRecoverableError: handleRecoverableError, + handleRecoverableError, sentryHub, sentryClient, }) diff --git a/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts b/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts index 6fd1e625..032deade 100644 --- a/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts +++ b/packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts @@ -17,7 +17,7 @@ export function fileDeletionPlugin({ }: FileDeletionPlugin): UnpluginOptions { return { name: "sentry-file-deletion-plugin", - async buildEnd() { + async writeBundle() { try { await deleteFilesUpForDeletion(); } catch (e) { From 8b49159165b0bd2926fcef915415372b50d9283a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 17 Jun 2024 17:26:06 +0200 Subject: [PATCH 5/9] maybee --- .../integration-tests/fixtures/after-upload-deletion/setup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts index 76ef7bb0..5629ad24 100644 --- a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts +++ b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts @@ -11,7 +11,7 @@ const outputDir = path.resolve(__dirname, "out"); outputDir, { sourcemaps: { - filesToDeleteAfterUpload: path.join(__dirname, "out", bundler, "*.map"), + filesToDeleteAfterUpload: path.join(".", "**", "after-upload-deletion", "**", "*.map"), }, }, [bundler] From da4d932c9e226815ce31d0bbb11f0ec3c83af9e1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 17 Jun 2024 17:34:42 +0200 Subject: [PATCH 6/9] debug --- .../integration-tests/fixtures/after-upload-deletion/setup.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts index 5629ad24..d34af224 100644 --- a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts +++ b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts @@ -10,6 +10,7 @@ const outputDir = path.resolve(__dirname, "out"); }, outputDir, { + debug: true, sourcemaps: { filesToDeleteAfterUpload: path.join(".", "**", "after-upload-deletion", "**", "*.map"), }, @@ -17,3 +18,5 @@ const outputDir = path.resolve(__dirname, "out"); [bundler] ); }); + +console.log("ASDFASDF", path.join(".", "**", "after-upload-deletion", "**", "*.map")); From 9ec00d8f9c709dea3f770ba30c5f6665f0cb81ba Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 17 Jun 2024 17:35:12 +0200 Subject: [PATCH 7/9] debug --- .../fixtures/after-upload-deletion/setup.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts index d34af224..00b9f2fa 100644 --- a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts +++ b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts @@ -19,4 +19,9 @@ const outputDir = path.resolve(__dirname, "out"); ); }); -console.log("ASDFASDF", path.join(".", "**", "after-upload-deletion", "**", "*.map")); +console.log( + "ASDFASDF", + __dirname, + process.cwd(), + path.join(".", "**", "after-upload-deletion", "**", "*.map") +); From 09ad82f1f7734b76152426478d394cbbf8cb71a2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 17 Jun 2024 17:50:26 +0200 Subject: [PATCH 8/9] maybe --- .../fixtures/after-upload-deletion/setup.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts index 00b9f2fa..8e1883be 100644 --- a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts +++ b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts @@ -10,18 +10,10 @@ const outputDir = path.resolve(__dirname, "out"); }, outputDir, { - debug: true, sourcemaps: { - filesToDeleteAfterUpload: path.join(".", "**", "after-upload-deletion", "**", "*.map"), + filesToDeleteAfterUpload: path.join("./**/after-upload-deletion/**/*.map"), }, }, [bundler] ); }); - -console.log( - "ASDFASDF", - __dirname, - process.cwd(), - path.join(".", "**", "after-upload-deletion", "**", "*.map") -); From 80a4d5bb4ae111e90816e7001ed02499dd513bc1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 19 Jun 2024 12:00:19 +0200 Subject: [PATCH 9/9] womp womp --- .../integration-tests/fixtures/after-upload-deletion/setup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts index 8e1883be..e28f10b9 100644 --- a/packages/integration-tests/fixtures/after-upload-deletion/setup.ts +++ b/packages/integration-tests/fixtures/after-upload-deletion/setup.ts @@ -11,7 +11,7 @@ const outputDir = path.resolve(__dirname, "out"); outputDir, { sourcemaps: { - filesToDeleteAfterUpload: path.join("./**/after-upload-deletion/**/*.map"), + filesToDeleteAfterUpload: [path.join(__dirname, "out", bundler, "bundle.js.map")], }, }, [bundler]