diff --git a/CHANGELOG.md b/CHANGELOG.md index 05b2cb9f8ad1..da0e95f0abcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +This release adds an environment check in `@sentry/nextjs` for Vercel deployments (using the `VERCEL_ENV` env variable), and only enables `SentryWebpackPlugin` if the environment is `production`. To override this, [setting `disableClientWebpackPlugin` or `disableServerWebpackPlugin` to `false`](https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#disable-sentrywebpackplugin) now takes precedence over other checks, rather than being a no-op. Note: Overriding this is not recommended! It can increase build time and clog Release Health data in Sentry with inaccurate noise. + +- fix(nextjs): Don't run webpack plugin on non-prod Vercel deployments (#5603) + ## 7.11.1 - fix(remix): Store transaction on express req (#5595) diff --git a/packages/nextjs/src/config/types.ts b/packages/nextjs/src/config/types.ts index 0305f4a5589e..3b83b23b8f31 100644 --- a/packages/nextjs/src/config/types.ts +++ b/packages/nextjs/src/config/types.ts @@ -35,6 +35,8 @@ export type NextConfigObject = { }; export type UserSentryOptions = { + // Override the SDK's default decision about whether or not to enable to the webpack plugin. Note that `false` forces + // the plugin to be enabled, even in situations where it's not recommended. disableServerWebpackPlugin?: boolean; disableClientWebpackPlugin?: boolean; hideSourceMaps?: boolean; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index dcc1b909ab0d..73b28038f01c 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -134,17 +134,7 @@ export function constructWebpackConfigFunction( newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, buildContext); // Enable the Sentry plugin (which uploads source maps to Sentry when not in dev) by default - const enableWebpackPlugin = - // TODO: this is a hack to fix https://github.com/getsentry/sentry-cli/issues/1085, which is caused by - // https://github.com/getsentry/sentry-cli/issues/915. Once the latter is addressed, this existence check can come - // out. (The check is necessary because currently, `@sentry/cli` uses a post-install script to download an - // architecture-specific version of the `sentry-cli` binary. If `yarn install`, `npm install`, or `npm ci` are run - // with the `--ignore-scripts` option, this will be blocked and the missing binary will cause an error when users - // try to build their apps.) - ensureCLIBinaryExists() && - (isServer ? !userSentryOptions.disableServerWebpackPlugin : !userSentryOptions.disableClientWebpackPlugin); - - if (enableWebpackPlugin) { + if (shouldEnableWebpackPlugin(buildContext, userSentryOptions)) { // TODO Handle possibility that user is using `SourceMapDevToolPlugin` (see // https://webpack.js.org/plugins/source-map-dev-tool-plugin/) @@ -460,3 +450,47 @@ function ensureCLIBinaryExists(): boolean { } return false; } + +/** Check various conditions to decide if we should run the plugin */ +function shouldEnableWebpackPlugin(buildContext: BuildContext, userSentryOptions: UserSentryOptions): boolean { + const { isServer, dev: isDev } = buildContext; + const { disableServerWebpackPlugin, disableClientWebpackPlugin } = userSentryOptions; + + /** Non-negotiable */ + + // TODO: this is a hack to fix https://github.com/getsentry/sentry-cli/issues/1085, which is caused by + // https://github.com/getsentry/sentry-cli/issues/915. Once the latter is addressed, this existence check can come + // out. (The check is necessary because currently, `@sentry/cli` uses a post-install script to download an + // architecture-specific version of the `sentry-cli` binary. If `yarn install`, `npm install`, or `npm ci` are run + // with the `--ignore-scripts` option, this will be blocked and the missing binary will cause an error when users + // try to build their apps.) + if (!ensureCLIBinaryExists()) { + return false; + } + + /** User override */ + + if (isServer && disableServerWebpackPlugin !== undefined) { + return !disableServerWebpackPlugin; + } else if (!isServer && disableClientWebpackPlugin !== undefined) { + return !disableClientWebpackPlugin; + } + + /** Situations where the default is to disable the plugin */ + + // TODO: Are there analogs to Vercel's preveiw and dev modes on other deployment platforms? + + if (isDev || process.env.NODE_ENV === 'development') { + // TODO (v8): Right now in dev we set the plugin to dryrun mode, and our boilerplate includes setting the plugin to + // `silent`, so for the vast majority of users, it's as if the plugin doesn't run at all in dev. Making that + // official is technically a breaking change, though, so we probably should wait until v8. + // return false + } + + if (process.env.VERCEL_ENV === 'preview' || process.env.VERCEL_ENV === 'development') { + return false; + } + + // We've passed all of the tests! + return true; +} diff --git a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts index aa4d54e3fbec..c18523850221 100644 --- a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts +++ b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts @@ -12,7 +12,7 @@ import { serverWebpackConfig, userNextConfig, } from '../fixtures'; -import { materializeFinalWebpackConfig } from '../testUtils'; +import { materializeFinalNextConfig, materializeFinalWebpackConfig } from '../testUtils'; describe('constructWebpackConfigFunction()', () => { it('includes expected properties', async () => { @@ -47,6 +47,17 @@ describe('constructWebpackConfigFunction()', () => { expect(finalWebpackConfig).toEqual(expect.objectContaining(materializedUserWebpackConfig)); }); + it("doesn't set devtool if webpack plugin is disabled", () => { + const finalNextConfig = materializeFinalNextConfig({ + ...exportedNextConfig, + webpack: () => ({ devtool: 'something-besides-source-map' } as any), + sentry: { disableServerWebpackPlugin: true }, + }); + const finalWebpackConfig = finalNextConfig.webpack?.(serverWebpackConfig, serverBuildContext); + + expect(finalWebpackConfig?.devtool).not.toEqual('source-map'); + }); + it('allows for the use of `hidden-source-map` as `devtool` value for client-side builds', async () => { const exportedNextConfigHiddenSourceMaps = { ...exportedNextConfig, diff --git a/packages/nextjs/test/config/webpack/sentryWebpackPlugin.test.ts b/packages/nextjs/test/config/webpack/sentryWebpackPlugin.test.ts index fb6de86ed538..c08736a7b2d9 100644 --- a/packages/nextjs/test/config/webpack/sentryWebpackPlugin.test.ts +++ b/packages/nextjs/test/config/webpack/sentryWebpackPlugin.test.ts @@ -2,7 +2,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { BuildContext } from '../../../src/config/types'; +import { BuildContext, ExportedNextConfig } from '../../../src/config/types'; import { getUserConfigFile, getWebpackPluginOptions, SentryWebpackPlugin } from '../../../src/config/webpack'; import { clientBuildContext, @@ -14,7 +14,7 @@ import { userSentryWebpackPluginConfig, } from '../fixtures'; import { exitsSync, mkdtempSyncSpy, mockExistsSync, realExistsSync } from '../mocks'; -import { findWebpackPlugin, materializeFinalNextConfig, materializeFinalWebpackConfig } from '../testUtils'; +import { findWebpackPlugin, materializeFinalWebpackConfig } from '../testUtils'; describe('Sentry webpack plugin config', () => { it('includes expected properties', async () => { @@ -283,44 +283,111 @@ describe('Sentry webpack plugin config', () => { }); }); - describe('disabling SentryWebpackPlugin', () => { - it('allows SentryWebpackPlugin to be turned off for client code (independent of server code)', () => { - const clientFinalNextConfig = materializeFinalNextConfig({ - ...exportedNextConfig, - sentry: { disableClientWebpackPlugin: true }, - }); - const clientFinalWebpackConfig = clientFinalNextConfig.webpack?.(clientWebpackConfig, clientBuildContext); + describe('SentryWebpackPlugin enablement', () => { + let processEnvBackup: typeof process.env; - const serverFinalNextConfig = materializeFinalNextConfig(exportedNextConfig, userSentryWebpackPluginConfig); - const serverFinalWebpackConfig = serverFinalNextConfig.webpack?.(serverWebpackConfig, serverBuildContext); - - expect(clientFinalWebpackConfig?.plugins).not.toEqual(expect.arrayContaining([expect.any(SentryWebpackPlugin)])); - expect(serverFinalWebpackConfig?.plugins).toEqual(expect.arrayContaining([expect.any(SentryWebpackPlugin)])); + beforeEach(() => { + processEnvBackup = { ...process.env }; }); - it('allows SentryWebpackPlugin to be turned off for server code (independent of client code)', () => { - const serverFinalNextConfig = materializeFinalNextConfig({ - ...exportedNextConfig, - sentry: { disableServerWebpackPlugin: true }, - }); - const serverFinalWebpackConfig = serverFinalNextConfig.webpack?.(serverWebpackConfig, serverBuildContext); - const clientFinalNextConfig = materializeFinalNextConfig(exportedNextConfig, userSentryWebpackPluginConfig); - const clientFinalWebpackConfig = clientFinalNextConfig.webpack?.(clientWebpackConfig, clientBuildContext); - - expect(serverFinalWebpackConfig?.plugins).not.toEqual(expect.arrayContaining([expect.any(SentryWebpackPlugin)])); - expect(clientFinalWebpackConfig?.plugins).toEqual(expect.arrayContaining([expect.any(SentryWebpackPlugin)])); + afterEach(() => { + process.env = processEnvBackup; }); - it("doesn't set devtool if webpack plugin is disabled", () => { - const finalNextConfig = materializeFinalNextConfig({ - ...exportedNextConfig, - webpack: () => ({ devtool: 'something-besides-source-map' } as any), - sentry: { disableServerWebpackPlugin: true }, - }); - const finalWebpackConfig = finalNextConfig.webpack?.(serverWebpackConfig, serverBuildContext); - - expect(finalWebpackConfig?.devtool).not.toEqual('source-map'); - }); + it.each([ + // [testName, exportedNextConfig, extraEnvValues, shouldFindServerPlugin, shouldFindClientPlugin] + [ + 'obeys `disableClientWebpackPlugin = true`', + { + ...exportedNextConfig, + sentry: { disableClientWebpackPlugin: true }, + }, + {}, + true, + false, + ], + + [ + 'obeys `disableServerWebpackPlugin = true`', + { + ...exportedNextConfig, + sentry: { disableServerWebpackPlugin: true }, + }, + {}, + false, + true, + ], + [ + 'disables the plugin in Vercel `preview` environment', + exportedNextConfig, + { VERCEL_ENV: 'preview' }, + false, + false, + ], + [ + 'disables the plugin in Vercel `development` environment', + exportedNextConfig, + { VERCEL_ENV: 'development' }, + false, + false, + ], + [ + 'allows `disableClientWebpackPlugin = false` to override env vars`', + { + ...exportedNextConfig, + sentry: { disableClientWebpackPlugin: false }, + }, + { VERCEL_ENV: 'preview' }, + false, + true, + ], + [ + 'allows `disableServerWebpackPlugin = false` to override env vars`', + { + ...exportedNextConfig, + sentry: { disableServerWebpackPlugin: false }, + }, + { VERCEL_ENV: 'preview' }, + true, + false, + ], + ])( + '%s', + async ( + _testName: string, + exportedNextConfig: ExportedNextConfig, + extraEnvValues: Record, + shouldFindServerPlugin: boolean, + shouldFindClientPlugin: boolean, + ) => { + process.env = { ...process.env, ...extraEnvValues }; + + // We create a copy of the next config for each `materializeFinalWebpackConfig` call because the `sentry` + // property gets deleted along the way, and its value matters for some of our test cases + const serverFinalWebpackConfig = await materializeFinalWebpackConfig({ + exportedNextConfig: { ...exportedNextConfig }, + userSentryWebpackPluginConfig, + incomingWebpackConfig: serverWebpackConfig, + incomingWebpackBuildContext: serverBuildContext, + }); + + const clientFinalWebpackConfig = await materializeFinalWebpackConfig({ + exportedNextConfig: { ...exportedNextConfig }, + userSentryWebpackPluginConfig, + incomingWebpackConfig: clientWebpackConfig, + incomingWebpackBuildContext: clientBuildContext, + }); + + const genericSentryWebpackPluginInstance = expect.any(SentryWebpackPlugin); + + expect(findWebpackPlugin(serverFinalWebpackConfig, 'SentryCliPlugin')).toEqual( + shouldFindServerPlugin ? genericSentryWebpackPluginInstance : undefined, + ); + expect(findWebpackPlugin(clientFinalWebpackConfig, 'SentryCliPlugin')).toEqual( + shouldFindClientPlugin ? genericSentryWebpackPluginInstance : undefined, + ); + }, + ); }); describe('getUserConfigFile', () => {