From 81b1621e0b77cd7044fad6803cd5d6826751bf73 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 8 Jan 2025 14:37:12 +0100 Subject: [PATCH 1/2] feat(astro): Respect user-specified source map setting --- docs/migration/v8-to-v9.md | 10 ++ packages/astro/src/integration/index.ts | 100 +++++++++++++++-- packages/astro/src/integration/types.ts | 10 ++ packages/astro/test/integration/index.test.ts | 101 +++++++++++++++++- 4 files changed, 209 insertions(+), 12 deletions(-) diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md index 56bf9c8bf9c4..3db2969f4c71 100644 --- a/docs/migration/v8-to-v9.md +++ b/docs/migration/v8-to-v9.md @@ -90,6 +90,16 @@ In v9, an `undefined` value will be treated the same as if the value is not defi - The `captureUserFeedback` method has been removed. Use `captureFeedback` instead and update the `comments` field to `message`. +### Meta-Framework SDKs (`@sentry/astro`, `@sentry/nuxt`) + +- Updated source map generation to respect the user-provided value of your build config, such as `vite.build.sourcemap`: + + - Explicitly disabled (false): Emit warning, no source map upload. + - Explicitly enabled (true, 'hidden', 'inline'): No changes, source maps are uploaded and not automatically deleted. + - Unset: Enable 'hidden', delete `.map` files after uploading them to Sentry. + + To customize which files are deleted after upload, define the `filesToDeleteAfterUpload` array with globs. + ### Uncategorized (TODO) TODO diff --git a/packages/astro/src/integration/index.ts b/packages/astro/src/integration/index.ts index 49e33ff0231d..de43400f7507 100644 --- a/packages/astro/src/integration/index.ts +++ b/packages/astro/src/integration/index.ts @@ -3,7 +3,7 @@ import * as path from 'path'; import { sentryVitePlugin } from '@sentry/vite-plugin'; import type { AstroConfig, AstroIntegration } from 'astro'; -import { dropUndefinedKeys } from '@sentry/core'; +import { consoleSandbox, dropUndefinedKeys } from '@sentry/core'; import { buildClientSnippet, buildSdkInitFileImportSnippet, buildServerSnippet } from './snippets'; import type { SentryOptions } from './types'; @@ -35,19 +35,30 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { // We don't need to check for AUTH_TOKEN here, because the plugin will pick it up from the env if (shouldUploadSourcemaps && command !== 'dev') { - // TODO(v9): Remove this warning - if (config?.vite?.build?.sourcemap === false) { - logger.warn( - "You disabled sourcemaps with the `vite.build.sourcemap` option. Currently, the Sentry SDK will override this option to generate sourcemaps. In future versions, the Sentry SDK will not override the `vite.build.sourcemap` option if you explicitly disable it. If you want to generate and upload sourcemaps please set the `vite.build.sourcemap` option to 'hidden' or undefined.", - ); + const computedSourceMapSettings = getUpdatedSourceMapSettings(config, options); + + let updatedFilesToDeleteAfterUpload: string[] | undefined = undefined; + + if ( + typeof uploadOptions?.filesToDeleteAfterUpload === 'undefined' && + computedSourceMapSettings.previousUserSourceMapSetting === 'unset' + ) { + updatedFilesToDeleteAfterUpload = ['./dist/**/client/**/*.map', './dist/**/server/**/*.map']; + + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + `[Sentry] Setting \`sourceMapsUploadOptions.filesToDeleteAfterUpload: ${JSON.stringify( + updatedFilesToDeleteAfterUpload, + )}\` to delete generated source maps after they were uploaded to Sentry.`, + ); + }); } - // TODO: Add deleteSourcemapsAfterUpload option and warn if it isn't set. - updateConfig({ vite: { build: { - sourcemap: true, + sourcemap: computedSourceMapSettings.updatedSourceMapSetting, }, plugins: [ sentryVitePlugin( @@ -58,6 +69,9 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { telemetry: uploadOptions.telemetry ?? true, sourcemaps: { assets: uploadOptions.assets ?? [getSourcemapsAssetsGlob(config)], + filesToDeleteAfterUpload: uploadOptions?.filesToDeleteAfterUpload + ? uploadOptions?.filesToDeleteAfterUpload + : updatedFilesToDeleteAfterUpload, }, bundleSizeOptimizations: { ...options.bundleSizeOptimizations, @@ -171,3 +185,71 @@ function getSourcemapsAssetsGlob(config: AstroConfig): string { // fallback to default output dir return 'dist/**/*'; } + +/** + * Whether the user enabled (true, 'hidden', 'inline') or disabled (false) source maps + */ +export type UserSourceMapSetting = 'enabled' | 'disabled' | 'unset' | undefined; + +/** There are 3 ways to set up source map generation (https://github.com/getsentry/sentry-javascript/issues/13993) + * + * 1. User explicitly disabled source maps + * - keep this setting (emit a warning that errors won't be unminified in Sentry) + * - We won't upload anything + * + * 2. Users enabled source map generation (true, 'hidden', 'inline'). + * - keep this setting (don't do anything - like deletion - besides uploading) + * + * 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) + * + * --> only exported for testing + */ +export function getUpdatedSourceMapSettings( + astroConfig: AstroConfig, + sentryOptions?: SentryOptions, +): { previousUserSourceMapSetting: UserSourceMapSetting; updatedSourceMapSetting: boolean | 'inline' | 'hidden' } { + let previousUserSourceMapSetting: UserSourceMapSetting = undefined; + + astroConfig.build = astroConfig.build || {}; + + const viteSourceMap = astroConfig?.vite?.build?.sourcemap; + let updatedSourceMapSetting = viteSourceMap; + + const settingKey = 'vite.build.sourcemap'; + + if (viteSourceMap === false) { + previousUserSourceMapSetting = 'disabled'; + updatedSourceMapSetting = viteSourceMap; + + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + `[Sentry] Source map generation are currently disabled in your Astro configuration (\`${settingKey}: false\`). This setting is either a default setting or was explicitly set in your configuration. Sentry won't override this setting. Without source maps, code snippets on the Sentry Issues page will remain minified. To show unminified code, enable source maps in \`${settingKey}\` (e.g. by setting them to \`hidden\`).`, + ); + }); + } else if (viteSourceMap && ['hidden', 'inline', true].includes(viteSourceMap)) { + previousUserSourceMapSetting = 'enabled'; + updatedSourceMapSetting = viteSourceMap; + + if (sentryOptions?.debug) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log( + `[Sentry] We discovered \`${settingKey}\` is set to \`${viteSourceMap.toString()}\`. Sentry will keep this source map setting. This will un-minify the code snippet on the Sentry Issue page.`, + ); + }); + } + } else { + previousUserSourceMapSetting = 'unset'; + updatedSourceMapSetting = 'hidden'; + + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.log(`[Sentry] Enabled source map generation in the build options with \`${settingKey}: 'hidden'\`.`); + }); + } + + return { previousUserSourceMapSetting, updatedSourceMapSetting }; +} diff --git a/packages/astro/src/integration/types.ts b/packages/astro/src/integration/types.ts index b32b62556140..6c2e41808eca 100644 --- a/packages/astro/src/integration/types.ts +++ b/packages/astro/src/integration/types.ts @@ -73,6 +73,16 @@ type SourceMapsOptions = { * @see https://www.npmjs.com/package/glob#glob-primer */ assets?: string | Array; + + /** + * A glob or an array of globs that specifies the build artifacts that should be deleted after the artifact + * upload to Sentry has been completed. + * + * @default [] - By default no files are deleted. + * + * The globbing patterns follow the implementation of the glob package. (https://www.npmjs.com/package/glob) + */ + filesToDeleteAfterUpload?: string | Array; }; type BundleSizeOptimizationOptions = { diff --git a/packages/astro/test/integration/index.test.ts b/packages/astro/test/integration/index.test.ts index eb6bdf555ae3..d65cea37b261 100644 --- a/packages/astro/test/integration/index.test.ts +++ b/packages/astro/test/integration/index.test.ts @@ -1,4 +1,7 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { AstroConfig } from 'astro'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { getUpdatedSourceMapSettings } from '../../src/integration/index'; +import type { SentryOptions } from '../../src/integration/types'; import { sentryAstro } from '../../src/integration'; @@ -31,7 +34,7 @@ describe('sentryAstro integration', () => { expect(integration.name).toBe('@sentry/astro'); }); - it('enables source maps and adds the sentry vite plugin if an auth token is detected', async () => { + it('enables "hidden" source maps, adds filesToDeleteAfterUpload and adds the sentry vite plugin if an auth token is detected', async () => { const integration = sentryAstro({ sourceMapsUploadOptions: { enabled: true, org: 'my-org', project: 'my-project', telemetry: false }, }); @@ -44,7 +47,7 @@ describe('sentryAstro integration', () => { expect(updateConfig).toHaveBeenCalledWith({ vite: { build: { - sourcemap: true, + sourcemap: 'hidden', }, plugins: ['sentryVitePlugin'], }, @@ -60,6 +63,7 @@ describe('sentryAstro integration', () => { bundleSizeOptimizations: {}, sourcemaps: { assets: ['out/**/*'], + filesToDeleteAfterUpload: ['./dist/**/client/**/*.map', './dist/**/server/**/*.map'], }, _metaOptions: { telemetry: { @@ -86,6 +90,7 @@ describe('sentryAstro integration', () => { bundleSizeOptimizations: {}, sourcemaps: { assets: ['dist/**/*'], + filesToDeleteAfterUpload: ['./dist/**/client/**/*.map', './dist/**/server/**/*.map'], }, _metaOptions: { telemetry: { @@ -119,6 +124,7 @@ describe('sentryAstro integration', () => { bundleSizeOptimizations: {}, sourcemaps: { assets: ['{.vercel,dist}/**/*'], + filesToDeleteAfterUpload: ['./dist/**/client/**/*.map', './dist/**/server/**/*.map'], }, _metaOptions: { telemetry: { @@ -157,6 +163,7 @@ describe('sentryAstro integration', () => { bundleSizeOptimizations: {}, sourcemaps: { assets: ['dist/server/**/*, dist/client/**/*'], + filesToDeleteAfterUpload: ['./dist/**/client/**/*.map', './dist/**/server/**/*.map'], }, _metaOptions: { telemetry: { @@ -166,6 +173,35 @@ describe('sentryAstro integration', () => { }); }); + it('prefers user-specified filesToDeleteAfterUpload over the default values', async () => { + const integration = sentryAstro({ + sourceMapsUploadOptions: { + enabled: true, + org: 'my-org', + project: 'my-project', + filesToDeleteAfterUpload: ['./custom/path/**/*'], + }, + }); + // @ts-expect-error - the hook exists, and we only need to pass what we actually use + await integration.hooks['astro:config:setup']({ + updateConfig, + injectScript, + // @ts-expect-error - only passing in partial config + config: { + outDir: new URL('file://path/to/project/build'), + }, + }); + + expect(sentryVitePluginSpy).toHaveBeenCalledTimes(1); + expect(sentryVitePluginSpy).toHaveBeenCalledWith( + expect.objectContaining({ + sourcemaps: expect.objectContaining({ + filesToDeleteAfterUpload: ['./custom/path/**/*'], + }), + }), + ); + }); + it("doesn't enable source maps if `sourceMapsUploadOptions.enabled` is `false`", async () => { const integration = sentryAstro({ sourceMapsUploadOptions: { enabled: false }, @@ -373,3 +409,62 @@ describe('sentryAstro integration', () => { expect(addMiddleware).toHaveBeenCalledTimes(0); }); }); + +describe('getUpdatedSourceMapSettings', () => { + let astroConfig: Omit & { vite: { build: { sourcemap?: any } } }; + let sentryOptions: SentryOptions; + + beforeEach(() => { + astroConfig = { vite: { build: {} } } as Omit & { vite: { build: { sourcemap?: any } } }; + sentryOptions = {}; + }); + + it('should keep explicitly disabled source maps disabled', () => { + astroConfig.vite.build.sourcemap = false; + const result = getUpdatedSourceMapSettings(astroConfig, sentryOptions); + expect(result.previousUserSourceMapSetting).toBe('disabled'); + expect(result.updatedSourceMapSetting).toBe(false); + }); + + it('should keep explicitly enabled source maps enabled', () => { + const cases = [ + { sourcemap: true, expected: true }, + { sourcemap: 'hidden', expected: 'hidden' }, + { sourcemap: 'inline', expected: 'inline' }, + ]; + + cases.forEach(({ sourcemap, expected }) => { + astroConfig.vite.build.sourcemap = sourcemap; + const result = getUpdatedSourceMapSettings(astroConfig, sentryOptions); + expect(result.previousUserSourceMapSetting).toBe('enabled'); + expect(result.updatedSourceMapSetting).toBe(expected); + }); + }); + + it('should enable "hidden" source maps when unset', () => { + astroConfig.vite.build.sourcemap = undefined; + const result = getUpdatedSourceMapSettings(astroConfig, sentryOptions); + expect(result.previousUserSourceMapSetting).toBe('unset'); + expect(result.updatedSourceMapSetting).toBe('hidden'); + }); + + it('should log warnings and messages when debug is enabled', () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + sentryOptions = { debug: true }; + + astroConfig.vite.build.sourcemap = false; + getUpdatedSourceMapSettings(astroConfig, sentryOptions); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Source map generation are currently disabled'), + ); + + astroConfig.vite.build.sourcemap = 'hidden'; + getUpdatedSourceMapSettings(astroConfig, sentryOptions); + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining('Sentry will keep this source map setting')); + + consoleWarnSpy.mockRestore(); + consoleLogSpy.mockRestore(); + }); +}); From f77e0216dd05cd3dff150bb45c4562cb87dbc45b Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 9 Jan 2025 12:47:31 +0100 Subject: [PATCH 2/2] review suggestions --- packages/astro/src/integration/index.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/astro/src/integration/index.ts b/packages/astro/src/integration/index.ts index de43400f7507..5efeefa62153 100644 --- a/packages/astro/src/integration/index.ts +++ b/packages/astro/src/integration/index.ts @@ -43,6 +43,7 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { typeof uploadOptions?.filesToDeleteAfterUpload === 'undefined' && computedSourceMapSettings.previousUserSourceMapSetting === 'unset' ) { + // This also works for adapters, as the source maps are also copied to e.g. the .vercel folder updatedFilesToDeleteAfterUpload = ['./dist/**/client/**/*.map', './dist/**/server/**/*.map']; consoleSandbox(() => { @@ -69,9 +70,8 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { telemetry: uploadOptions.telemetry ?? true, sourcemaps: { assets: uploadOptions.assets ?? [getSourcemapsAssetsGlob(config)], - filesToDeleteAfterUpload: uploadOptions?.filesToDeleteAfterUpload - ? uploadOptions?.filesToDeleteAfterUpload - : updatedFilesToDeleteAfterUpload, + filesToDeleteAfterUpload: + uploadOptions?.filesToDeleteAfterUpload ?? updatedFilesToDeleteAfterUpload, }, bundleSizeOptimizations: { ...options.bundleSizeOptimizations, @@ -247,7 +247,9 @@ export function getUpdatedSourceMapSettings( consoleSandbox(() => { // eslint-disable-next-line no-console - console.log(`[Sentry] Enabled source map generation in the build options with \`${settingKey}: 'hidden'\`.`); + console.log( + `[Sentry] Enabled source map generation in the build options with \`${settingKey}: 'hidden'\`. The source maps will be deleted after they were uploaded to Sentry.`, + ); }); }