From 1f45440f1fccb312037c54439c7900399d72af43 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 2 Sep 2025 10:49:28 +0200 Subject: [PATCH 1/3] glob no more --- .../src/build-plugin-manager.ts | 127 ++++++++++-------- .../test/build-plugin-manager.test.ts | 89 ++++++++++-- 2 files changed, 154 insertions(+), 62 deletions(-) diff --git a/packages/bundler-plugin-core/src/build-plugin-manager.ts b/packages/bundler-plugin-core/src/build-plugin-manager.ts index 65b395f0..25edf10c 100644 --- a/packages/bundler-plugin-core/src/build-plugin-manager.ts +++ b/packages/bundler-plugin-core/src/build-plugin-manager.ts @@ -564,6 +564,15 @@ export function createSentryBuildPluginManager( return; } + // Early exit if assets is explicitly set to an empty array + const assets = options.sourcemaps?.assets; + if (Array.isArray(assets) && assets.length === 0) { + logger.debug( + "Empty `sourcemaps.assets` option provided. Will not upload sourcemaps with debug ID." + ); + return; + } + await startSpan( // 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 }, @@ -578,65 +587,77 @@ export function createSentryBuildPluginManager( const freeUploadDependencyOnBuildArtifacts = createDependencyOnBuildArtifacts(); try { - const assets = options.sourcemaps?.assets; - - let globAssets: string | string[]; - if (assets) { - globAssets = assets; - } else { - logger.debug( - "No `sourcemaps.assets` option provided, falling back to uploading detected build artifacts." - ); - globAssets = buildArtifactPaths; - } + if (!shouldPrepare) { + // Direct CLI upload from existing artifact paths (no globbing, no preparation) + let pathsToUpload: string[]; - const globResult = await startSpan( - { name: "glob", scope: sentryScope }, - async () => - await glob(globAssets, { - absolute: 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, - }) - ); + if (assets) { + pathsToUpload = Array.isArray(assets) ? assets : [assets]; + logger.debug( + `Direct upload mode: passing user-provided assets directly to CLI: ${pathsToUpload.join( + ", " + )}` + ); + } else { + // Use original paths e.g. like ['.next/server'] directly –> preferred way when no globbing is done + pathsToUpload = buildArtifactPaths; + } - const debugIdChunkFilePaths = shouldPrepare - ? globResult.filter((debugIdChunkFilePath) => { - return !!stripQueryAndHashFromPath(debugIdChunkFilePath).match(/\.(js|mjs|cjs)$/); - }) - : globResult; + const ignorePaths = options.sourcemaps?.ignore + ? Array.isArray(options.sourcemaps?.ignore) + ? options.sourcemaps?.ignore + : [options.sourcemaps?.ignore] + : []; + await startSpan({ name: "upload", scope: sentryScope }, async () => { + const cliInstance = createCliInstance(options); + await cliInstance.releases.uploadSourceMaps(options.release.name ?? "undefined", { + include: [ + { + paths: pathsToUpload, + rewrite: false, + dist: options.release.dist, + }, + ], + ignore: ignorePaths, + live: "rejectOnError", + }); + }); - // The order of the files output by glob() is not deterministic - // Ensure order within the files so that {debug-id}-{chunkIndex} coupling is consistent - debugIdChunkFilePaths.sort(); + logger.info("Successfully uploaded source maps to Sentry"); + } else { + // Prepare artifacts in temp folder before uploading + let globAssets: string | string[]; + if (assets) { + globAssets = assets; + } else { + logger.debug( + "No `sourcemaps.assets` option provided, falling back to uploading detected build artifacts." + ); + globAssets = buildArtifactPaths; + } - if (Array.isArray(assets) && assets.length === 0) { - logger.debug( - "Empty `sourcemaps.assets` option provided. Will not upload sourcemaps with debug ID." - ); - } else if (debugIdChunkFilePaths.length === 0) { - logger.warn( - "Didn't find any matching sources for debug ID upload. Please check the `sourcemaps.assets` option." + const globResult = await startSpan( + { name: "glob", scope: sentryScope }, + async () => + await glob(globAssets, { + absolute: true, + nodir: true, // We need individual files for preparation + ignore: options.sourcemaps?.ignore, + }) ); - } else { - 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"); + const debugIdChunkFilePaths = globResult.filter((debugIdChunkFilePath) => { + return !!stripQueryAndHashFromPath(debugIdChunkFilePath).match(/\.(js|mjs|cjs)$/); + }); + + // The order of the files output by glob() is not deterministic + // Ensure order within the files so that {debug-id}-{chunkIndex} coupling is consistent + debugIdChunkFilePaths.sort(); + + if (debugIdChunkFilePaths.length === 0) { + logger.warn( + "Didn't find any matching sources for debug ID upload. Please check the `sourcemaps.assets` option." + ); } else { const tmpUploadFolder = await startSpan( { name: "mkdtemp", scope: sentryScope }, 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 0a9bcedf..4048d18d 100644 --- a/packages/bundler-plugin-core/test/build-plugin-manager.test.ts +++ b/packages/bundler-plugin-core/test/build-plugin-manager.test.ts @@ -98,9 +98,6 @@ describe("createSentryBuildPluginManager", () => { 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", @@ -123,12 +120,8 @@ describe("createSentryBuildPluginManager", () => { 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", - ]), + // User-provided assets should be passed directly to CLI (no globbing) + paths: ["/app/dist/**/*"], rewrite: false, dist: "1", }), @@ -136,6 +129,84 @@ describe("createSentryBuildPluginManager", () => { live: "rejectOnError", }) ); + // Should not glob when prepareArtifacts is false + expect(mockGlob).not.toHaveBeenCalled(); + expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled(); + }); + + it("uploads build artifact paths when prepareArtifacts is false and no assets provided", async () => { + mockCliUploadSourceMaps.mockResolvedValue(undefined); + + const manager = createSentryBuildPluginManager( + { + authToken: "t", + org: "o", + project: "p", + release: { name: "some-release-name", dist: "1" }, + // No assets provided + }, + { buildTool: "webpack", loggerPrefix: "[sentry-webpack-plugin]" } + ); + + await manager.uploadSourcemaps([".next", "dist"], { 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({ + // Should use buildArtifactPaths directly + paths: [".next", "dist"], + rewrite: false, + dist: "1", + }), + ]), + live: "rejectOnError", + }) + ); + expect(mockGlob).not.toHaveBeenCalled(); + expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled(); + }); + + it("exits early when assets is an empty array", async () => { + const manager = createSentryBuildPluginManager( + { + authToken: "t", + org: "o", + project: "p", + release: { name: "some-release-name", dist: "1" }, + sourcemaps: { assets: [] }, + }, + { buildTool: "webpack", loggerPrefix: "[sentry-webpack-plugin]" } + ); + + await manager.uploadSourcemaps([".next"], { prepareArtifacts: false }); + + expect(mockCliUploadSourceMaps).not.toHaveBeenCalled(); + expect(mockGlob).not.toHaveBeenCalled(); + expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled(); + }); + + it("exits early when assets is an empty array even for default mode", async () => { + const manager = createSentryBuildPluginManager( + { + authToken: "t", + org: "o", + project: "p", + release: { name: "some-release-name", dist: "1" }, + sourcemaps: { assets: [] }, + }, + { buildTool: "webpack", loggerPrefix: "[sentry-webpack-plugin]" } + ); + + await manager.uploadSourcemaps([".next"]); + + expect(mockCliUploadSourceMaps).not.toHaveBeenCalled(); + expect(mockGlob).not.toHaveBeenCalled(); expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled(); }); From 6ae68f5bc0ff41402420279102406f5d87aa4627 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 2 Sep 2025 11:12:46 +0200 Subject: [PATCH 2/3] set rewrite also for direct uploads --- packages/bundler-plugin-core/src/build-plugin-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/bundler-plugin-core/src/build-plugin-manager.ts b/packages/bundler-plugin-core/src/build-plugin-manager.ts index 25edf10c..31f5a2eb 100644 --- a/packages/bundler-plugin-core/src/build-plugin-manager.ts +++ b/packages/bundler-plugin-core/src/build-plugin-manager.ts @@ -614,7 +614,7 @@ export function createSentryBuildPluginManager( include: [ { paths: pathsToUpload, - rewrite: false, + rewrite: true, dist: options.release.dist, }, ], From 8a374b239cc08e40f6636274e42478bd428db25a Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 2 Sep 2025 11:39:43 +0200 Subject: [PATCH 3/3] fix tests --- .../bundler-plugin-core/test/build-plugin-manager.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 4048d18d..d9886ede 100644 --- a/packages/bundler-plugin-core/test/build-plugin-manager.test.ts +++ b/packages/bundler-plugin-core/test/build-plugin-manager.test.ts @@ -122,7 +122,7 @@ describe("createSentryBuildPluginManager", () => { expect.objectContaining({ // User-provided assets should be passed directly to CLI (no globbing) paths: ["/app/dist/**/*"], - rewrite: false, + rewrite: true, dist: "1", }), ]), @@ -161,7 +161,7 @@ describe("createSentryBuildPluginManager", () => { expect.objectContaining({ // Should use buildArtifactPaths directly paths: [".next", "dist"], - rewrite: false, + rewrite: true, dist: "1", }), ]),