Skip to content

Commit 777850f

Browse files
authored
fix(esbuild): fix debug ID injection when moduleMetadata or applicationKey is set (#828)
* tests: added tests to confirm bug * fix(esbuild): ensure debug IDs are injected alongside moduleMetadata * fix: linting and enable es6 * chore: comments * test: added applicationKey setting tests * fix(esbuild): strip query strings from importer paths in metadata proxy check * fix(esbuild): clear state from previous builds for watch mode and tests * fix(esbuild): clear stale metadata entries during build state reset
1 parent 05084f2 commit 777850f

File tree

8 files changed

+246
-25
lines changed

8 files changed

+246
-25
lines changed

packages/esbuild-plugin/.eslintrc.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ module.exports = {
1111
},
1212
env: {
1313
node: true,
14+
es6: true,
1415
},
1516
settings: {
1617
jest: {

packages/esbuild-plugin/src/index.ts

Lines changed: 78 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,18 @@ function esbuildReleaseInjectionPlugin(injectionCode: string): UnpluginOptions {
4242
};
4343
}
4444

45+
/**
46+
* Shared set to track entry points that have been wrapped by the metadata plugin
47+
* This allows the debug ID plugin to know when an import is coming from a metadata proxy
48+
*/
49+
const metadataProxyEntryPoints = new Set<string>();
50+
51+
/**
52+
* Set to track which paths have already been wrapped with debug ID injection
53+
* This prevents the debug ID plugin from wrapping the same module multiple times
54+
*/
55+
const debugIdWrappedPaths = new Set<string>();
56+
4557
function esbuildDebugIdInjectionPlugin(logger: Logger): UnpluginOptions {
4658
const pluginName = "sentry-esbuild-debug-id-injection-plugin";
4759
const stubNamespace = "sentry-debug-id-stub";
@@ -51,40 +63,70 @@ function esbuildDebugIdInjectionPlugin(logger: Logger): UnpluginOptions {
5163

5264
esbuild: {
5365
setup({ initialOptions, onLoad, onResolve }) {
66+
// Clear state from previous builds (important for watch mode and test suites)
67+
debugIdWrappedPaths.clear();
68+
// Also clear metadataProxyEntryPoints here because if moduleMetadataInjectionPlugin
69+
// is not instantiated in this build (e.g., moduleMetadata was disabled), we don't
70+
// want stale entries from a previous build to affect the current one.
71+
metadataProxyEntryPoints.clear();
72+
5473
if (!initialOptions.bundle) {
5574
logger.warn(
5675
"The Sentry esbuild plugin only supports esbuild with `bundle: true` being set in the esbuild build options. Esbuild will probably crash now. Sorry about that. If you need to upload sourcemaps without `bundle: true`, it is recommended to use Sentry CLI instead: https://docs.sentry.io/platforms/javascript/sourcemaps/uploading/cli/"
5776
);
5877
}
5978

6079
onResolve({ filter: /.*/ }, (args) => {
61-
if (args.kind !== "entry-point") {
80+
// Inject debug IDs into entry points and into imports from metadata proxy modules
81+
const isEntryPoint = args.kind === "entry-point";
82+
83+
// Check if this import is coming from a metadata proxy module
84+
// The metadata plugin registers entry points it wraps in the shared Set
85+
// We need to strip the query string suffix because esbuild includes the suffix
86+
// (e.g., ?sentryMetadataProxyModule=true) in args.importer
87+
const importerPath = args.importer?.split("?")[0];
88+
const isImportFromMetadataProxy =
89+
args.kind === "import-statement" &&
90+
importerPath !== undefined &&
91+
metadataProxyEntryPoints.has(importerPath);
92+
93+
if (!isEntryPoint && !isImportFromMetadataProxy) {
6294
return;
63-
} else {
64-
// Injected modules via the esbuild `inject` option do also have `kind == "entry-point"`.
65-
// We do not want to inject debug IDs into those files because they are already bundled into the entrypoints
66-
if (initialOptions.inject?.includes(args.path)) {
67-
return;
68-
}
95+
}
6996

70-
return {
71-
pluginName,
72-
// needs to be an abs path, otherwise esbuild will complain
73-
path: path.isAbsolute(args.path) ? args.path : path.join(args.resolveDir, args.path),
74-
pluginData: {
75-
isProxyResolver: true,
76-
originalPath: args.path,
77-
originalResolveDir: args.resolveDir,
78-
},
79-
// We need to add a suffix here, otherwise esbuild will mark the entrypoint as resolved and won't traverse
80-
// the module tree any further down past the proxy module because we're essentially creating a dependency
81-
// loop back to the proxy module.
82-
// By setting a suffix we're telling esbuild that the entrypoint and proxy module are two different things,
83-
// making it re-resolve the entrypoint when it is imported from the proxy module.
84-
// Super confusing? Yes. Works? Apparently... Let's see.
85-
suffix: "?sentryProxyModule=true",
86-
};
97+
// Skip injecting debug IDs into modules specified in the esbuild `inject` option
98+
// since they're already part of the entry points
99+
if (initialOptions.inject?.includes(args.path)) {
100+
return;
101+
}
102+
103+
const resolvedPath = path.isAbsolute(args.path)
104+
? args.path
105+
: path.join(args.resolveDir, args.path);
106+
107+
// Skip injecting debug IDs into paths that have already been wrapped
108+
if (debugIdWrappedPaths.has(resolvedPath)) {
109+
return;
87110
}
111+
debugIdWrappedPaths.add(resolvedPath);
112+
113+
return {
114+
pluginName,
115+
// needs to be an abs path, otherwise esbuild will complain
116+
path: resolvedPath,
117+
pluginData: {
118+
isProxyResolver: true,
119+
originalPath: args.path,
120+
originalResolveDir: args.resolveDir,
121+
},
122+
// We need to add a suffix here, otherwise esbuild will mark the entrypoint as resolved and won't traverse
123+
// the module tree any further down past the proxy module because we're essentially creating a dependency
124+
// loop back to the proxy module.
125+
// By setting a suffix we're telling esbuild that the entrypoint and proxy module are two different things,
126+
// making it re-resolve the entrypoint when it is imported from the proxy module.
127+
// Super confusing? Yes. Works? Apparently... Let's see.
128+
suffix: "?sentryProxyModule=true",
129+
};
88130
});
89131

90132
onLoad({ filter: /.*/ }, (args) => {
@@ -142,6 +184,9 @@ function esbuildModuleMetadataInjectionPlugin(injectionCode: string): UnpluginOp
142184

143185
esbuild: {
144186
setup({ initialOptions, onLoad, onResolve }) {
187+
// Clear state from previous builds (important for watch mode and test suites)
188+
metadataProxyEntryPoints.clear();
189+
145190
onResolve({ filter: /.*/ }, (args) => {
146191
if (args.kind !== "entry-point") {
147192
return;
@@ -152,10 +197,18 @@ function esbuildModuleMetadataInjectionPlugin(injectionCode: string): UnpluginOp
152197
return;
153198
}
154199

200+
const resolvedPath = path.isAbsolute(args.path)
201+
? args.path
202+
: path.join(args.resolveDir, args.path);
203+
204+
// Register this entry point so the debug ID plugin knows to wrap imports from
205+
// this proxy module, this because the debug ID may run after the metadata plugin
206+
metadataProxyEntryPoints.add(resolvedPath);
207+
155208
return {
156209
pluginName,
157210
// needs to be an abs path, otherwise esbuild will complain
158-
path: path.isAbsolute(args.path) ? args.path : path.join(args.resolveDir, args.path),
211+
path: resolvedPath,
159212
pluginData: {
160213
isMetadataProxyResolver: true,
161214
originalPath: args.path,
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/* eslint-disable jest/no-standalone-expect */
2+
/* eslint-disable jest/expect-expect */
3+
import { execSync } from "child_process";
4+
import path from "path";
5+
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";
6+
7+
interface BundleOutput {
8+
debugIds: Record<string, string> | undefined;
9+
metadata: Record<string, unknown> | undefined;
10+
}
11+
12+
function checkBundle(bundlePath: string): void {
13+
const output = execSync(`node ${bundlePath}`, { encoding: "utf-8" });
14+
const result = JSON.parse(output) as BundleOutput;
15+
16+
// Check that debug IDs are present
17+
expect(result.debugIds).toBeDefined();
18+
const debugIds = Object.values(result.debugIds ?? {});
19+
expect(debugIds.length).toBeGreaterThan(0);
20+
// Verify debug ID format (UUID v4)
21+
expect(debugIds).toContainEqual(
22+
expect.stringMatching(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/)
23+
);
24+
// The key should be a stack trace
25+
expect(Object.keys(result.debugIds ?? {})[0]).toContain("Error");
26+
27+
// Check that applicationKey metadata is present
28+
expect(result.metadata).toBeDefined();
29+
const metadataValues = Object.values(result.metadata ?? {});
30+
expect(metadataValues).toHaveLength(1);
31+
// applicationKey sets a special key in the metadata
32+
expect(metadataValues[0]).toEqual({ "_sentryBundlerPluginAppKey:my-app-key": true });
33+
// The key should be a stack trace
34+
expect(Object.keys(result.metadata ?? {})[0]).toContain("Error");
35+
}
36+
37+
describe("applicationKey with debug ID injection", () => {
38+
testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
39+
checkBundle(path.join(__dirname, "out", "webpack4", "bundle.js"));
40+
});
41+
42+
test("webpack 5 bundle", () => {
43+
checkBundle(path.join(__dirname, "out", "webpack5", "bundle.js"));
44+
});
45+
46+
test("esbuild bundle", () => {
47+
checkBundle(path.join(__dirname, "out", "esbuild", "bundle.js"));
48+
});
49+
50+
test("rollup bundle", () => {
51+
checkBundle(path.join(__dirname, "out", "rollup", "bundle.js"));
52+
});
53+
54+
test("vite bundle", () => {
55+
checkBundle(path.join(__dirname, "out", "vite", "bundle.js"));
56+
});
57+
});
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access */
2+
// Output both debug IDs and metadata to verify applicationKey works with debug ID injection
3+
// eslint-disable-next-line no-console
4+
console.log(
5+
JSON.stringify({
6+
debugIds: global._sentryDebugIds,
7+
metadata: global._sentryModuleMetadata,
8+
})
9+
);
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 { createCjsBundles } from "../../utils/create-cjs-bundles";
3+
4+
const outputDir = path.resolve(__dirname, "out");
5+
6+
createCjsBundles(
7+
{
8+
bundle: path.resolve(__dirname, "input", "bundle.js"),
9+
},
10+
outputDir,
11+
{
12+
// Enable applicationKey AND debug ID injection (sourcemaps enabled by default)
13+
applicationKey: "my-app-key",
14+
telemetry: false,
15+
release: { name: "test-release", create: false },
16+
},
17+
["webpack4", "webpack5", "esbuild", "rollup", "vite"]
18+
);
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access */
2+
// Output both debug IDs and metadata to verify both features work together
3+
// eslint-disable-next-line no-console
4+
console.log(
5+
JSON.stringify({
6+
debugIds: global._sentryDebugIds,
7+
metadata: global._sentryModuleMetadata,
8+
})
9+
);
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/* eslint-disable jest/no-standalone-expect */
2+
/* eslint-disable jest/expect-expect */
3+
import { execSync } from "child_process";
4+
import path from "path";
5+
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";
6+
7+
interface BundleOutput {
8+
debugIds: Record<string, string> | undefined;
9+
metadata: Record<string, unknown> | undefined;
10+
}
11+
12+
function checkBundle(bundlePath: string): void {
13+
const output = execSync(`node ${bundlePath}`, { encoding: "utf-8" });
14+
const result = JSON.parse(output) as BundleOutput;
15+
16+
// Check that debug IDs are present
17+
expect(result.debugIds).toBeDefined();
18+
const debugIds = Object.values(result.debugIds ?? {});
19+
expect(debugIds.length).toBeGreaterThan(0);
20+
// Verify debug ID format (UUID v4)
21+
expect(debugIds).toContainEqual(
22+
expect.stringMatching(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/)
23+
);
24+
// The key should be a stack trace
25+
expect(Object.keys(result.debugIds ?? {})[0]).toContain("Error");
26+
27+
// Check that metadata is present
28+
expect(result.metadata).toBeDefined();
29+
const metadataValues = Object.values(result.metadata ?? {});
30+
expect(metadataValues).toHaveLength(1);
31+
expect(metadataValues).toEqual([{ team: "frontend" }]);
32+
// The key should be a stack trace
33+
expect(Object.keys(result.metadata ?? {})[0]).toContain("Error");
34+
}
35+
36+
describe("metadata with debug ID injection", () => {
37+
testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
38+
checkBundle(path.join(__dirname, "out", "webpack4", "bundle.js"));
39+
});
40+
41+
test("webpack 5 bundle", () => {
42+
checkBundle(path.join(__dirname, "out", "webpack5", "bundle.js"));
43+
});
44+
45+
test("esbuild bundle", () => {
46+
checkBundle(path.join(__dirname, "out", "esbuild", "bundle.js"));
47+
});
48+
49+
test("rollup bundle", () => {
50+
checkBundle(path.join(__dirname, "out", "rollup", "bundle.js"));
51+
});
52+
53+
test("vite bundle", () => {
54+
checkBundle(path.join(__dirname, "out", "vite", "bundle.js"));
55+
});
56+
});
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 { createCjsBundles } from "../../utils/create-cjs-bundles";
3+
4+
const outputDir = path.resolve(__dirname, "out");
5+
6+
createCjsBundles(
7+
{
8+
bundle: path.resolve(__dirname, "input", "bundle.js"),
9+
},
10+
outputDir,
11+
{
12+
// Enable both moduleMetadata AND debug ID injection (sourcemaps enabled by default)
13+
moduleMetadata: { team: "frontend" },
14+
telemetry: false,
15+
release: { name: "test-release", create: false },
16+
},
17+
["webpack4", "webpack5", "esbuild", "rollup", "vite"]
18+
);

0 commit comments

Comments
 (0)