Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/migration/v8-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
102 changes: 93 additions & 9 deletions packages/astro/src/integration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -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 };
}
10 changes: 10 additions & 0 deletions packages/astro/src/integration/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
101 changes: 98 additions & 3 deletions packages/astro/test/integration/index.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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 },
});
Expand All @@ -44,7 +47,7 @@ describe('sentryAstro integration', () => {
expect(updateConfig).toHaveBeenCalledWith({
vite: {
build: {
sourcemap: true,
sourcemap: 'hidden',
},
plugins: ['sentryVitePlugin'],
},
Expand All @@ -60,6 +63,7 @@ describe('sentryAstro integration', () => {
bundleSizeOptimizations: {},
sourcemaps: {
assets: ['out/**/*'],
filesToDeleteAfterUpload: ['./dist/**/client/**/*.map', './dist/**/server/**/*.map'],
},
_metaOptions: {
telemetry: {
Expand All @@ -86,6 +90,7 @@ describe('sentryAstro integration', () => {
bundleSizeOptimizations: {},
sourcemaps: {
assets: ['dist/**/*'],
filesToDeleteAfterUpload: ['./dist/**/client/**/*.map', './dist/**/server/**/*.map'],
},
_metaOptions: {
telemetry: {
Expand Down Expand Up @@ -119,6 +124,7 @@ describe('sentryAstro integration', () => {
bundleSizeOptimizations: {},
sourcemaps: {
assets: ['{.vercel,dist}/**/*'],
filesToDeleteAfterUpload: ['./dist/**/client/**/*.map', './dist/**/server/**/*.map'],
},
_metaOptions: {
telemetry: {
Expand Down Expand Up @@ -157,6 +163,7 @@ describe('sentryAstro integration', () => {
bundleSizeOptimizations: {},
sourcemaps: {
assets: ['dist/server/**/*, dist/client/**/*'],
filesToDeleteAfterUpload: ['./dist/**/client/**/*.map', './dist/**/server/**/*.map'],
},
_metaOptions: {
telemetry: {
Expand All @@ -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 },
Expand Down Expand Up @@ -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();
});
});
Loading