From e99172239203daa23dde6c7bf504eb590311ca68 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 28 Apr 2025 11:59:50 +0200 Subject: [PATCH 1/4] fix: Replace existing debug ID comments --- .gitignore | 1 + package.json | 2 +- .../src/build-plugin-manager.ts | 9 +- .../src/debug-id-upload.ts | 16 ++- .../debug-ids-already-injected.test.ts | 108 ++++++++++++++++++ .../input/bundle.js | 2 + .../input/rollup4/package.json | 5 + .../input/rollup4/rollup.config.js | 15 +++ .../input/vite6/package.json | 5 + .../input/vite6/vite.config.js | 21 ++++ .../input/webpack5/package.json | 6 + .../input/webpack5/webpack.config.js | 18 +++ packages/integration-tests/utils/testIf.ts | 9 +- 13 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 packages/integration-tests/fixtures/debug-ids-already-injected/debug-ids-already-injected.test.ts create mode 100644 packages/integration-tests/fixtures/debug-ids-already-injected/input/bundle.js create mode 100644 packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/package.json create mode 100644 packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js create mode 100644 packages/integration-tests/fixtures/debug-ids-already-injected/input/vite6/package.json create mode 100644 packages/integration-tests/fixtures/debug-ids-already-injected/input/vite6/vite.config.js create mode 100644 packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/package.json create mode 100644 packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js diff --git a/.gitignore b/.gitignore index 7f95f101..cd23e592 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ yarn-error.log *.tgz .nxcache +packages/**/yarn.lock \ No newline at end of file diff --git a/package.json b/package.json index 1abce4f1..d0348a79 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "npm-run-all": "^4.1.5" }, "volta": { - "node": "14.21.2", + "node": "20.19.1", "yarn": "1.22.19" } } diff --git a/packages/bundler-plugin-core/src/build-plugin-manager.ts b/packages/bundler-plugin-core/src/build-plugin-manager.ts index 4bc85e4f..397f04bf 100644 --- a/packages/bundler-plugin-core/src/build-plugin-manager.ts +++ b/packages/bundler-plugin-core/src/build-plugin-manager.ts @@ -457,8 +457,11 @@ export function createSentryBuildPluginManager( const tmpUploadFolder = await startSpan( { name: "mkdtemp", scope: sentryScope }, async () => { - return await fs.promises.mkdtemp( - path.join(os.tmpdir(), "sentry-bundler-plugin-upload-") + return ( + process.env?.["SENTRY_TEST_OVERRIDE_TEMP_DIR"] || + (await fs.promises.mkdtemp( + path.join(os.tmpdir(), "sentry-bundler-plugin-upload-") + )) ); } ); @@ -586,7 +589,7 @@ export function createSentryBuildPluginManager( sentryScope.captureException('Error in "debugIdUploadPlugin" writeBundle hook'); handleRecoverableError(e, false); } finally { - if (folderToCleanUp) { + if (folderToCleanUp && !process.env?.["SENTRY_TEST_OVERRIDE_TEMP_DIR"]) { void startSpan({ name: "cleanup", scope: sentryScope }, async () => { if (folderToCleanUp) { await fs.promises.rm(folderToCleanUp, { recursive: true, force: true }); diff --git a/packages/bundler-plugin-core/src/debug-id-upload.ts b/packages/bundler-plugin-core/src/debug-id-upload.ts index b9bba5a3..c9f9302e 100644 --- a/packages/bundler-plugin-core/src/debug-id-upload.ts +++ b/packages/bundler-plugin-core/src/debug-id-upload.ts @@ -50,7 +50,7 @@ export async function prepareBundleForDebugIdUpload( const uniqueUploadName = `${debugId}-${chunkIndex}`; - bundleContent += `\n//# debugId=${debugId}`; + bundleContent = addDebugIdToBundleSource(bundleContent, debugId); const writeSourceFilePromise = fs.promises.writeFile( path.join(uploadFolder, `${uniqueUploadName}.js`), bundleContent, @@ -95,6 +95,20 @@ function determineDebugIdFromBundleSource(code: string): string | undefined { } } +const SPEC_LAST_DEBUG_ID_REGEX = /\/\/# debugId=([a-fA-F0-9-]+)(?![\s\S]*\/\/# debugId=)/m; + +function hasSpecCompliantDebugId(bundleSource: string): boolean { + return SPEC_LAST_DEBUG_ID_REGEX.test(bundleSource); +} + +function addDebugIdToBundleSource(bundleSource: string, debugId: string): string { + if (hasSpecCompliantDebugId(bundleSource)) { + return bundleSource.replace(SPEC_LAST_DEBUG_ID_REGEX, `//# debugId=${debugId}`); + } else { + return `${bundleSource}\n//# debugId=${debugId}`; + } +} + /** * Applies a set of heuristics to find the source map for a particular bundle. * diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/debug-ids-already-injected.test.ts b/packages/integration-tests/fixtures/debug-ids-already-injected/debug-ids-already-injected.test.ts new file mode 100644 index 00000000..5581d4dd --- /dev/null +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/debug-ids-already-injected.test.ts @@ -0,0 +1,108 @@ +/* eslint-disable jest/no-standalone-expect */ +/* eslint-disable jest/expect-expect */ +import path from "path"; +import * as fs from "fs"; +import * as os from "os"; +import { describeNode18Plus } from "../../utils/testIf"; +import { execSync } from "child_process"; + +function createTempDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), "sentry-bundler-plugin-upload-")); +} + +const SPEC_DEBUG_ID_REGEX = /\/\/# debugId=([a-fA-F0-9-]+)/g; + +function countDebugIdComments(source: string): number { + const matches = source.match(SPEC_DEBUG_ID_REGEX); + if (matches) { + return matches.length; + } + return 0; +} + +function getSingleJavaScriptSourceFileFromDirectory( + dir: string, + fileExtension = ".js" +): string | undefined { + const files = fs.readdirSync(dir); + const jsFiles = files.filter((file) => file.endsWith(fileExtension)); + if (jsFiles.length === 1) { + return fs.readFileSync(path.join(dir, jsFiles[0] as string), "utf-8"); + } + return undefined; +} + +describeNode18Plus("vite 6 bundle", () => { + const viteRoot = path.join(__dirname, "input", "vite6"); + const tempDir = createTempDir(); + + beforeEach(() => { + execSync("yarn install", { cwd: viteRoot, stdio: "inherit" }); + execSync("yarn vite build", { + cwd: viteRoot, + stdio: "inherit", + env: { ...process.env, SENTRY_TEST_OVERRIDE_TEMP_DIR: tempDir }, + }); + }); + + test("check vite 6 bundle", () => { + const source = getSingleJavaScriptSourceFileFromDirectory(tempDir); + expect(source).toBeDefined(); + const debugIds = countDebugIdComments(source as string); + expect(debugIds).toBe(1); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); +}); + +describeNode18Plus("webpack 5 bundle", () => { + const viteRoot = path.join(__dirname, "input", "webpack5"); + const tempDir = createTempDir(); + + beforeEach(() => { + execSync("yarn install", { cwd: viteRoot, stdio: "inherit" }); + execSync("yarn webpack build", { + cwd: viteRoot, + stdio: "inherit", + env: { ...process.env, SENTRY_TEST_OVERRIDE_TEMP_DIR: tempDir }, + }); + }); + + test("check webpack 5 bundle", () => { + const source = getSingleJavaScriptSourceFileFromDirectory(tempDir); + expect(source).toBeDefined(); + const debugIds = countDebugIdComments(source as string); + expect(debugIds).toBe(1); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); +}); + +describeNode18Plus("rollup bundle", () => { + const viteRoot = path.join(__dirname, "input", "rollup4"); + const tempDir = createTempDir(); + + beforeEach(() => { + execSync("yarn install", { cwd: viteRoot, stdio: "inherit" }); + execSync("yarn rollup --config rollup.config.js", { + cwd: viteRoot, + stdio: "inherit", + env: { ...process.env, SENTRY_TEST_OVERRIDE_TEMP_DIR: tempDir }, + }); + }); + + test("check rollup bundle", () => { + const source = getSingleJavaScriptSourceFileFromDirectory(tempDir); + expect(source).toBeDefined(); + const debugIds = countDebugIdComments(source as string); + expect(debugIds).toBe(1); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); +}); diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/bundle.js b/packages/integration-tests/fixtures/debug-ids-already-injected/input/bundle.js new file mode 100644 index 00000000..74cb2663 --- /dev/null +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/bundle.js @@ -0,0 +1,2 @@ +// eslint-disable-next-line no-console +console.log("Hello world"); diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/package.json b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/package.json new file mode 100644 index 00000000..4435487c --- /dev/null +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "rollup": "^4" + } +} diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js new file mode 100644 index 00000000..e5453246 --- /dev/null +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js @@ -0,0 +1,15 @@ +import { defineConfig } from "rollup"; +import { sentryRollupPlugin } from "@sentry/rollup-plugin"; +import { join } from "path"; + +const __dirname = new URL(".", import.meta.url).pathname; + +export default defineConfig({ + input: { index: join(__dirname, "..", "bundle.js") }, + output: { + dir: join(__dirname, "..", "..", "out", "rollup4"), + sourcemap: true, + sourcemapDebugIds: true, + }, + plugins: [sentryRollupPlugin({ telemetry: false })], +}); diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/vite6/package.json b/packages/integration-tests/fixtures/debug-ids-already-injected/input/vite6/package.json new file mode 100644 index 00000000..94bf169f --- /dev/null +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/vite6/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "vite": "^6" + } +} diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/vite6/vite.config.js b/packages/integration-tests/fixtures/debug-ids-already-injected/input/vite6/vite.config.js new file mode 100644 index 00000000..39e04917 --- /dev/null +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/vite6/vite.config.js @@ -0,0 +1,21 @@ +import { defineConfig } from "vite"; +import { sentryVitePlugin } from "@sentry/vite-plugin"; +import { join } from "path"; + +export default defineConfig({ + clearScreen: false, + mode: "production", + build: { + sourcemap: true, + outDir: join(__dirname, "..", "..", "out", "vite6"), + rollupOptions: { + input: { index: join(__dirname, "..", "bundle.js") }, + output: { + format: "cjs", + entryFileNames: "[name].js", + sourcemapDebugIds: true, + }, + }, + }, + plugins: [sentryVitePlugin({ telemetry: false })], +}); diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/package.json b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/package.json new file mode 100644 index 00000000..2f8444d7 --- /dev/null +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/package.json @@ -0,0 +1,6 @@ +{ + "dependencies": { + "webpack": "^5", + "webpack-cli": "^6" + } +} diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js new file mode 100644 index 00000000..5ef92798 --- /dev/null +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js @@ -0,0 +1,18 @@ +import { sentryWebpackPlugin } from "@sentry/webpack-plugin"; +import { join } from "path"; + +const __dirname = new URL(".", import.meta.url).pathname; + +export default { + devtool: "source-map-debugids", + cache: false, + entry: { index: join(__dirname, "..", "bundle.js") }, + output: { + path: join(__dirname, "..", "..", "out", "webpack5"), + library: { + type: "commonjs", + }, + }, + mode: "production", + plugins: [sentryWebpackPlugin({ telemetry: false })], +}; diff --git a/packages/integration-tests/utils/testIf.ts b/packages/integration-tests/utils/testIf.ts index 6c8ac66a..cdfaca96 100644 --- a/packages/integration-tests/utils/testIf.ts +++ b/packages/integration-tests/utils/testIf.ts @@ -1,3 +1,5 @@ +const [NODE_MAJOR_VERSION] = process.version.replace("v", "").split(".").map(Number) as [number]; + // eslint-disable-next-line no-undef export function testIf(condition: boolean): jest.It { if (condition) { @@ -15,7 +17,10 @@ export function testIf(condition: boolean): jest.It { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-undef, @typescript-eslint/no-unsafe-assignment export const testIfNodeMajorVersionIsLessThan18: jest.It = function () { - const nodejsMajorversion = process.version.split(".")[0]?.slice(1); - return testIf(!nodejsMajorversion || parseInt(nodejsMajorversion) < 18); + return testIf(NODE_MAJOR_VERSION < 18); // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; + +// eslint-disable-next-line no-undef +export const describeNode18Plus: jest.Describe = + NODE_MAJOR_VERSION >= 18 ? describe : describe.skip; From 27bae7fdb0592f2689ff235c1d0f850bb508a66e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 28 Apr 2025 12:03:08 +0200 Subject: [PATCH 2/4] Oops --- package.json | 2 +- .../debug-ids-already-injected.test.ts | 4 +--- packages/integration-tests/utils/testIf.ts | 1 + 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index d0348a79..1abce4f1 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "npm-run-all": "^4.1.5" }, "volta": { - "node": "20.19.1", + "node": "14.21.2", "yarn": "1.22.19" } } diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/debug-ids-already-injected.test.ts b/packages/integration-tests/fixtures/debug-ids-already-injected/debug-ids-already-injected.test.ts index 5581d4dd..f0c77f67 100644 --- a/packages/integration-tests/fixtures/debug-ids-already-injected/debug-ids-already-injected.test.ts +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/debug-ids-already-injected.test.ts @@ -1,6 +1,4 @@ -/* eslint-disable jest/no-standalone-expect */ -/* eslint-disable jest/expect-expect */ -import path from "path"; +import * as path from "path"; import * as fs from "fs"; import * as os from "os"; import { describeNode18Plus } from "../../utils/testIf"; diff --git a/packages/integration-tests/utils/testIf.ts b/packages/integration-tests/utils/testIf.ts index cdfaca96..8f47bc78 100644 --- a/packages/integration-tests/utils/testIf.ts +++ b/packages/integration-tests/utils/testIf.ts @@ -23,4 +23,5 @@ export const testIfNodeMajorVersionIsLessThan18: jest.It = function () { // eslint-disable-next-line no-undef export const describeNode18Plus: jest.Describe = + // eslint-disable-next-line no-undef NODE_MAJOR_VERSION >= 18 ? describe : describe.skip; From b3fbaa4e31e6cffdc1313f6c8c3a08ce0b3bd581 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 28 Apr 2025 12:43:50 +0200 Subject: [PATCH 3/4] Use cjs for bundler configs --- .../input/rollup4/rollup.config.js | 13 ++++++------- .../input/webpack5/webpack.config.js | 11 +++++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js index e5453246..6d732e35 100644 --- a/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js @@ -1,13 +1,12 @@ -import { defineConfig } from "rollup"; -import { sentryRollupPlugin } from "@sentry/rollup-plugin"; -import { join } from "path"; - -const __dirname = new URL(".", import.meta.url).pathname; +/* eslint-disable @typescript-eslint/no-var-requires */ +const { defineConfig } = require("rollup"); +const { sentryRollupPlugin } = require("@sentry/rollup-plugin"); +const path = require("path"); export default defineConfig({ - input: { index: join(__dirname, "..", "bundle.js") }, + input: { index: path.join(__dirname, "..", "bundle.js") }, output: { - dir: join(__dirname, "..", "..", "out", "rollup4"), + dir: path.join(__dirname, "..", "..", "out", "rollup4"), sourcemap: true, sourcemapDebugIds: true, }, diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js index 5ef92798..5af923d9 100644 --- a/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js @@ -1,14 +1,13 @@ -import { sentryWebpackPlugin } from "@sentry/webpack-plugin"; -import { join } from "path"; - -const __dirname = new URL(".", import.meta.url).pathname; +/* eslint-disable @typescript-eslint/no-var-requires */ +const { sentryWebpackPlugin } = require("@sentry/webpack-plugin"); +const path = require("path"); export default { devtool: "source-map-debugids", cache: false, - entry: { index: join(__dirname, "..", "bundle.js") }, + entry: { index: path.join(__dirname, "..", "bundle.js") }, output: { - path: join(__dirname, "..", "..", "out", "webpack5"), + path: path.join(__dirname, "..", "..", "out", "webpack5"), library: { type: "commonjs", }, From f7727125aed92f345eccacae8338bc1284a9965a Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 28 Apr 2025 12:49:55 +0200 Subject: [PATCH 4/4] type: module --- .../input/rollup4/package.json | 1 + .../input/rollup4/rollup.config.js | 13 +++++++------ .../input/webpack5/package.json | 1 + .../input/webpack5/webpack.config.js | 11 ++++++----- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/package.json b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/package.json index 4435487c..b455216a 100644 --- a/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/package.json +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/package.json @@ -1,4 +1,5 @@ { + "type": "module", "dependencies": { "rollup": "^4" } diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js index 6d732e35..e5453246 100644 --- a/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/rollup.config.js @@ -1,12 +1,13 @@ -/* eslint-disable @typescript-eslint/no-var-requires */ -const { defineConfig } = require("rollup"); -const { sentryRollupPlugin } = require("@sentry/rollup-plugin"); -const path = require("path"); +import { defineConfig } from "rollup"; +import { sentryRollupPlugin } from "@sentry/rollup-plugin"; +import { join } from "path"; + +const __dirname = new URL(".", import.meta.url).pathname; export default defineConfig({ - input: { index: path.join(__dirname, "..", "bundle.js") }, + input: { index: join(__dirname, "..", "bundle.js") }, output: { - dir: path.join(__dirname, "..", "..", "out", "rollup4"), + dir: join(__dirname, "..", "..", "out", "rollup4"), sourcemap: true, sourcemapDebugIds: true, }, diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/package.json b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/package.json index 2f8444d7..e1a155e7 100644 --- a/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/package.json +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/package.json @@ -1,4 +1,5 @@ { + "type": "module", "dependencies": { "webpack": "^5", "webpack-cli": "^6" diff --git a/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js index 5af923d9..5ef92798 100644 --- a/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js +++ b/packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/webpack.config.js @@ -1,13 +1,14 @@ -/* eslint-disable @typescript-eslint/no-var-requires */ -const { sentryWebpackPlugin } = require("@sentry/webpack-plugin"); -const path = require("path"); +import { sentryWebpackPlugin } from "@sentry/webpack-plugin"; +import { join } from "path"; + +const __dirname = new URL(".", import.meta.url).pathname; export default { devtool: "source-map-debugids", cache: false, - entry: { index: path.join(__dirname, "..", "bundle.js") }, + entry: { index: join(__dirname, "..", "bundle.js") }, output: { - path: path.join(__dirname, "..", "..", "out", "webpack5"), + path: join(__dirname, "..", "..", "out", "webpack5"), library: { type: "commonjs", },