feat(astro): Respect user-specified source map setting (#14941)

Closes #14934

Author: s1gr1d

docs/migration/v8-to-v9.md

@@ -98,6 +98,16 @@ In v9, an `undefined` value will be treated the same as if the value is not defi
 
   This behavior was changed because the Next.js Build ID is non-deterministic and the release name is injected into client bundles, causing build artifacts to be non-deterministic. This caused issues for some users. Additionally, because it is uncertain whether it will be possible to rely on a Build ID when Turbopack becomes stable, we decided to pull the plug now instead of introducing confusing behavior in the future.
 
+### All 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

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,31 @@ 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'
+          ) {
+            // 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'];
 
-          // TODO: Add deleteSourcemapsAfterUpload option and warn if it isn't set.
+            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.`,
+              );
+            });
+          }
 
           updateConfig({
             vite: {
               build: {
-                sourcemap: true,
+                sourcemap: computedSourceMapSettings.updatedSourceMapSetting,
               },
               plugins: [
                 sentryVitePlugin(
@@ -58,6 +70,8 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
                     telemetry: uploadOptions.telemetry ?? true,
                     sourcemaps: {
                       assets: uploadOptions.assets ?? [getSourcemapsAssetsGlob(config)],
+                      filesToDeleteAfterUpload:
+                        uploadOptions?.filesToDeleteAfterUpload ?? updatedFilesToDeleteAfterUpload,
                     },
                     bundleSizeOptimizations: {
                       ...options.bundleSizeOptimizations,
@@ -171,3 +185,73 @@ 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'\`. The source maps will be deleted after they were uploaded to Sentry.`,
+      );
+    });
+  }
+
+  return { previousUserSourceMapSetting, updatedSourceMapSetting };
+}

packages/astro/src/integration/types.ts

@@ -73,6 +73,16 @@ type SourceMapsOptions = {
    * @see https://www.npmjs.com/package/glob#glob-primer
    */
   assets?: string | Array<string>;
+
+  /**
+   * 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<string>;
 };
 
 type BundleSizeOptimizationOptions = {

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<AstroConfig, 'vite'> & { vite: { build: { sourcemap?: any } } };
+  let sentryOptions: SentryOptions;
+
+  beforeEach(() => {
+    astroConfig = { vite: { build: {} } } as Omit<AstroConfig, 'vite'> & { 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();
+  });
+});