Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
193 changes: 111 additions & 82 deletions packages/bundler-plugin-core/src/build-plugin-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ export type SentryBuildPluginManager = {
/**
* Uploads sourcemaps using the "Debug ID" method. This function takes a list of build artifact paths that will be uploaded
*/
uploadSourcemaps(buildArtifactPaths: string[]): Promise<void>;
uploadSourcemaps(
buildArtifactPaths: string[],
opts?: { prepareArtifacts?: boolean }
): Promise<void>;

/**
* Will delete artifacts based on the passed `sourcemaps.filesToDeleteAfterUpload` option.
Expand Down Expand Up @@ -546,9 +549,16 @@ export function createSentryBuildPluginManager(
},

/**
* Uploads sourcemaps using the "Debug ID" method. This function takes a list of build artifact paths that will be uploaded
* Uploads sourcemaps using the "Debug ID" method.
*
* By default, this prepares bundles in a temporary folder before uploading. You can opt into an
* in-place, direct upload path by setting `prepareArtifacts` to `false`. If `prepareArtifacts` is set to
* `false`, no preparation (e.g. adding `//# debugId=...` and writing adjusted source maps) is performed and no temp folder is used.
*
* @param buildArtifactPaths - The paths of the build artifacts to upload
* @param opts - Optional flags to control temp folder usage and preparation
*/
async uploadSourcemaps(buildArtifactPaths: string[]) {
async uploadSourcemaps(buildArtifactPaths: string[], opts?: { prepareArtifacts?: boolean }) {
if (!canUploadSourceMaps(options, logger, isDevMode)) {
return;
}
Expand All @@ -557,26 +567,16 @@ export function createSentryBuildPluginManager(
// This is `forceTransaction`ed because this span is used in dashboards in the form of indexed transactions.
{ name: "debug-id-sourcemap-upload", scope: sentryScope, forceTransaction: true },
async () => {
// If we're not using a temp folder, we must not prepare artifacts in-place (to avoid mutating user files)
const shouldPrepare = opts?.prepareArtifacts ?? true;

let folderToCleanUp: string | undefined;

// It is possible that this writeBundle hook (which calls this function) is called multiple times in one build (for example when reusing the plugin, or when using build tooling like `@vitejs/plugin-legacy`)
// Therefore we need to actually register the execution of this hook as dependency on the sourcemap files.
const freeUploadDependencyOnBuildArtifacts = createDependencyOnBuildArtifacts();

try {
const tmpUploadFolder = await startSpan(
{ name: "mkdtemp", scope: sentryScope },
async () => {
return (
process.env?.["SENTRY_TEST_OVERRIDE_TEMP_DIR"] ||
(await fs.promises.mkdtemp(
path.join(os.tmpdir(), "sentry-bundler-plugin-upload-")
))
);
}
);

folderToCleanUp = tmpUploadFolder;
const assets = options.sourcemaps?.assets;

let globAssets: string | string[];
Expand All @@ -594,14 +594,17 @@ export function createSentryBuildPluginManager(
async () =>
await glob(globAssets, {
absolute: true,
nodir: true,
// If we do not use a temp folder, we allow directories and files; CLI will traverse as needed when given paths.
nodir: shouldPrepare,
ignore: options.sourcemaps?.ignore,
})
);

const debugIdChunkFilePaths = globResult.filter((debugIdChunkFilePath) => {
return !!stripQueryAndHashFromPath(debugIdChunkFilePath).match(/\.(js|mjs|cjs)$/);
});
const debugIdChunkFilePaths = shouldPrepare
? globResult.filter((debugIdChunkFilePath) => {
return !!stripQueryAndHashFromPath(debugIdChunkFilePath).match(/\.(js|mjs|cjs)$/);
})
: globResult;

// The order of the files output by glob() is not deterministic
// Ensure order within the files so that {debug-id}-{chunkIndex} coupling is consistent
Expand All @@ -616,75 +619,101 @@ export function createSentryBuildPluginManager(
"Didn't find any matching sources for debug ID upload. Please check the `sourcemaps.assets` option."
);
} else {
const numUploadedFiles = await startSpan(
{ name: "prepare-bundles", scope: sentryScope },
async (prepBundlesSpan) => {
// Preparing the bundles can be a lot of work and doing it all at once has the potential of nuking the heap so
// instead we do it with a maximum of 16 concurrent workers
const preparationTasks = debugIdChunkFilePaths.map(
(chunkFilePath, chunkIndex) => async () => {
await prepareBundleForDebugIdUpload(
chunkFilePath,
tmpUploadFolder,
chunkIndex,
logger,
options.sourcemaps?.rewriteSources ?? defaultRewriteSourcesHook,
options.sourcemaps?.resolveSourceMap
);
}
);
const workers: Promise<void>[] = [];
const worker = async (): Promise<void> => {
while (preparationTasks.length > 0) {
const task = preparationTasks.shift();
if (task) {
await task();
if (!shouldPrepare) {
// Direct CLI upload from existing artifact paths (no preparation or temp copies)
await startSpan({ name: "upload", scope: sentryScope }, async () => {
const cliInstance = createCliInstance(options);
await cliInstance.releases.uploadSourceMaps(options.release.name ?? "undefined", {
include: [
{
paths: debugIdChunkFilePaths,
rewrite: false,
dist: options.release.dist,
},
],
live: "rejectOnError",
});
});

logger.info("Successfully uploaded source maps to Sentry");
} else {
const tmpUploadFolder = await startSpan(
{ name: "mkdtemp", scope: sentryScope },
async () => {
return (
process.env?.["SENTRY_TEST_OVERRIDE_TEMP_DIR"] ||
(await fs.promises.mkdtemp(
path.join(os.tmpdir(), "sentry-bundler-plugin-upload-")
))
);
}
);
folderToCleanUp = tmpUploadFolder;

// Prepare into temp folder, then upload
await startSpan(
{ name: "prepare-bundles", scope: sentryScope },
async (prepBundlesSpan) => {
// Preparing the bundles can be a lot of work and doing it all at once has the potential of nuking the heap so
// instead we do it with a maximum of 16 concurrent workers
const preparationTasks = debugIdChunkFilePaths.map(
(chunkFilePath, chunkIndex) => async () => {
await prepareBundleForDebugIdUpload(
chunkFilePath,
tmpUploadFolder,
chunkIndex,
logger,
options.sourcemaps?.rewriteSources ?? defaultRewriteSourcesHook,
options.sourcemaps?.resolveSourceMap
);
}
);
const workers: Promise<void>[] = [];
const worker = async (): Promise<void> => {
while (preparationTasks.length > 0) {
const task = preparationTasks.shift();
if (task) {
await task();
}
}
};
for (let workerIndex = 0; workerIndex < 16; workerIndex++) {
workers.push(worker());
}
};
for (let workerIndex = 0; workerIndex < 16; workerIndex++) {
workers.push(worker());
}

await Promise.all(workers);
await Promise.all(workers);

const files = await fs.promises.readdir(tmpUploadFolder);
const stats = files.map((file) =>
fs.promises.stat(path.join(tmpUploadFolder, file))
);
const uploadSize = (await Promise.all(stats)).reduce(
(accumulator, { size }) => accumulator + size,
0
);

setMeasurement("files", files.length, "none", prepBundlesSpan);
setMeasurement("upload_size", uploadSize, "byte", prepBundlesSpan);

await startSpan({ name: "upload", scope: sentryScope }, async () => {
const cliInstance = createCliInstance(options);

await cliInstance.releases.uploadSourceMaps(
options.release.name ?? "undefined", // unfortunately this needs a value for now but it will not matter since debug IDs overpower releases anyhow
{
include: [
{
paths: [tmpUploadFolder],
rewrite: false,
dist: options.release.dist,
},
],
// We want this promise to throw if the sourcemaps fail to upload so that we know about it.
// see: https://github.com/getsentry/sentry-cli/pull/2605
live: "rejectOnError",
}
const files = await fs.promises.readdir(tmpUploadFolder);
const stats = files.map((file) =>
fs.promises.stat(path.join(tmpUploadFolder, file))
);
const uploadSize = (await Promise.all(stats)).reduce(
(accumulator, { size }) => accumulator + size,
0
);
});

return files.length;
}
);
setMeasurement("files", files.length, "none", prepBundlesSpan);
setMeasurement("upload_size", uploadSize, "byte", prepBundlesSpan);

await startSpan({ name: "upload", scope: sentryScope }, async () => {
const cliInstance = createCliInstance(options);
await cliInstance.releases.uploadSourceMaps(
options.release.name ?? "undefined",
{
include: [
{
paths: [tmpUploadFolder],
rewrite: false,
dist: options.release.dist,
},
],
live: "rejectOnError",
}
);
});
}
);

if (numUploadedFiles > 0) {
logger.info("Successfully uploaded source maps to Sentry");
}
}
Expand Down
105 changes: 103 additions & 2 deletions packages/bundler-plugin-core/test/build-plugin-manager.test.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,46 @@
import { createSentryBuildPluginManager } from "../src/build-plugin-manager";
import fs from "fs";
import { glob } from "glob";
import { prepareBundleForDebugIdUpload } from "../src/debug-id-upload";

const mockCliExecute = jest.fn();
const mockCliUploadSourceMaps = jest.fn();

jest.mock("@sentry/cli", () => {
return jest.fn().mockImplementation(() => ({
execute: mockCliExecute,
releases: {
uploadSourceMaps: mockCliUploadSourceMaps,
new: jest.fn(),
finalize: jest.fn(),
setCommits: jest.fn(),
newDeploy: jest.fn(),
},
}));
});

// eslint-disable-next-line @typescript-eslint/no-unsafe-return
jest.mock("../src/sentry/telemetry", () => ({
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
...jest.requireActual("../src/sentry/telemetry"),
safeFlushTelemetry: jest.fn(),
}));

// eslint-disable-next-line @typescript-eslint/no-unsafe-return
jest.mock("@sentry/core", () => ({
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
...jest.requireActual("@sentry/core"),
// eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-return
startSpan: jest.fn((options, callback) => callback()),
startSpan: jest.fn((options: unknown, callback: () => unknown) => callback()),
}));

jest.mock("glob");
jest.mock("../src/debug-id-upload");

const mockGlob = glob as jest.MockedFunction<typeof glob>;
const mockPrepareBundleForDebugIdUpload = prepareBundleForDebugIdUpload as jest.MockedFunction<
typeof prepareBundleForDebugIdUpload
>;

describe("createSentryBuildPluginManager", () => {
beforeEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -73,6 +94,86 @@ describe("createSentryBuildPluginManager", () => {
});
});

describe("uploadSourcemaps", () => {
it("uploads in-place when prepareArtifacts is false", async () => {
mockCliUploadSourceMaps.mockResolvedValue(undefined);

// Return a mixture of files/dirs; in-place path should pass through as-is
mockGlob.mockResolvedValue(["/app/dist/a.js", "/app/dist/dir", "/app/dist/a.js.map"]);

const manager = createSentryBuildPluginManager(
{
authToken: "t",
org: "o",
project: "p",
release: { name: "some-release-name", dist: "1" },
sourcemaps: { assets: ["/app/dist/**/*"] },
},
{ buildTool: "webpack", loggerPrefix: "[sentry-webpack-plugin]" }
);

await manager.uploadSourcemaps(["/unused"], { prepareArtifacts: false });

expect(mockCliUploadSourceMaps).toHaveBeenCalledTimes(1);
expect(mockCliUploadSourceMaps).toHaveBeenCalledWith(
"some-release-name",
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
expect.objectContaining({
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
include: expect.arrayContaining([
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
expect.objectContaining({
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
paths: expect.arrayContaining([
"/app/dist/a.js",
"/app/dist/dir",
"/app/dist/a.js.map",
]),
rewrite: false,
dist: "1",
}),
]),
live: "rejectOnError",
})
);
expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled();
});

it("prepares into temp folder and uploads when prepareArtifacts is true (default)", async () => {
mockCliUploadSourceMaps.mockResolvedValue(undefined);

mockGlob.mockResolvedValue(["/app/dist/a.js", "/app/dist/a.js.map", "/app/dist/other.txt"]);

jest.spyOn(fs.promises, "mkdtemp").mockResolvedValue("/tmp/sentry-upload-xyz");
jest.spyOn(fs.promises, "readdir").mockResolvedValue(["a.js", "a.js.map"] as never);
jest.spyOn(fs.promises, "stat").mockResolvedValue({ size: 10 } as fs.Stats);
jest.spyOn(fs.promises, "rm").mockResolvedValue(undefined as never);

mockPrepareBundleForDebugIdUpload.mockResolvedValue(undefined);

const manager = createSentryBuildPluginManager(
{
authToken: "t",
org: "o",
project: "p",
release: { name: "some-release-name", dist: "1" },
sourcemaps: { assets: ["/app/dist/**/*"] },
},
{ buildTool: "webpack", loggerPrefix: "[sentry-webpack-plugin]" }
);

await manager.uploadSourcemaps(["/unused"]);

// Should call prepare for each JS chunk discovered by glob
expect(mockPrepareBundleForDebugIdUpload).toHaveBeenCalled();
// Should upload from temp folder
expect(mockCliUploadSourceMaps).toHaveBeenCalledWith("some-release-name", {
include: [{ paths: ["/tmp/sentry-upload-xyz"], rewrite: false, dist: "1" }],
live: "rejectOnError",
});
});
});

describe("injectDebugIds", () => {
it("should call CLI with correct sourcemaps inject command", async () => {
mockCliExecute.mockResolvedValue(undefined);
Expand Down
Loading