diff --git a/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic-cjs.test.ts b/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic-cjs.test.ts index e91fb5ed133b..f07edbcb8c58 100644 --- a/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic-cjs.test.ts +++ b/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic-cjs.test.ts @@ -35,6 +35,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => { "sentry-cli-mock.json": "["release","create","CURRENT_SHA","--project","fake-project"], ["release","set-commits","CURRENT_SHA","--auto"], ["release","finalize","CURRENT_SHA"], + ["sourcemap","upload","-p","fake-project","--release","CURRENT_SHA","sentry-bundler-plugin-upload-path"], ", } `); diff --git a/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic.test.ts b/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic.test.ts index e91fb5ed133b..f07edbcb8c58 100644 --- a/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic.test.ts +++ b/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/basic.test.ts @@ -35,6 +35,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => { "sentry-cli-mock.json": "["release","create","CURRENT_SHA","--project","fake-project"], ["release","set-commits","CURRENT_SHA","--auto"], ["release","finalize","CURRENT_SHA"], + ["sourcemap","upload","-p","fake-project","--release","CURRENT_SHA","sentry-bundler-plugin-upload-path"], ", } `); diff --git a/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/release-disabled.test.ts b/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/release-disabled.test.ts index 64fdb039b963..7aac92da1976 100644 --- a/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/release-disabled.test.ts +++ b/dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/release-disabled.test.ts @@ -34,6 +34,7 @@ test(import.meta.url, ({ runBundler, readOutputFiles, runFileInNode }) => { ", "sentry-cli-mock.json": "["release","set-commits","CURRENT_SHA","--auto"], ["release","finalize","CURRENT_SHA"], + ["sourcemap","upload","-p","fake-project","--release","CURRENT_SHA","sentry-bundler-plugin-upload-path"], ", } `); diff --git a/packages/bundler-plugins/src/esbuild/index.ts b/packages/bundler-plugins/src/esbuild/index.ts index fc17904624be..3d487b41b750 100644 --- a/packages/bundler-plugins/src/esbuild/index.ts +++ b/packages/bundler-plugins/src/esbuild/index.ts @@ -1,4 +1,4 @@ -import type { Options } from '../core'; +import type { Options, SourceMapKind } from '../core'; import { createSentryBuildPluginManager, generateReleaseInjectorCode, @@ -9,6 +9,7 @@ import { addDebugIdToEmittedArtifacts, isJsFile, } from '../core'; +import * as fs from 'node:fs'; import * as path from 'node:path'; import { createRequire } from 'node:module'; import { randomUUID } from 'node:crypto'; @@ -48,6 +49,8 @@ interface EsbuildOnEndArgs { }; } +type EsbuildSourceMapSetting = boolean | 'inline' | 'external' | 'linked' | 'both' | undefined; + interface EsbuildInitialOptions { bundle?: boolean; inject?: string[]; @@ -55,6 +58,24 @@ interface EsbuildInitialOptions { define?: Record; write?: boolean; absWorkingDir?: string; + sourcemap?: EsbuildSourceMapSetting; +} + +/** Maps esbuild's `sourcemap` setting onto the plugin's source map kinds. */ +function sourceMapKindFromEsbuildSetting(setting: EsbuildSourceMapSetting): SourceMapKind { + switch (setting) { + case undefined: + return 'unset'; + case false: + return 'off'; + case 'external': + return 'hidden'; + case 'inline': + case 'both': + return 'inline'; + default: + return 'visible'; + } } interface EsbuildPluginBuild { @@ -280,6 +301,13 @@ export function sentryEsbuildPlugin(userOptions: Options = {}): any { }); } + const sourceMapDecision = sentryBuildPluginManager.sourceMaps.decide( + sourceMapKindFromEsbuildSetting(initialOptions.sourcemap), + ); + if (sourceMapDecision.enableHidden) { + initialOptions.sourcemap = 'external'; + } + // Create release and optionally upload const freeGlobalDependencyOnBuildArtifacts = createDependencyOnBuildArtifacts(); const upload = createDebugIdUploadFunction({ sentryBuildPluginManager }); @@ -290,27 +318,36 @@ export function sentryEsbuildPlugin(userOptions: Options = {}): any { await sentryBuildPluginManager.createRelease(); if (sourcemapsEnabled) { - const buildArtifacts = result.metafile ? Object.keys(result.metafile.outputs) : []; + // The metafile lists outputs relative to `absWorkingDir`, not to the process. + const outputDir = initialOptions.absWorkingDir ?? process.cwd(); + const buildArtifacts = result.metafile + ? Object.keys(result.metafile.outputs).map(output => path.resolve(outputDir, output)) + : []; if (options.sourcemaps?.disable !== 'disable-upload') { - await upload(buildArtifacts); + const { uploaded } = await upload(buildArtifacts); + + // esbuild writes before `onEnd` runs, so the maps are deleted from disk + // afterwards instead of being dropped from the output like in other bundlers. + if (sourceMapDecision.deleteAfterUpload && initialOptions.write !== false) { + const sourceMaps = buildArtifacts.filter(artifact => artifact.endsWith('.map')); + + if (uploaded) { + await Promise.all(sourceMaps.map(sourceMap => fs.promises.rm(sourceMap, { force: true }))); + } else if (sourceMaps.length > 0) { + logger.error('The source maps were not uploaded, so they stay in the build output.'); + } + } } else if (initialOptions.write === false) { logger.debug('Build output is not written to disk. Skipping debug ID injection into build artifacts.'); } else { // The upload routine (which stamps debug IDs into temp copies of the artifacts) is // skipped with `disable-upload`. esbuild has no hook to modify outputs before they are // written, so the emitted artifacts get stamped on disk instead. - const outputDir = initialOptions.absWorkingDir ?? process.cwd(); await Promise.all( buildArtifacts .filter(isJsFile) - .map(bundle => - addDebugIdToEmittedArtifacts( - path.resolve(outputDir, bundle), - logger, - options.sourcemaps?.resolveSourceMap, - ), - ), + .map(bundle => addDebugIdToEmittedArtifacts(bundle, logger, options.sourcemaps?.resolveSourceMap)), ); } } diff --git a/packages/bundler-plugins/test/esbuild/delete-sourcemaps-after-upload.test.ts b/packages/bundler-plugins/test/esbuild/delete-sourcemaps-after-upload.test.ts new file mode 100644 index 000000000000..d1b55b59fdc4 --- /dev/null +++ b/packages/bundler-plugins/test/esbuild/delete-sourcemaps-after-upload.test.ts @@ -0,0 +1,167 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import * as esbuild from 'esbuild'; +import type { BuildOptions } from 'esbuild'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { sentryEsbuildPlugin } from '../../src/esbuild'; +import type { Options } from '../../src/core'; + +const { mockSourcemapUpload } = vi.hoisted(() => ({ mockSourcemapUpload: vi.fn() })); + +vi.mock('sentry', () => ({ + createSentrySDK: () => ({ + release: { create: vi.fn(), finalize: vi.fn(), 'set-commits': vi.fn(), deploy: vi.fn() }, + sourcemap: { upload: mockSourcemapUpload, inject: vi.fn() }, + run: vi.fn(), + }), +})); + +const DEBUG_ID_MARKER = /sentry-dbid-([0-9a-f-]{36})/; + +const credentials: Options = { + authToken: 't', + org: 'o', + project: 'p', + release: { name: 'test-release', create: false, finalize: false }, + telemetry: false, + silent: true, +}; + +type Upload = { directory: string; files: Record }; + +describe('sourcemaps.deleteSourcemapsAfterUpload (esbuild)', () => { + let tmpDir: string; + let entry: string; + let outDir: string; + let uploads: Upload[]; + + beforeEach(() => { + vi.clearAllMocks(); + uploads = []; + mockSourcemapUpload.mockImplementation(async ({ directory }: { directory: string }) => { + const files: Record = {}; + for (const file of fs.readdirSync(directory)) { + files[file] = fs.readFileSync(path.join(directory, file), 'utf8'); + } + uploads.push({ directory, files }); + }); + + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sentry-esbuild-delete-maps-')); + entry = path.join(tmpDir, 'entry.js'); + outDir = path.join(tmpDir, 'dist'); + fs.writeFileSync(entry, 'export const answer = 42;\nconsole.log(answer);\n'); + }); + + afterEach(() => { + vi.restoreAllMocks(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + function build(pluginOptions: Options = {}, overrides: BuildOptions = {}) { + return esbuild.build({ + entryPoints: [entry], + bundle: true, + outdir: outDir, + absWorkingDir: tmpDir, + plugins: [sentryEsbuildPlugin({ ...credentials, ...pluginOptions })], + ...overrides, + }); + } + + function expectUploadedPair(upload: Upload, bundle: string): void { + const debugId = bundle.match(DEBUG_ID_MARKER)?.[1]; + expect(debugId).toBeDefined(); + expect(Object.keys(upload.files).sort()).toEqual([`${debugId}-0.js`, `${debugId}-0.js.map`]); + expect(JSON.parse(upload.files[`${debugId}-0.js.map`]!)).toMatchObject({ debug_id: debugId }); + expect(fs.existsSync(upload.directory)).toBe(false); + } + + it('enables external source maps, uploads them and deletes them when the setting is unset', async () => { + await build(); + + expect(fs.readdirSync(outDir)).toEqual(['entry.js']); + const bundle = fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8'); + expect(bundle).not.toContain('sourceMappingURL'); + + expect(uploads).toHaveLength(1); + expectUploadedPair(uploads[0]!, bundle); + }); + + it('switches linked source maps to external and deletes them when the option is true', async () => { + await build({ sourcemaps: { deleteSourcemapsAfterUpload: true } }, { sourcemap: true }); + + expect(fs.readdirSync(outDir)).toEqual(['entry.js']); + expect(fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8')).not.toContain('sourceMappingURL'); + expect(uploads).toHaveLength(1); + }); + + it('keeps external source maps the user enabled', async () => { + await build({}, { sourcemap: 'external' }); + + expect(fs.readdirSync(outDir).sort()).toEqual(['entry.js', 'entry.js.map']); + expect(uploads).toHaveLength(1); + expectUploadedPair(uploads[0]!, fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8')); + }); + + it('keeps linked source maps when the option is unset', async () => { + await build({}, { sourcemap: true }); + + expect(fs.readdirSync(outDir).sort()).toEqual(['entry.js', 'entry.js.map']); + expect(fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8')).toContain('//# sourceMappingURL=entry.js.map'); + }); + + it('does not enable source maps when the option is false', async () => { + await build({ sourcemaps: { deleteSourcemapsAfterUpload: false } }); + + expect(fs.readdirSync(outDir)).toEqual(['entry.js']); + expect(uploads).toHaveLength(0); + }); + + it('warns when source maps are turned off', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + await build({ silent: false }, { sourcemap: false }); + + expect(fs.readdirSync(outDir)).toEqual(['entry.js']); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Source map generation is turned off')); + }); + + it('warns when source maps are inlined and keeps them', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + await build({ silent: false, sourcemaps: { deleteSourcemapsAfterUpload: true } }, { sourcemap: 'inline' }); + + expect(fs.readFileSync(path.join(outDir, 'entry.js'), 'utf8')).toContain('sourceMappingURL=data:'); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Source maps are inlined')); + }); + + it('does not enable source maps when no upload can run', async () => { + await build({ authToken: undefined }); + + expect(fs.readdirSync(outDir)).toEqual(['entry.js']); + expect(uploads).toHaveLength(0); + }); + + it('does not delete anything when the build does not write', async () => { + const rmSpy = vi.spyOn(fs.promises, 'rm'); + const result = await build({}, { write: false }); + + expect(result.outputFiles?.map(file => path.basename(file.path)).sort()).toEqual(['entry.js', 'entry.js.map']); + expect(rmSpy).not.toHaveBeenCalledWith(expect.stringContaining('entry.js.map'), expect.anything()); + }); + + it('keeps the source maps when the upload fails', async () => { + mockSourcemapUpload.mockRejectedValue(new Error('upload failed')); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + await build({ silent: false }); + + expect(fs.readdirSync(outDir).sort()).toEqual(['entry.js', 'entry.js.map']); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('were not uploaded')); + }); + + it('leaves "disable-upload" builds untouched', async () => { + await build({ sourcemaps: { disable: 'disable-upload' } }, { sourcemap: true }); + + expect(fs.readdirSync(outDir).sort()).toEqual(['entry.js', 'entry.js.map']); + expect(uploads).toHaveLength(0); + }); +});