From 03a9d1ae4209634a51609471baa838e7a3fa4edb Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 12 Nov 2026 11:22:05 +0000 Subject: [PATCH] feat(astro): Delete source maps through the bundler plugin instead of a glob --- .../astro-7-sourcemaps/assert-build.ts | 60 ++++-- .../astro-7-sourcemaps/astro.config.mjs | 22 ++- .../astro-7-sourcemaps/package.json | 9 +- packages/astro/src/integration/index.ts | 58 +++--- packages/astro/test/integration/index.test.ts | 172 +++++++++++++++--- 5 files changed, 242 insertions(+), 79 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/assert-build.ts b/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/assert-build.ts index ad96fabd3f14..75d7a8a207d0 100644 --- a/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/assert-build.ts +++ b/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/assert-build.ts @@ -1,23 +1,53 @@ +import * as assert from 'node:assert/strict'; +import * as fs from 'node:fs'; import * as path from 'node:path'; -import { assertSourceMapUpload } from '@sentry-internal/test-utils'; +import { + assertSourceMapUpload, + findInjectedDebugIds, + findSourceMapFiles, + findSourceMappingUrlComments, +} from '@sentry-internal/test-utils'; const isStatic = process.env.E2E_ASTRO_OUTPUT === 'static'; +const noUpload = !!process.env.E2E_NO_UPLOAD; -const BUILD_OUTPUT = 'dist'; +const BUILD_OUTPUT = noUpload ? 'build' : 'dist'; const OUTPUT_DIRS = isStatic ? [BUILD_OUTPUT] : [path.join(BUILD_OUTPUT, 'client'), path.join(BUILD_OUTPUT, 'server')]; -assertSourceMapUpload({ - outputDirs: OUTPUT_DIRS, - // The app does not set `build.sourcemap`, so the SDK enables hidden maps and must delete them after upload. - strippedDirs: [BUILD_OUTPUT], - markers: { - 'client source': 'SOURCEMAP_MARKER_CLIENT', - ...(isStatic ? {} : { 'server source': 'SOURCEMAP_MARKER_SERVER' }), - // Astro 7 writes the prerender build to a hidden `.prerender` folder, which a plain `dist/**/*` glob skips - 'prerendered page source': 'SOURCEMAP_MARKER_PRERENDER', - }, - // Written by the build, see `recordChunksWithoutSourceMap` in `astro.config.mjs`. - debugIdsWithoutSourceMap: '.tmp_debug_ids_without_source_map', -}); +// The SDK enables hidden source maps when the app sets none, and asks the bundler plugin to remove +// them from the client, server and prerender output, also when no upload runs. +if (noUpload) { + // Written by the build, see `recordSourceMaps` in `astro.config.mjs`. + const mapsMade = new Map(); + for (const line of fs.readFileSync('.tmp_source_maps_made', 'utf8').split('\n').filter(Boolean)) { + const [environment, count] = line.split(' '); + mapsMade.set(environment as string, (mapsMade.get(environment as string) ?? 0) + Number(count)); + } + for (const environment of ['client', 'ssr', 'prerender']) { + assert.ok(mapsMade.get(environment), `Expected the ${environment} build to make source maps`); + } + assert.notDeepEqual(findInjectedDebugIds({ outputDir: BUILD_OUTPUT }), [], `Expected debug IDs in ${BUILD_OUTPUT}`); + + assert.deepEqual(findSourceMapFiles({ outputDir: BUILD_OUTPUT }), [], `Expected no source maps in ${BUILD_OUTPUT}`); + assert.deepEqual( + findSourceMappingUrlComments({ outputDir: BUILD_OUTPUT }), + [], + `Expected no sourceMappingURL comments in ${BUILD_OUTPUT}`, + ); +} else { + assertSourceMapUpload({ + outputDirs: OUTPUT_DIRS, + // The app does not set `build.sourcemap`, so the SDK enables hidden maps and must delete them after upload. + strippedDirs: [BUILD_OUTPUT], + markers: { + 'client source': 'SOURCEMAP_MARKER_CLIENT', + ...(isStatic ? {} : { 'server source': 'SOURCEMAP_MARKER_SERVER' }), + // Astro 7 writes the prerender build to a hidden `.prerender` folder, which a plain `dist/**/*` glob skips + 'prerendered page source': 'SOURCEMAP_MARKER_PRERENDER', + }, + // Written by the build, see `recordChunksWithoutSourceMap` in `astro.config.mjs`. + debugIdsWithoutSourceMap: '.tmp_debug_ids_without_source_map', + }); +} console.log('All sourcemap assertions passed!'); diff --git a/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/astro.config.mjs b/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/astro.config.mjs index 53887471a483..c752bde6074d 100644 --- a/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/astro.config.mjs +++ b/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/astro.config.mjs @@ -4,15 +4,19 @@ import { defineConfig } from 'astro/config'; import * as fs from 'node:fs'; const isStatic = process.env.E2E_ASTRO_OUTPUT === 'static'; +const noUpload = !!process.env.E2E_NO_UPLOAD; -// Rolldown emits no map for chunks without mappable source (e.g. virtual modules or its runtime), so their debug -// IDs can't be uploaded. Recorded here because the SDK deletes all maps after the upload. -function recordChunksWithoutSourceMap() { +// Records the source maps each environment made, before the Sentry plugin removes them in its +// `generateBundle` with `order: 'post'`. Rolldown emits no map for chunks without mappable source +// (e.g. virtual modules or its runtime), so their debug IDs can't be uploaded. +function recordSourceMaps() { return { - name: 'record-chunks-without-source-map', + name: 'record-source-maps', apply: 'build', - enforce: 'post', - writeBundle(_options, bundle) { + generateBundle(_options, bundle) { + const maps = Object.keys(bundle).filter(fileName => fileName.endsWith('.map')); + fs.appendFileSync('.tmp_source_maps_made', `${this.environment.name} ${maps.length}\n`); + for (const output of Object.values(bundle)) { if (output.type !== 'chunk' || bundle[`${output.fileName}.map`]) { continue; @@ -39,7 +43,7 @@ export default defineConfig({ integrations: [ sentry({ sentryUrl: 'http://localhost:3032', - authToken: 'fake-auth-token', + authToken: noUpload ? undefined : 'fake-auth-token', org: 'test-org', project: 'test-project', release: { name: 'test-release' }, @@ -48,7 +52,9 @@ export default defineConfig({ ...(isStatic ? [] : [ssrRoute]), ], // Keeps Astro from inlining the small client script into the HTML, where it would get no source map - vite: { build: { assetsInlineLimit: 0 }, plugins: [recordChunksWithoutSourceMap()] }, + vite: { build: { assetsInlineLimit: 0 }, plugins: [recordSourceMaps()] }, + // The removal must follow the configured output directory + ...(noUpload && { outDir: './build' }), output: isStatic ? 'static' : 'server', adapter: isStatic ? undefined : node({ mode: 'standalone' }), }); diff --git a/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/package.json b/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/package.json index 586b73f4e4d1..4afe91d335cf 100644 --- a/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/package.json +++ b/dev-packages/e2e-tests/test-applications/astro-7-sourcemaps/package.json @@ -9,7 +9,9 @@ "test:build": "pnpm install && pnpm build", "test:assert": "node assert-build.ts", "test:build:static": "E2E_ASTRO_OUTPUT=static pnpm test:build", - "test:assert:static": "E2E_ASTRO_OUTPUT=static pnpm test:assert" + "test:assert:static": "E2E_ASTRO_OUTPUT=static pnpm test:assert", + "test:build:no-upload": "E2E_NO_UPLOAD=1 pnpm test:build", + "test:assert:no-upload": "E2E_NO_UPLOAD=1 pnpm test:assert" }, "dependencies": { "@astrojs/node": "^11.1.4", @@ -26,6 +28,11 @@ "build-command": "pnpm test:build:static", "assert-command": "pnpm test:assert:static", "label": "astro-7-sourcemaps (static output)" + }, + { + "build-command": "pnpm test:build:no-upload", + "assert-command": "pnpm test:assert:no-upload", + "label": "astro-7-sourcemaps (no upload)" } ] } diff --git a/packages/astro/src/integration/index.ts b/packages/astro/src/integration/index.ts index 0df042e17afd..7b676285b32e 100644 --- a/packages/astro/src/integration/index.ts +++ b/packages/astro/src/integration/index.ts @@ -67,29 +67,15 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { if (shouldUploadSourcemaps && command !== 'dev') { const computedSourceMapSettings = _getUpdatedSourceMapSettings(config, options, logger); - let updatedFilesToDeleteAfterUpload: string[] | undefined = undefined; - - if ( - typeof sourcemaps?.filesToDeleteAfterUpload === 'undefined' && - computedSourceMapSettings.previousUserSourceMapSetting === 'unset' - ) { - // This also works for adapters, as the source maps are also copied to e.g. the .vercel folder. - // Static output has no client folder and writes client assets to `/` instead. - // Astro removes the hidden `.prerender` folder itself, but we don't want to rely on that. - updatedFilesToDeleteAfterUpload = [ - './dist/**/client/**/*.map', - './dist/**/server/**/*.map', - `./dist/**/${config.build?.assets ?? '_astro'}/**/*.map`, - './dist/**/.prerender/**/*.map', - ]; - - debug && - logger.info( - `Automatically setting \`sourcemaps.filesToDeleteAfterUpload: ${JSON.stringify( - updatedFilesToDeleteAfterUpload, - )}\` to delete generated source maps after they were uploaded to Sentry.`, - ); - } + const sdkEnabledSourceMaps = computedSourceMapSettings.previousUserSourceMapSetting === 'unset'; + const buildState: ViteBuildState = { writesCloudflareWorker: false }; + // A user's own glob replaces the SDK's deletion. Maps in a Cloudflare `_worker.js` folder + // stay, so that wrangler can read them for `upload_source_maps`. + const deleteSourcemapsAfterUpload = + sourcemaps?.deleteSourcemapsAfterUpload ?? + (sdkEnabledSourceMaps && sourcemaps?.filesToDeleteAfterUpload === undefined + ? () => (buildState.writesCloudflareWorker ? undefined : true) + : undefined); updateConfig({ vite: { @@ -97,6 +83,7 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { sourcemap: computedSourceMapSettings.updatedSourceMapSetting, }, plugins: [ + makeViteBuildStatePlugin(buildState), sentryVitePlugin({ applicationKey, moduleMetadata, @@ -119,7 +106,8 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { sourcemaps: { ...sourcemaps, assets: sourcemaps?.assets ?? getSourcemapsAssetsGlobs(config), - filesToDeleteAfterUpload: sourcemaps?.filesToDeleteAfterUpload ?? updatedFilesToDeleteAfterUpload, + deleteSourcemapsAfterUpload, + sdkEnabledSourceMaps, }, bundleSizeOptimizations: { ...bundleSizeOptimizations, @@ -274,6 +262,26 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { }; }; +type ViteBuildState = { + /** The current Vite build writes the Worker of an older Cloudflare adapter to `_worker.js`. */ + writesCloudflareWorker: boolean; +}; + +/** + * Records what the SDK needs to know about each Vite build. Its `config` hook runs before the one + * of the Sentry plugin, which reads `buildState` there. + */ +function makeViteBuildStatePlugin(buildState: ViteBuildState): VitePlugin { + return { + name: 'sentry-astro-vite-build-state', + apply: 'build', + enforce: 'post', + config(viteConfig) { + buildState.writesCloudflareWorker = /(?:^|[\\/])_worker\.js[\\/]*$/.test(viteConfig.build?.outDir ?? ''); + }, + }; +} + // Every sub-plugin of `sentryCloudflareVitePlugin()` but its orchestrion transform has this prefix, // and some are added whatever its options are. const CLOUDFLARE_VITE_PLUGIN_NAME_PREFIX = 'sentry-cloudflare-'; @@ -390,7 +398,7 @@ export type UserSourceMapSetting = 'enabled' | 'disabled' | 'unset' | undefined; * * 3. Users didn't set source maps generation * - we enable 'hidden' source maps generation - * - configure `filesToDeleteAfterUpload` to delete all .map files (we emit a log about this) + * - remove the maps from the build output, except in a Cloudflare `_worker.js` folder * * --> only exported for testing */ diff --git a/packages/astro/test/integration/index.test.ts b/packages/astro/test/integration/index.test.ts index 870c28dc5b30..d643f2397b80 100644 --- a/packages/astro/test/integration/index.test.ts +++ b/packages/astro/test/integration/index.test.ts @@ -4,11 +4,17 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { _getUpdatedSourceMapSettings, sentryAstro } from '../../src/integration'; import type { SentryOptions } from '../../src/integration/types'; -const sentryVitePluginSpy = vi.fn(() => 'sentryVitePlugin'); +type CapturedSourcemapsOptions = { deleteSourcemapsAfterUpload?: boolean | (() => boolean | undefined) }; + +let capturedSourcemapsOptions: CapturedSourcemapsOptions | undefined; + +const sentryVitePluginSpy = vi.fn((options: { sourcemaps?: CapturedSourcemapsOptions }) => { + capturedSourcemapsOptions = options.sourcemaps; + return 'sentryVitePlugin'; +}); vi.mock('@sentry/bundler-plugins/vite', () => ({ - // @ts-expect-error - just mocking around - sentryVitePlugin: vi.fn(args => sentryVitePluginSpy(args)), + sentryVitePlugin: vi.fn((args: { sourcemaps?: CapturedSourcemapsOptions }) => sentryVitePluginSpy(args)), })); // Stub the orchestrion plugin so these stay pure wiring tests (no apm code transformer pulled in). @@ -83,7 +89,7 @@ describe('sentryAstro integration', () => { expect(integration.name).toBe('@sentry/astro'); }); - it('enables "hidden" source maps, adds filesToDeleteAfterUpload and adds the sentry vite plugin if an auth token is detected', async () => { + it('enables "hidden" source maps, asks for their deletion and adds the sentry vite plugin if an auth token is detected', async () => { const integration = sentryAstro({ org: 'my-org', project: 'my-project', @@ -101,7 +107,7 @@ describe('sentryAstro integration', () => { build: { sourcemap: 'hidden', }, - plugins: ['sentryVitePlugin'], + plugins: [expect.objectContaining({ name: 'sentry-astro-vite-build-state' }), 'sentryVitePlugin'], }, }); expect(updateConfig).toHaveBeenCalledWith({ @@ -121,12 +127,8 @@ describe('sentryAstro integration', () => { bundleSizeOptimizations: {}, sourcemaps: { assets: ['out/**/*', 'out/**/.prerender/**/*'], - filesToDeleteAfterUpload: [ - './dist/**/client/**/*.map', - './dist/**/server/**/*.map', - './dist/**/_astro/**/*.map', - './dist/**/.prerender/**/*.map', - ], + deleteSourcemapsAfterUpload: expect.any(Function), + sdkEnabledSourceMaps: true, }, _metaOptions: { telemetry: { @@ -157,12 +159,8 @@ describe('sentryAstro integration', () => { bundleSizeOptimizations: {}, sourcemaps: { assets: ['dist/**/*', 'dist/**/.prerender/**/*'], - filesToDeleteAfterUpload: [ - './dist/**/client/**/*.map', - './dist/**/server/**/*.map', - './dist/**/_astro/**/*.map', - './dist/**/.prerender/**/*.map', - ], + deleteSourcemapsAfterUpload: expect.any(Function), + sdkEnabledSourceMaps: true, }, _metaOptions: { telemetry: { @@ -201,12 +199,8 @@ describe('sentryAstro integration', () => { bundleSizeOptimizations: {}, sourcemaps: { assets: ['{.vercel,dist}/**/*', '{.vercel,dist}/**/.prerender/**/*'], - filesToDeleteAfterUpload: [ - './dist/**/client/**/*.map', - './dist/**/server/**/*.map', - './dist/**/_astro/**/*.map', - './dist/**/.prerender/**/*.map', - ], + deleteSourcemapsAfterUpload: expect.any(Function), + sdkEnabledSourceMaps: true, }, _metaOptions: { telemetry: { @@ -245,12 +239,8 @@ describe('sentryAstro integration', () => { bundleSizeOptimizations: {}, sourcemaps: { assets: ['dist/server/**/*, dist/client/**/*'], - filesToDeleteAfterUpload: [ - './dist/**/client/**/*.map', - './dist/**/server/**/*.map', - './dist/**/_astro/**/*.map', - './dist/**/.prerender/**/*.map', - ], + deleteSourcemapsAfterUpload: expect.any(Function), + sdkEnabledSourceMaps: true, }, _metaOptions: { telemetry: { @@ -261,7 +251,7 @@ describe('sentryAstro integration', () => { ); }); - it('prefers user-specified filesToDeleteAfterUpload over the default values', async () => { + it('lets user-specified filesToDeleteAfterUpload replace the SDK deletion', async () => { const integration = sentryAstro({ org: 'my-org', project: 'my-project', @@ -283,11 +273,133 @@ describe('sentryAstro integration', () => { expect.objectContaining({ sourcemaps: expect.objectContaining({ filesToDeleteAfterUpload: ['./custom/path/**/*'], + deleteSourcemapsAfterUpload: undefined, }), }), ); }); + describe('deleteSourcemapsAfterUpload', () => { + async function runSetup(options: SentryOptions, astroConfig: Partial = {}): Promise { + const integration = sentryAstro({ org: 'my-org', project: 'my-project', ...options }); + // @ts-expect-error - the hook exists, and we only need to pass what we actually use + await integration.hooks['astro:config:setup']({ + ...baseConfigHookObject, + updateConfig, + injectScript, + config: astroConfig as AstroConfig, + }); + } + + function expectSourcemapsOptions(sourcemaps: Record): void { + expect(sentryVitePluginSpy).toHaveBeenCalledTimes(1); + expect(sentryVitePluginSpy).toHaveBeenCalledWith( + expect.objectContaining({ sourcemaps: expect.objectContaining(sourcemaps) }), + ); + } + + // The bundler plugin calls the function form in its `config` hook, after the build state + // plugin ran. + function runBuildStatePlugin(viteConfig: { build?: { outDir?: string } }): void { + const plugin = updateConfig.mock.calls + .flatMap(([astroConfigUpdate]) => astroConfigUpdate.vite?.plugins ?? []) + .find(plugin => plugin?.name === 'sentry-astro-vite-build-state'); + plugin.config(viteConfig); + } + + function expectDeleteOption(value: boolean | undefined): void { + expect(sentryVitePluginSpy).toHaveBeenCalledTimes(1); + const option = capturedSourcemapsOptions?.deleteSourcemapsAfterUpload; + expect(typeof option === 'function' ? option() : option).toBe(value); + } + + it('is `true` when the SDK enabled the source maps', async () => { + await runSetup({}); + runBuildStatePlugin({ build: { outDir: 'dist/' } }); + expectDeleteOption(true); + }); + + // Older Cloudflare adapters write the Worker there, and wrangler reads its maps. + it.each(['dist/_worker.js/', '/path/to/project/dist/_worker.js', 'C:\\project\\dist\\_worker.js\\'])( + 'is not set for a build into `%s`', + async outDir => { + await runSetup({}); + runBuildStatePlugin({ build: { outDir } }); + expectDeleteOption(undefined); + + runBuildStatePlugin({ build: { outDir: 'dist/' } }); + expectDeleteOption(true); + }, + ); + + it('lets a user-set `true` remove the maps of a Cloudflare `_worker.js` build', async () => { + await runSetup({ sourcemaps: { deleteSourcemapsAfterUpload: true } }); + runBuildStatePlugin({ build: { outDir: 'dist/_worker.js/' } }); + expectDeleteOption(true); + }); + + it.each(['hidden', true] as const)('is not set when the user set `vite.build.sourcemap: %s`', async sourcemap => { + await runSetup({}, { vite: { build: { sourcemap } } }); + expectDeleteOption(undefined); + }); + + it.each([ + [true, undefined], + [false, undefined], + [true, 'hidden'], + [false, 'hidden'], + ] as const)('passes a user-set `%s` through when `vite.build.sourcemap` is `%s`', async (value, sourcemap) => { + await runSetup({ sourcemaps: { deleteSourcemapsAfterUpload: value } }, { vite: { build: { sourcemap } } }); + expectDeleteOption(value); + }); + + it('is not set when the user set `filesToDeleteAfterUpload`', async () => { + await runSetup({ sourcemaps: { filesToDeleteAfterUpload: ['./dist/**/*.map'] } }); + expectDeleteOption(undefined); + }); + + it('lets a user-set `true` win over the user glob', async () => { + await runSetup({ + sourcemaps: { deleteSourcemapsAfterUpload: true, filesToDeleteAfterUpload: ['./dist/**/*.map'] }, + }); + expectDeleteOption(true); + }); + + // Then the plugin removes all of these maps, also those of chunks outside the user's + // `assets` or `ignore`. + it('marks the source maps as SDK-enabled when the user narrowed the upload', async () => { + await runSetup({ sourcemaps: { assets: ['./dist/client/**/*'], ignore: ['**/vendor/**'] } }); + expectSourcemapsOptions({ + assets: ['./dist/client/**/*'], + ignore: ['**/vendor/**'], + sdkEnabledSourceMaps: true, + }); + expectDeleteOption(true); + }); + + // The bundler plugin then also removes the maps of an output that sets `sourcemap` itself. + it('marks the source maps as SDK-enabled when only `rollupOptions.output.sourcemap` is set', async () => { + await runSetup({}, { vite: { build: { rollupOptions: { output: { sourcemap: true } } } } }); + expectSourcemapsOptions({ sdkEnabledSourceMaps: true }); + expectDeleteOption(true); + }); + + it.each(['hidden', true, false] as const)( + 'does not mark source maps as SDK-enabled when the user set `vite.build.sourcemap: %s`', + async sourcemap => { + await runSetup({}, { vite: { build: { sourcemap } } }); + expectSourcemapsOptions({ sdkEnabledSourceMaps: false }); + }, + ); + + // The maps are removed in this mode too, after the plugin stamps the debug IDs. + it("is `true` with `disable: 'disable-upload'`", async () => { + await runSetup({ sourcemaps: { disable: 'disable-upload' } }); + expectSourcemapsOptions({ disable: 'disable-upload', sdkEnabledSourceMaps: true }); + expectDeleteOption(true); + }); + }); + it('ignores the removed `sourceMapsUploadOptions` when computing the vite plugin options', async () => { const integration = sentryAstro({ org: 'my-org',