Skip to content

fix(plugin-manager): Respect sourcemap.ignore values for injecting debugIDs #836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
s1gr1d merged 4 commits into from
Jan 16, 2026
+52 −3
Merged

fix(plugin-manager): Respect sourcemap.ignore values for injecting debugIDs #836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
985b74a
fix(plugin-manager): Respect `sourcemap.ignore` values for injecting …
s1gr1d Jan 15, 2026
c7eaa5e
fix order
s1gr1d Jan 15, 2026
19271c1
fix tests
s1gr1d Jan 15, 2026
7518e3a
Merge branch 'main' into sig/add-ignore-to-inject
s1gr1d Jan 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/bundler-plugin-core/src/build-plugin-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
arrayify,
getProjects,
getTurborepoEnvPassthroughWarning,
serializeIgnoreOptions,
stripQueryAndHashFromPath,
} from "./utils";
import { glob } from "glob";
Expand Down Expand Up @@ -554,7 +555,12 @@ export function createSentryBuildPluginManager(
try {
const cliInstance = createCliInstance(options);
await cliInstance.execute(
["sourcemaps", "inject", ...buildArtifactPaths],
[
"sourcemaps",
"inject",
...serializeIgnoreOptions(options.sourcemaps?.ignore),
Copy link

@sentry sentry bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The injectDebugIds function defaults to ignoring node_modules, but uploadSourcemaps does not, causing source maps to be uploaded without debug IDs.
Severity: HIGH

Suggested Fix

Ensure the uploadSourcemaps function applies the same default ignore behavior as injectDebugIds. When options.sourcemaps?.ignore is undefined, it should default to ignoring ["node_modules"]. This will align the set of files that have debug IDs injected with the set of files that are uploaded as source maps.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/bundler-plugin-core/src/build-plugin-manager.ts#L561

Potential issue: There is an inconsistency in how default ignore behavior is handled
between `injectDebugIds` and `uploadSourcemaps`. When `options.sourcemaps?.ignore` is
undefined, `injectDebugIds` defaults to ignoring `node_modules`. However,
`uploadSourcemaps` does not apply this default in either its direct upload or prepare
modes. This causes source maps from `node_modules` to be uploaded without the
corresponding debug IDs being injected. As a result, the association between the source
code and the error in Sentry is broken for these files, rendering the uploaded source
maps unusable and wasting upload bandwidth.

Did we get this right? 👍 / 👎 to inform future reviews.

...buildArtifactPaths,
],
options.debug ? "rejectOnError" : false
);
} catch (e) {
Expand Down
20 changes: 20 additions & 0 deletions packages/bundler-plugin-core/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,3 +408,23 @@ export function getProjects(project: string | string[] | undefined): string[] |

return undefined;
}

/**
* Inlined functionality from @sentry/cli helper code to add `--ignore` options.
*
* Temporary workaround until we expose a function for injecting debug IDs. Currently, we directly call `execute` with CLI args to inject them.
*/
export function serializeIgnoreOptions(ignoreValue: string | string[] | undefined): string[] {
const DEFAULT_IGNORE = ["node_modules"];
Copy link
Member

@Lms24 Lms24 Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I wonder if including node_modules has any implications but I don't think so: We inject debug ids into emitted bundles, meaning the node_modules required by the bundle should already be in there.

Also, the ignore option for upload also defaults to node_modules, right?

Copy link
Member Author

@s1gr1d s1gr1d Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not ignoring it can result in file import loops (this is what happens in e.g. Nuxt).

And yes, it's also the default option for uploading source maps.


const ignoreOptions: string[] = Array.isArray(ignoreValue)
? ignoreValue
: typeof ignoreValue === "string"
? [ignoreValue]
: DEFAULT_IGNORE;
Copy link
Member

@chargome chargome Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if we don't always want to add this ignore? In the sense that a user should likely never inject debug ids in node_modules but they usually want to additionally ignore other paths etc. no?

Copy link
Member Author

@s1gr1d s1gr1d Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! I just resembled the existing logic from the CLI here (user-provided value overwrites): https://github.com/getsentry/sentry-cli/blob/a618fb33ddfc3646ac8d5e2391ea1ee6e208b709/lib/releases/index.ts#L159-L161

Either we document, that ignore has a default value of 'node_modules', which gets overwritten or we just always add 'node_modules'. I am leaning towards just better documenting this default value, so it's still possible to overwrite it.

@Lms24 any opinions on that?

Copy link
Member

@Lms24 Lms24 Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So according to this doc, the default is [], which seems like its wrong either way (?).

I am leaning towards just better documenting this default value, so it's still possible to overwrite it.

Me too, because the alternative would be a behaviour break, right? So let's avoid that 😅

Copy link
Member Author

@s1gr1d s1gr1d Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default is [], which seems like its wrong either way

In the JS bundler plugins, this is correct. But this value is passed down a bunch of times:

CLI Bundler Plugins Meta-Framework SDK
adds 'node_modules' as default uses CLI in plugin manager (no defaults) uses bundler plugins (no defaults)

Maybe it's worth applying this default in the bundler plugins already, as we could align the behavior better on this stage.

Only the Plugin Manger uses the CLI under the hood, and there are no defaults otherwise.

Copy link
Member Author

@s1gr1d s1gr1d Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked offline, created an issue: #838


return ignoreOptions.reduce(
(acc, value) => acc.concat(["--ignore", String(value)]),
[] as string[]
);
}
Comment on lines +417 to +430
Copy link
Member Author

@s1gr1d s1gr1d Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a simplified version of: https://github.com/getsentry/sentry-cli/blob/9f34df859bf2975155aad0fe5c6c423b45c07084/lib/helper.ts#L199-L217

4 changes: 2 additions & 2 deletions packages/bundler-plugin-core/test/build-plugin-manager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ describe("createSentryBuildPluginManager", () => {
await buildPluginManager.injectDebugIds(buildArtifactPaths);

expect(mockCliExecute).toHaveBeenCalledWith(
["sourcemaps", "inject", "/path/to/1", "/path/to/2"],
["sourcemaps", "inject", "--ignore", "node_modules", "/path/to/1", "/path/to/2"],
false
);
});
Expand All @@ -459,7 +459,7 @@ describe("createSentryBuildPluginManager", () => {
await buildPluginManager.injectDebugIds(buildArtifactPaths);

expect(mockCliExecute).toHaveBeenCalledWith(
["sourcemaps", "inject", "/path/to/bundle"],
["sourcemaps", "inject", "--ignore", "node_modules", "/path/to/bundle"],
"rejectOnError"
);
});
Expand Down
23 changes: 23 additions & 0 deletions packages/bundler-plugin-core/test/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getPackageJson,
parseMajorVersion,
replaceBooleanFlagsInCode,
serializeIgnoreOptions,
stringToUUID,
} from "../src/utils";

Expand Down Expand Up @@ -270,3 +271,25 @@ describe("generateModuleMetadataInjectorCode", () => {
expect(generatedCode).toMatchSnapshot();
});
});

describe("serializeIgnoreOptions", () => {
it("returns default ignore options when undefined", () => {
const result = serializeIgnoreOptions(undefined);
expect(result).toEqual(["--ignore", "node_modules"]);
});

it("handles array of ignore patterns", () => {
const result = serializeIgnoreOptions(["dist", "**/build/**", "*.log"]);
expect(result).toEqual(["--ignore", "dist", "--ignore", "**/build/**", "--ignore", "*.log"]);
});

it("handles single string pattern", () => {
const result = serializeIgnoreOptions("dist");
expect(result).toEqual(["--ignore", "dist"]);
});

it("handles empty array", () => {
const result = serializeIgnoreOptions([]);
expect(result).toEqual([]);
});
});
Loading