Skip to content

Commit 7ee1f9f

Browse files
ThristhartLuca Forstner
Thristhart
and
Luca Forstner
authored
fix: Allow injection plugins to apply to files with query parameters and fragments in their name (#597)
Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
1 parent 687a9f5 commit 7ee1f9f

File tree

8 files changed

+236
-12
lines changed

8 files changed

+236
-12
lines changed

‎packages/bundler-plugin-core/src/debug-id-upload.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { Hub, NodeClient } from "@sentry/node";
99
import SentryCli from "@sentry/cli";
1010
import { dynamicSamplingContextToSentryBaggageHeader } from "@sentry/utils";
1111
import { safeFlushTelemetry } from "./sentry/telemetry";
12+
import { stripQueryAndHashFromPath } from "./utils";
1213

1314
interface RewriteSourcesHook {
1415
(source: string, map: any): string;
@@ -89,12 +90,9 @@ export function createDebugIdUploadFunction({
8990
});
9091
globSpan.finish();
9192

92-
const debugIdChunkFilePaths = globResult.filter(
93-
(debugIdChunkFilePath) =>
94-
debugIdChunkFilePath.endsWith(".js") ||
95-
debugIdChunkFilePath.endsWith(".mjs") ||
96-
debugIdChunkFilePath.endsWith(".cjs")
97-
);
93+
const debugIdChunkFilePaths = globResult.filter((debugIdChunkFilePath) => {
94+
return !!stripQueryAndHashFromPath(debugIdChunkFilePath).match(/\.(js|mjs|cjs)$/);
95+
});
9896

9997
// The order of the files output by glob() is not deterministic
10098
// Ensure order within the files so that {debug-id}-{chunkIndex} coupling is consistent

‎packages/bundler-plugin-core/src/index.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,10 @@ export function createRollupDebugIdInjectionHooks() {
528528
return {
529529
renderChunk(code: string, chunk: { fileName: string }) {
530530
if (
531-
[".js", ".mjs", ".cjs"].some((ending) => chunk.fileName.endsWith(ending)) // chunks could be any file (html, md, ...)
531+
// chunks could be any file (html, md, ...)
532+
[".js", ".mjs", ".cjs"].some((ending) =>
533+
stripQueryAndHashFromPath(chunk.fileName).endsWith(ending)
534+
)
532535
) {
533536
const debugId = stringToUUID(code); // generate a deterministic debug ID
534537
const codeToInject = getDebugIdSnippet(debugId);
@@ -562,7 +565,10 @@ export function createRollupModuleMetadataInjectionHooks(injectionCode: string)
562565
return {
563566
renderChunk(code: string, chunk: { fileName: string }) {
564567
if (
565-
[".js", ".mjs", ".cjs"].some((ending) => chunk.fileName.endsWith(ending)) // chunks could be any file (html, md, ...)
568+
// chunks could be any file (html, md, ...)
569+
[".js", ".mjs", ".cjs"].some((ending) =>
570+
stripQueryAndHashFromPath(chunk.fileName).endsWith(ending)
571+
)
566572
) {
567573
const ms = new MagicString(code, { filename: chunk.fileName });
568574

@@ -600,7 +606,14 @@ export function createRollupDebugIdUploadHooks(
600606
if (outputOptions.dir) {
601607
const outputDir = outputOptions.dir;
602608
const buildArtifacts = await glob(
603-
["/**/*.js", "/**/*.mjs", "/**/*.cjs", "/**/*.js.map", "/**/*.mjs.map", "/**/*.cjs.map"],
609+
[
610+
"/**/*.js",
611+
"/**/*.mjs",
612+
"/**/*.cjs",
613+
"/**/*.js.map",
614+
"/**/*.mjs.map",
615+
"/**/*.cjs.map",
616+
].map((q) => `${q}?(\\?*)?(#*)`), // We want to allow query and hashes strings at the end of files
604617
{
605618
root: outputDir,
606619
absolute: true,

‎packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/* eslint-disable jest/no-standalone-expect */
2+
/* eslint-disable jest/expect-expect */
3+
import childProcess from "child_process";
4+
import path from "path";
5+
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";
6+
7+
function checkBundleForDebugIds(bundlePath1: string, bundlePath2: string): string[] {
8+
const process1Output = childProcess.execSync(`node ${bundlePath1}`, { encoding: "utf-8" });
9+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
10+
const debugIdMap1 = JSON.parse(process1Output).debugIds as Record<string, string>;
11+
const debugIds1 = Object.values(debugIdMap1);
12+
expect(debugIds1.length).toBeGreaterThan(0);
13+
expect(debugIds1).toContainEqual(
14+
expect.stringMatching(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/)
15+
);
16+
17+
expect(Object.keys(debugIdMap1)[0]).toContain("Error");
18+
19+
const process2Output = childProcess.execSync(`node ${bundlePath2}`, { encoding: "utf-8" });
20+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
21+
const debugIdMap2 = JSON.parse(process2Output).debugIds as Record<string, string>;
22+
const debugIds2 = Object.values(debugIdMap2);
23+
expect(debugIds2.length).toBeGreaterThan(0);
24+
expect(debugIds2).toContainEqual(
25+
expect.stringMatching(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/)
26+
);
27+
28+
expect(Object.keys(debugIdMap2)[0]).toContain("Error");
29+
30+
expect(debugIds1).not.toEqual(debugIds2);
31+
32+
return [...debugIds1, ...debugIds2];
33+
}
34+
35+
function checkBundleForRelease(bundlePath: string): void {
36+
const processOutput = childProcess.execSync(`node ${bundlePath}`, { encoding: "utf-8" });
37+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
38+
expect(JSON.parse(processOutput).release).toBe("I AM A RELEASE!");
39+
}
40+
41+
// Query params and hashes are weird on windows
42+
(process.platform === "win32" ? describe.skip : describe)("Injection with query params", () => {
43+
test("vite bundle", () => {
44+
checkBundleForDebugIds(
45+
path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz"),
46+
path.join(__dirname, "out", "vite", "bundle2.js?foo=bar#baz")
47+
);
48+
checkBundleForRelease(path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz"));
49+
});
50+
51+
test("rollup bundle", () => {
52+
checkBundleForDebugIds(
53+
path.join(__dirname, "out", "rollup", "bundle1.js?foo=bar#baz"),
54+
path.join(__dirname, "out", "rollup", "bundle2.js?foo=bar#baz")
55+
);
56+
checkBundleForRelease(path.join(__dirname, "out", "rollup", "bundle1.js?foo=bar#baz"));
57+
});
58+
59+
testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
60+
checkBundleForDebugIds(
61+
path.join(__dirname, "out", "webpack4", "bundle1.js"),
62+
path.join(__dirname, "out", "webpack4", "bundle2.js")
63+
);
64+
checkBundleForRelease(path.join(__dirname, "out", "webpack4", "bundle1.js"));
65+
});
66+
67+
test("webpack 5 bundle", () => {
68+
checkBundleForDebugIds(
69+
path.join(__dirname, "out", "webpack5", "bundle1.js"),
70+
path.join(__dirname, "out", "webpack5", "bundle2.js")
71+
);
72+
checkBundleForRelease(path.join(__dirname, "out", "webpack5", "bundle1.js"));
73+
});
74+
});

‎packages/integration-tests/fixtures/injection-with-query-param/input/bundle1.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// eslint-disable-next-line no-console
2+
console.log(
3+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
4+
JSON.stringify({ debugIds: global._sentryDebugIds, release: global.SENTRY_RELEASE.id })
5+
);
6+
7+
// Just so the two bundles generate different hashes:
8+
global.iAmBundle1 = 1;

‎packages/integration-tests/fixtures/injection-with-query-param/input/bundle2.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// eslint-disable-next-line no-console
2+
console.log(
3+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
4+
JSON.stringify({ debugIds: global._sentryDebugIds, release: global.SENTRY_RELEASE.id })
5+
);
6+
7+
// Just so the two bundles generate different hashes:
8+
global.iAmBundle2 = 2;

‎packages/integration-tests/fixtures/injection-with-query-param/setup.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import * as path from "path";
2+
import { createCjsBundlesWithQueryParam } from "../../utils/create-cjs-bundles-with-query";
3+
4+
// Query params and hashes are weird on windows
5+
if (process.platform !== "win32") {
6+
const outputDir = path.resolve(__dirname, "out");
7+
createCjsBundlesWithQueryParam(
8+
{
9+
bundle1: path.resolve(__dirname, "input", "bundle1.js"),
10+
bundle2: path.resolve(__dirname, "input", "bundle2.js"),
11+
},
12+
outputDir,
13+
{
14+
telemetry: false,
15+
release: { name: "I AM A RELEASE!", create: false },
16+
}
17+
);
18+
}

‎packages/integration-tests/utils/create-cjs-bundles-with-query.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import * as vite from "vite";
2+
import * as path from "path";
3+
import * as rollup from "rollup";
4+
import { default as webpack4 } from "webpack4";
5+
import { webpack as webpack5 } from "webpack5";
6+
import { Options } from "@sentry/bundler-plugin-core";
7+
import { sentryVitePlugin } from "@sentry/vite-plugin";
8+
import { sentryWebpackPlugin } from "@sentry/webpack-plugin";
9+
import { sentryRollupPlugin } from "@sentry/rollup-plugin";
10+
11+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
12+
const nodejsMajorVersion = process.version.split(".")[0]!.slice(1);
13+
14+
export function createCjsBundlesWithQueryParam(
15+
entrypoints: { [name: string]: string },
16+
outFolder: string,
17+
sentryUnpluginOptions: Options,
18+
plugins: string[] = []
19+
): void {
20+
if (plugins.length === 0 || plugins.includes("vite")) {
21+
void vite.build({
22+
clearScreen: false,
23+
build: {
24+
sourcemap: true,
25+
outDir: path.join(outFolder, "vite"),
26+
rollupOptions: {
27+
input: entrypoints,
28+
output: {
29+
format: "cjs",
30+
entryFileNames: "[name].js?foo=bar#baz",
31+
},
32+
},
33+
},
34+
plugins: [sentryVitePlugin(sentryUnpluginOptions)],
35+
});
36+
}
37+
if (plugins.length === 0 || plugins.includes("rollup")) {
38+
void rollup
39+
.rollup({
40+
input: entrypoints,
41+
plugins: [sentryRollupPlugin(sentryUnpluginOptions)],
42+
})
43+
.then((bundle) =>
44+
bundle.write({
45+
sourcemap: true,
46+
dir: path.join(outFolder, "rollup"),
47+
format: "cjs",
48+
exports: "named",
49+
entryFileNames: "[name].js?foo=bar#baz",
50+
})
51+
);
52+
}
53+
54+
if (plugins.length === 0 || plugins.includes("esbuild")) {
55+
// esbuild doesn't have an option to add a query param
56+
}
57+
58+
// Webpack 4 doesn't work on Node.js versions >= 18
59+
if (parseInt(nodejsMajorVersion) < 18 && (plugins.length === 0 || plugins.includes("webpack4"))) {
60+
webpack4(
61+
{
62+
devtool: "source-map",
63+
mode: "production",
64+
entry: entrypoints,
65+
cache: false,
66+
output: {
67+
path: path.join(outFolder, "webpack4"),
68+
filename: "[name].js?foo=bar#baz", // For some weird reason, the query param is not actually put to disk but the "virtual" behaviour we want to test still applies
69+
libraryTarget: "commonjs",
70+
},
71+
target: "node", // needed for webpack 4 so we can access node api
72+
plugins: [sentryWebpackPlugin(sentryUnpluginOptions)],
73+
},
74+
(err) => {
75+
if (err) {
76+
throw err;
77+
}
78+
}
79+
);
80+
}
81+
82+
if (plugins.length === 0 || plugins.includes("webpack5")) {
83+
webpack5(
84+
{
85+
devtool: "source-map",
86+
cache: false,
87+
entry: entrypoints,
88+
output: {
89+
path: path.join(outFolder, "webpack5"),
90+
filename: "[name].js?foo=bar#baz", // For some weird reason, the query param is not actually put to disk but the "virtual" behaviour we want to test still applies
91+
library: {
92+
type: "commonjs",
93+
},
94+
},
95+
mode: "production",
96+
plugins: [sentryWebpackPlugin(sentryUnpluginOptions)],
97+
},
98+
(err) => {
99+
if (err) {
100+
throw err;
101+
}
102+
}
103+
);
104+
}
105+
}

‎packages/webpack-plugin/src/index.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ function webpackReleaseInjectionPlugin(injectionCode: string): UnpluginOptions {
3939
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call
4040
new BannerPlugin({
4141
raw: true,
42-
include: /\.(js|ts|jsx|tsx|mjs|cjs)$/,
42+
include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?[^?]*)?(#[^#]*)?$/,
4343
banner: injectionCode,
4444
})
4545
);
@@ -105,7 +105,7 @@ function webpackDebugIdInjectionPlugin(): UnpluginOptions {
105105
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call
106106
new BannerPlugin({
107107
raw: true,
108-
include: /\.(js|ts|jsx|tsx|mjs|cjs)$/,
108+
include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?[^?]*)?(#[^#]*)?$/,
109109
banner: (arg?: BannerPluginCallbackArg) => {
110110
const debugId = arg?.chunk?.hash ? stringToUUID(arg.chunk.hash) : uuidv4();
111111
return getDebugIdSnippet(debugId);
@@ -156,7 +156,7 @@ function webpackModuleMetadataInjectionPlugin(injectionCode: string): UnpluginOp
156156
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call
157157
new BannerPlugin({
158158
raw: true,
159-
include: /\.(js|ts|jsx|tsx|mjs|cjs)$/,
159+
include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?[^?]*)?(#[^#]*)?$/,
160160
banner: injectionCode,
161161
})
162162
);

0 commit comments

Comments
 (0)