diff --git a/packages/bundler-plugin-core/src/build-plugin-manager.ts b/packages/bundler-plugin-core/src/build-plugin-manager.ts index 4f5e5193..8aa01e8a 100644 --- a/packages/bundler-plugin-core/src/build-plugin-manager.ts +++ b/packages/bundler-plugin-core/src/build-plugin-manager.ts @@ -77,7 +77,10 @@ export type SentryBuildPluginManager = { /** * Uploads sourcemaps using the "Debug ID" method. This function takes a list of build artifact paths that will be uploaded */ - uploadSourcemaps(buildArtifactPaths: string[]): Promise; + uploadSourcemaps( + buildArtifactPaths: string[], + opts?: { prepareArtifacts?: boolean } + ): Promise; /** * Will delete artifacts based on the passed `sourcemaps.filesToDeleteAfterUpload` option. @@ -546,9 +549,16 @@ export function createSentryBuildPluginManager( }, /** - * Uploads sourcemaps using the "Debug ID" method. This function takes a list of build artifact paths that will be uploaded + * Uploads sourcemaps using the "Debug ID" method. + * + * By default, this prepares bundles in a temporary folder before uploading. You can opt into an + * in-place, direct upload path by setting `prepareArtifacts` to `false`. If `prepareArtifacts` is set to + * `false`, no preparation (e.g. adding `//# debugId=...` and writing adjusted source maps) is performed and no temp folder is used. + * + * @param buildArtifactPaths - The paths of the build artifacts to upload + * @param opts - Optional flags to control temp folder usage and preparation */ - async uploadSourcemaps(buildArtifactPaths: string[]) { + async uploadSourcemaps(buildArtifactPaths: string[], opts?: { prepareArtifacts?: boolean }) { if (!canUploadSourceMaps(options, logger, isDevMode)) { return; } @@ -557,6 +567,9 @@ export function createSentryBuildPluginManager( // This is `forceTransaction`ed because this span is used in dashboards in the form of indexed transactions. { name: "debug-id-sourcemap-upload", scope: sentryScope, forceTransaction: true }, async () => { + // If we're not using a temp folder, we must not prepare artifacts in-place (to avoid mutating user files) + const shouldPrepare = opts?.prepareArtifacts ?? true; + 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`) @@ -564,19 +577,6 @@ export function createSentryBuildPluginManager( const freeUploadDependencyOnBuildArtifacts = createDependencyOnBuildArtifacts(); try { - const tmpUploadFolder = await startSpan( - { name: "mkdtemp", scope: sentryScope }, - async () => { - return ( - process.env?.["SENTRY_TEST_OVERRIDE_TEMP_DIR"] || - (await fs.promises.mkdtemp( - path.join(os.tmpdir(), "sentry-bundler-plugin-upload-") - )) - ); - } - ); - - folderToCleanUp = tmpUploadFolder; const assets = options.sourcemaps?.assets; let globAssets: string | string[]; @@ -594,14 +594,17 @@ export function createSentryBuildPluginManager( async () => await glob(globAssets, { absolute: true, - nodir: true, + // If we do not use a temp folder, we allow directories and files; CLI will traverse as needed when given paths. + nodir: shouldPrepare, ignore: options.sourcemaps?.ignore, }) ); - const debugIdChunkFilePaths = globResult.filter((debugIdChunkFilePath) => { - return !!stripQueryAndHashFromPath(debugIdChunkFilePath).match(/\.(js|mjs|cjs)$/); - }); + const debugIdChunkFilePaths = shouldPrepare + ? globResult.filter((debugIdChunkFilePath) => { + return !!stripQueryAndHashFromPath(debugIdChunkFilePath).match(/\.(js|mjs|cjs)$/); + }) + : globResult; // The order of the files output by glob() is not deterministic // Ensure order within the files so that {debug-id}-{chunkIndex} coupling is consistent @@ -616,75 +619,101 @@ export function createSentryBuildPluginManager( "Didn't find any matching sources for debug ID upload. Please check the `sourcemaps.assets` option." ); } else { - const numUploadedFiles = await startSpan( - { name: "prepare-bundles", scope: sentryScope }, - async (prepBundlesSpan) => { - // Preparing the bundles can be a lot of work and doing it all at once has the potential of nuking the heap so - // instead we do it with a maximum of 16 concurrent workers - const preparationTasks = debugIdChunkFilePaths.map( - (chunkFilePath, chunkIndex) => async () => { - await prepareBundleForDebugIdUpload( - chunkFilePath, - tmpUploadFolder, - chunkIndex, - logger, - options.sourcemaps?.rewriteSources ?? defaultRewriteSourcesHook, - options.sourcemaps?.resolveSourceMap - ); - } - ); - const workers: Promise[] = []; - const worker = async (): Promise => { - while (preparationTasks.length > 0) { - const task = preparationTasks.shift(); - if (task) { - await task(); + if (!shouldPrepare) { + // Direct CLI upload from existing artifact paths (no preparation or temp copies) + await startSpan({ name: "upload", scope: sentryScope }, async () => { + const cliInstance = createCliInstance(options); + await cliInstance.releases.uploadSourceMaps(options.release.name ?? "undefined", { + include: [ + { + paths: debugIdChunkFilePaths, + rewrite: false, + dist: options.release.dist, + }, + ], + live: "rejectOnError", + }); + }); + + logger.info("Successfully uploaded source maps to Sentry"); + } else { + const tmpUploadFolder = await startSpan( + { name: "mkdtemp", scope: sentryScope }, + async () => { + return ( + process.env?.["SENTRY_TEST_OVERRIDE_TEMP_DIR"] || + (await fs.promises.mkdtemp( + path.join(os.tmpdir(), "sentry-bundler-plugin-upload-") + )) + ); + } + ); + folderToCleanUp = tmpUploadFolder; + + // Prepare into temp folder, then upload + await startSpan( + { name: "prepare-bundles", scope: sentryScope }, + async (prepBundlesSpan) => { + // Preparing the bundles can be a lot of work and doing it all at once has the potential of nuking the heap so + // instead we do it with a maximum of 16 concurrent workers + const preparationTasks = debugIdChunkFilePaths.map( + (chunkFilePath, chunkIndex) => async () => { + await prepareBundleForDebugIdUpload( + chunkFilePath, + tmpUploadFolder, + chunkIndex, + logger, + options.sourcemaps?.rewriteSources ?? defaultRewriteSourcesHook, + options.sourcemaps?.resolveSourceMap + ); + } + ); + const workers: Promise[] = []; + const worker = async (): Promise => { + while (preparationTasks.length > 0) { + const task = preparationTasks.shift(); + if (task) { + await task(); + } } + }; + for (let workerIndex = 0; workerIndex < 16; workerIndex++) { + workers.push(worker()); } - }; - for (let workerIndex = 0; workerIndex < 16; workerIndex++) { - workers.push(worker()); - } - await Promise.all(workers); + await Promise.all(workers); - const files = await fs.promises.readdir(tmpUploadFolder); - const stats = files.map((file) => - fs.promises.stat(path.join(tmpUploadFolder, file)) - ); - const uploadSize = (await Promise.all(stats)).reduce( - (accumulator, { size }) => accumulator + size, - 0 - ); - - setMeasurement("files", files.length, "none", prepBundlesSpan); - setMeasurement("upload_size", uploadSize, "byte", prepBundlesSpan); - - await startSpan({ name: "upload", scope: sentryScope }, async () => { - const cliInstance = createCliInstance(options); - - await cliInstance.releases.uploadSourceMaps( - options.release.name ?? "undefined", // unfortunately this needs a value for now but it will not matter since debug IDs overpower releases anyhow - { - include: [ - { - paths: [tmpUploadFolder], - rewrite: false, - dist: options.release.dist, - }, - ], - // We want this promise to throw if the sourcemaps fail to upload so that we know about it. - // see: https://github.com/getsentry/sentry-cli/pull/2605 - live: "rejectOnError", - } + const files = await fs.promises.readdir(tmpUploadFolder); + const stats = files.map((file) => + fs.promises.stat(path.join(tmpUploadFolder, file)) + ); + const uploadSize = (await Promise.all(stats)).reduce( + (accumulator, { size }) => accumulator + size, + 0 ); - }); - return files.length; - } - ); + setMeasurement("files", files.length, "none", prepBundlesSpan); + setMeasurement("upload_size", uploadSize, "byte", prepBundlesSpan); + + await startSpan({ name: "upload", scope: sentryScope }, async () => { + const cliInstance = createCliInstance(options); + await cliInstance.releases.uploadSourceMaps( + options.release.name ?? "undefined", + { + include: [ + { + paths: [tmpUploadFolder], + rewrite: false, + dist: options.release.dist, + }, + ], + live: "rejectOnError", + } + ); + }); + } + ); - if (numUploadedFiles > 0) { logger.info("Successfully uploaded source maps to Sentry"); } } diff --git a/packages/bundler-plugin-core/test/build-plugin-manager.test.ts b/packages/bundler-plugin-core/test/build-plugin-manager.test.ts index 47ec645f..0a9bcedf 100644 --- a/packages/bundler-plugin-core/test/build-plugin-manager.test.ts +++ b/packages/bundler-plugin-core/test/build-plugin-manager.test.ts @@ -1,25 +1,46 @@ import { createSentryBuildPluginManager } from "../src/build-plugin-manager"; +import fs from "fs"; +import { glob } from "glob"; +import { prepareBundleForDebugIdUpload } from "../src/debug-id-upload"; const mockCliExecute = jest.fn(); +const mockCliUploadSourceMaps = jest.fn(); + jest.mock("@sentry/cli", () => { return jest.fn().mockImplementation(() => ({ execute: mockCliExecute, + releases: { + uploadSourceMaps: mockCliUploadSourceMaps, + new: jest.fn(), + finalize: jest.fn(), + setCommits: jest.fn(), + newDeploy: jest.fn(), + }, })); }); // eslint-disable-next-line @typescript-eslint/no-unsafe-return jest.mock("../src/sentry/telemetry", () => ({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment ...jest.requireActual("../src/sentry/telemetry"), safeFlushTelemetry: jest.fn(), })); // eslint-disable-next-line @typescript-eslint/no-unsafe-return jest.mock("@sentry/core", () => ({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment ...jest.requireActual("@sentry/core"), - // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-return - startSpan: jest.fn((options, callback) => callback()), + startSpan: jest.fn((options: unknown, callback: () => unknown) => callback()), })); +jest.mock("glob"); +jest.mock("../src/debug-id-upload"); + +const mockGlob = glob as jest.MockedFunction; +const mockPrepareBundleForDebugIdUpload = prepareBundleForDebugIdUpload as jest.MockedFunction< + typeof prepareBundleForDebugIdUpload +>; + describe("createSentryBuildPluginManager", () => { beforeEach(() => { jest.clearAllMocks(); @@ -73,6 +94,86 @@ describe("createSentryBuildPluginManager", () => { }); }); + describe("uploadSourcemaps", () => { + it("uploads in-place when prepareArtifacts is false", async () => { + mockCliUploadSourceMaps.mockResolvedValue(undefined); + + // Return a mixture of files/dirs; in-place path should pass through as-is + mockGlob.mockResolvedValue(["/app/dist/a.js", "/app/dist/dir", "/app/dist/a.js.map"]); + + const manager = createSentryBuildPluginManager( + { + authToken: "t", + org: "o", + project: "p", + release: { name: "some-release-name", dist: "1" }, + sourcemaps: { assets: ["/app/dist/**/*"] }, + }, + { buildTool: "webpack", loggerPrefix: "[sentry-webpack-plugin]" } + ); + + await manager.uploadSourcemaps(["/unused"], { prepareArtifacts: false }); + + expect(mockCliUploadSourceMaps).toHaveBeenCalledTimes(1); + expect(mockCliUploadSourceMaps).toHaveBeenCalledWith( + "some-release-name", + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + expect.objectContaining({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + include: expect.arrayContaining([ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + expect.objectContaining({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + paths: expect.arrayContaining([ + "/app/dist/a.js", + "/app/dist/dir", + "/app/dist/a.js.map", + ]), + rewrite: false, + dist: "1", + }), + ]), + live: "rejectOnError", + }) + ); + expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled(); + }); + + it("prepares into temp folder and uploads when prepareArtifacts is true (default)", async () => { + mockCliUploadSourceMaps.mockResolvedValue(undefined); + + mockGlob.mockResolvedValue(["/app/dist/a.js", "/app/dist/a.js.map", "/app/dist/other.txt"]); + + jest.spyOn(fs.promises, "mkdtemp").mockResolvedValue("/tmp/sentry-upload-xyz"); + jest.spyOn(fs.promises, "readdir").mockResolvedValue(["a.js", "a.js.map"] as never); + jest.spyOn(fs.promises, "stat").mockResolvedValue({ size: 10 } as fs.Stats); + jest.spyOn(fs.promises, "rm").mockResolvedValue(undefined as never); + + mockPrepareBundleForDebugIdUpload.mockResolvedValue(undefined); + + const manager = createSentryBuildPluginManager( + { + authToken: "t", + org: "o", + project: "p", + release: { name: "some-release-name", dist: "1" }, + sourcemaps: { assets: ["/app/dist/**/*"] }, + }, + { buildTool: "webpack", loggerPrefix: "[sentry-webpack-plugin]" } + ); + + await manager.uploadSourcemaps(["/unused"]); + + // Should call prepare for each JS chunk discovered by glob + expect(mockPrepareBundleForDebugIdUpload).toHaveBeenCalled(); + // Should upload from temp folder + expect(mockCliUploadSourceMaps).toHaveBeenCalledWith("some-release-name", { + include: [{ paths: ["/tmp/sentry-upload-xyz"], rewrite: false, dist: "1" }], + live: "rejectOnError", + }); + }); + }); + describe("injectDebugIds", () => { it("should call CLI with correct sourcemaps inject command", async () => { mockCliExecute.mockResolvedValue(undefined);