fix: Replace existing debug ID comments

Author: timfish

.gitignore

@@ -7,3 +7,4 @@ yarn-error.log
 *.tgz
 
 .nxcache
+packages/**/yarn.lock

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"
   }
 }

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 });

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.
  *

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 });
+  });
+});

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");

packages/integration-tests/fixtures/debug-ids-already-injected/input/rollup4/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "rollup": "^4"
+  }
+}

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 })],
+});

packages/integration-tests/fixtures/debug-ids-already-injected/input/vite6/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "vite": "^6"
+  }
+}

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 })],
+});

packages/integration-tests/fixtures/debug-ids-already-injected/input/webpack5/package.json

@@ -0,0 +1,6 @@
+{
+  "dependencies": {
+    "webpack": "^5",
+    "webpack-cli": "^6"
+  }
+}

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 })],
+};

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;