Skip to content

Commit 4040760

Browse files
committed
feat(bundler-plugins): Enable external source maps and delete them after upload in the esbuild plugin
esbuild writes its output before `onEnd` runs and has no hook to drop an output, so the maps are deleted from disk after a successful upload instead. When the upload fails they stay, with an error log.
1 parent 4709561 commit 4040760

5 files changed

Lines changed: 218 additions & 11 deletions

File tree

‎dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic-cjs.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => {
3535
"sentry-cli-mock.json": "["release","create","CURRENT_SHA","--project","fake-project"],
3636
["release","set-commits","CURRENT_SHA","--auto"],
3737
["release","finalize","CURRENT_SHA"],
38+
["sourcemap","upload","-p","fake-project","--release","CURRENT_SHA","sentry-bundler-plugin-upload-path"],
3839
",
3940
}
4041
`);

‎dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => {
3535
"sentry-cli-mock.json": "["release","create","CURRENT_SHA","--project","fake-project"],
3636
["release","set-commits","CURRENT_SHA","--auto"],
3737
["release","finalize","CURRENT_SHA"],
38+
["sourcemap","upload","-p","fake-project","--release","CURRENT_SHA","sentry-bundler-plugin-upload-path"],
3839
",
3940
}
4041
`);

‎dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/release-disabled.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => {
3434
",
3535
"sentry-cli-mock.json": "["release","set-commits","CURRENT_SHA","--auto"],
3636
["release","finalize","CURRENT_SHA"],
37+
["sourcemap","upload","-p","fake-project","--release","CURRENT_SHA","sentry-bundler-plugin-upload-path"],
3738
",
3839
}
3940
`);

‎packages/bundler-plugins/src/esbuild/index.ts‎

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Options } from '../core';
1+
import type { Options, SourceMapKind } from '../core';
22
import {
33
createSentryBuildPluginManager,
44
generateReleaseInjectorCode,
@@ -9,6 +9,7 @@ import {
99
addDebugIdToEmittedArtifacts,
1010
isJsFile,
1111
} from '../core';
12+
import * as fs from 'node:fs';
1213
import * as path from 'node:path';
1314
import { createRequire } from 'node:module';
1415
import { randomUUID } from 'node:crypto';
@@ -48,13 +49,33 @@ interface EsbuildOnEndArgs {
4849
};
4950
}
5051

52+
type EsbuildSourceMapSetting = boolean | 'inline' | 'external' | 'linked' | 'both' | undefined;
53+
5154
interface EsbuildInitialOptions {
5255
bundle?: boolean;
5356
inject?: string[];
5457
metafile?: boolean;
5558
define?: Record<string, string>;
5659
write?: boolean;
5760
absWorkingDir?: string;
61+
sourcemap?: EsbuildSourceMapSetting;
62+
}
63+
64+
/** Maps esbuild's `sourcemap` setting onto the plugin's source map kinds. */
65+
function sourceMapKindFromEsbuildSetting(setting: EsbuildSourceMapSetting): SourceMapKind {
66+
switch (setting) {
67+
case undefined:
68+
return 'unset';
69+
case false:
70+
return 'off';
71+
case 'external':
72+
return 'hidden';
73+
case 'inline':
74+
case 'both':
75+
return 'inline';
76+
default:
77+
return 'visible';
78+
}
5879
}
5980

6081
interface EsbuildPluginBuild {
@@ -280,6 +301,13 @@ export function sentryEsbuildPlugin(userOptions: Options = {}): any {
280301
});
281302
}
282303

304+
const sourceMapDecision = sentryBuildPluginManager.sourceMaps.decide(
305+
sourceMapKindFromEsbuildSetting(initialOptions.sourcemap),
306+
);
307+
if (sourceMapDecision.enableHidden) {
308+
initialOptions.sourcemap = 'external';
309+
}
310+
283311
// Create release and optionally upload
284312
const freeGlobalDependencyOnBuildArtifacts = createDependencyOnBuildArtifacts();
285313
const upload = createDebugIdUploadFunction({ sentryBuildPluginManager });
@@ -290,27 +318,36 @@ export function sentryEsbuildPlugin(userOptions: Options = {}): any {
290318
await sentryBuildPluginManager.createRelease();
291319

292320
if (sourcemapsEnabled) {
293-
const buildArtifacts = result.metafile ? Object.keys(result.metafile.outputs) : [];
321+
// The metafile lists outputs relative to `absWorkingDir`, not to the process.
322+
const outputDir = initialOptions.absWorkingDir ?? process.cwd();
323+
const buildArtifacts = result.metafile
324+
? Object.keys(result.metafile.outputs).map(output => path.resolve(outputDir, output))
325+
: [];
294326

295327
if (options.sourcemaps?.disable !== 'disable-upload') {
296-
await upload(buildArtifacts);
328+
const { uploaded } = await upload(buildArtifacts);
329+
330+
// esbuild writes before `onEnd` runs, so the maps are deleted from disk
331+
// afterwards instead of being dropped from the output like in other bundlers.
332+
if (sourceMapDecision.deleteAfterUpload && initialOptions.write !== false) {
333+
const sourceMaps = buildArtifacts.filter(artifact => artifact.endsWith('.map'));
334+
335+
if (uploaded) {
336+
await Promise.all(sourceMaps.map(sourceMap => fs.promises.rm(sourceMap, { force: true })));
337+
} else if (sourceMaps.length > 0) {
338+
logger.error('The source maps were not uploaded, so they stay in the build output.');
339+
}
340+
}
297341
} else if (initialOptions.write === false) {
298342
logger.debug('Build output is not written to disk. Skipping debug ID injection into build artifacts.');
299343
} else {
300344
// The upload routine (which stamps debug IDs into temp copies of the artifacts) is
301345
// skipped with `disable-upload`. esbuild has no hook to modify outputs before they are
302346
// written, so the emitted artifacts get stamped on disk instead.
303-
const outputDir = initialOptions.absWorkingDir ?? process.cwd();
304347
await Promise.all(
305348
buildArtifacts
306349
.filter(isJsFile)
307-
.map(bundle =>
308-
addDebugIdToEmittedArtifacts(
309-
path.resolve(outputDir, bundle),
310-
logger,
311-
options.sourcemaps?.resolveSourceMap,
312-
),
313-
),
350+
.map(bundle => addDebugIdToEmittedArtifacts(bundle, logger, options.sourcemaps?.resolveSourceMap)),
314351
);
315352
}
316353
}
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import * as fs from 'fs';
2+
import * as os from 'os';
3+
import * as path from 'path';
4+
import * as esbuild from 'esbuild';
5+
import type { BuildOptions } from 'esbuild';
6+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
7+
import { sentryEsbuildPlugin } from '../../src/esbuild';
8+
import type { Options } from '../../src/core';
9+
10+
const { mockSourcemapUpload } = vi.hoisted(() => ({ mockSourcemapUpload: vi.fn() }));
11+
12+
vi.mock('sentry', () => ({
13+
createSentrySDK: () => ({
14+
release: { create: vi.fn(), finalize: vi.fn(), 'set-commits': vi.fn(), deploy: vi.fn() },
15+
sourcemap: { upload: mockSourcemapUpload, inject: vi.fn() },
16+
run: vi.fn(),
17+
}),
18+
}));
19+
20+
const DEBUG_ID_MARKER = /sentry-dbid-([0-9a-f-]{36})/;
21+
22+
const credentials: Options = {
23+
authToken: 't',
24+
org: 'o',
25+
project: 'p',
26+
release: { name: 'test-release', create: false, finalize: false },
27+
telemetry: false,
28+
silent: true,
29+
};
30+
31+
type Upload = { directory: string; files: Record<string, string> };
32+
33+
describe('sourcemaps.deleteSourcemapsAfterUpload (esbuild)', () => {
34+
let tmpDir: string;
35+
let entry: string;
36+
let outDir: string;
37+
let uploads: Upload[];
38+
39+
beforeEach(() => {
40+
vi.clearAllMocks();
41+
uploads = [];
42+
mockSourcemapUpload.mockImplementation(async ({ directory }: { directory: string }) => {
43+
const files: Record<string, string> = {};
44+
for (const file of fs.readdirSync(directory)) {
45+
files[file] = fs.readFileSync(path.join(directory, file), 'utf8');
46+
}
47+
uploads.push({ directory, files });
48+
});
49+
50+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sentry-esbuild-delete-maps-'));
51+
entry = path.join(tmpDir, 'entry.js');
52+
outDir = path.join(tmpDir, 'dist');
53+
fs.writeFileSync(entry, 'export const answer = 42;\nconsole.log(answer);\n');
54+
});
55+
56+
afterEach(() => {
57+
vi.restoreAllMocks();
58+
fs.rmSync(tmpDir, { recursive: true, force: true });
59+
});
60+
61+
function build(pluginOptions: Options = {}, overrides: BuildOptions = {}) {
62+
return esbuild.build({
63+
entryPoints: [entry],
64+
bundle: true,
65+
outdir: outDir,
66+
absWorkingDir: tmpDir,
67+
plugins: [sentryEsbuildPlugin({ ...credentials, ...pluginOptions })],
68+
...overrides,
69+
});
70+
}
71+
72+
function expectUploadedPair(upload: Upload, bundle: string): void {
73+
const debugId = bundle.match(DEBUG_ID_MARKER)?.[1];
74+
expect(debugId).toBeDefined();
75+
expect(Object.keys(upload.files).sort()).toEqual([`${debugId}-0.js`, `${debugId}-0.js.map`]);
76+
expect(JSON.parse(upload.files[`${debugId}-0.js.map`]!)).toMatchObject({ debug_id: debugId });
77+
expect(fs.existsSync(upload.directory)).toBe(false);
78+
}
79+
80+
it('enables external source maps, uploads them and deletes them when the setting is unset', async () => {
81+
await build();
82+
83+
expect(fs.readdirSync(outDir)).toEqual(['entry.js']);
84+
const bundle = fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8');
85+
expect(bundle).not.toContain('sourceMappingURL');
86+
87+
expect(uploads).toHaveLength(1);
88+
expectUploadedPair(uploads[0]!, bundle);
89+
});
90+
91+
it('switches linked source maps to external and deletes them when the option is true', async () => {
92+
await build({ sourcemaps: { deleteSourcemapsAfterUpload: true } }, { sourcemap: true });
93+
94+
expect(fs.readdirSync(outDir)).toEqual(['entry.js']);
95+
expect(fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8')).not.toContain('sourceMappingURL');
96+
expect(uploads).toHaveLength(1);
97+
});
98+
99+
it('keeps external source maps the user enabled', async () => {
100+
await build({}, { sourcemap: 'external' });
101+
102+
expect(fs.readdirSync(outDir).sort()).toEqual(['entry.js', 'entry.js.map']);
103+
expect(uploads).toHaveLength(1);
104+
expectUploadedPair(uploads[0]!, fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8'));
105+
});
106+
107+
it('keeps linked source maps when the option is unset', async () => {
108+
await build({}, { sourcemap: true });
109+
110+
expect(fs.readdirSync(outDir).sort()).toEqual(['entry.js', 'entry.js.map']);
111+
expect(fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8')).toContain('//# sourceMappingURL=entry.js.map');
112+
});
113+
114+
it('does not enable source maps when the option is false', async () => {
115+
await build({ sourcemaps: { deleteSourcemapsAfterUpload: false } });
116+
117+
expect(fs.readdirSync(outDir)).toEqual(['entry.js']);
118+
expect(uploads).toHaveLength(0);
119+
});
120+
121+
it('warns when source maps are turned off', async () => {
122+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
123+
await build({ silent: false }, { sourcemap: false });
124+
125+
expect(fs.readdirSync(outDir)).toEqual(['entry.js']);
126+
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Source map generation is turned off'));
127+
});
128+
129+
it('warns when source maps are inlined and keeps them', async () => {
130+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
131+
await build({ silent: false, sourcemaps: { deleteSourcemapsAfterUpload: true } }, { sourcemap: 'inline' });
132+
133+
expect(fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8')).toContain('sourceMappingURL=data:');
134+
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Source maps are inlined'));
135+
});
136+
137+
it('does not enable source maps when no upload can run', async () => {
138+
await build({ authToken: undefined });
139+
140+
expect(fs.readdirSync(outDir)).toEqual(['entry.js']);
141+
expect(uploads).toHaveLength(0);
142+
});
143+
144+
it('does not delete anything when the build does not write', async () => {
145+
const rmSpy = vi.spyOn(fs.promises, 'rm');
146+
const result = await build({}, { write: false });
147+
148+
expect(result.outputFiles?.map(file => path.basename(file.path)).sort()).toEqual(['entry.js', 'entry.js.map']);
149+
expect(rmSpy).not.toHaveBeenCalledWith(expect.stringContaining('entry.js.map'), expect.anything());
150+
});
151+
152+
it('keeps the source maps when the upload fails', async () => {
153+
mockSourcemapUpload.mockRejectedValue(new Error('upload failed'));
154+
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined);
155+
await build({ silent: false });
156+
157+
expect(fs.readdirSync(outDir).sort()).toEqual(['entry.js', 'entry.js.map']);
158+
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('were not uploaded'));
159+
});
160+
161+
it('leaves "disable-upload" builds untouched', async () => {
162+
await build({ sourcemaps: { disable: 'disable-upload' } }, { sourcemap: true });
163+
164+
expect(fs.readdirSync(outDir).sort()).toEqual(['entry.js', 'entry.js.map']);
165+
expect(uploads).toHaveLength(0);
166+
});
167+
});

0 commit comments

Comments
 (0)