fix(nextjs): Stop SentryWebpackPlugin from uploading unnecessary fi…

…les (#3845)

Though sourcemap uploading using `SentryWebpackPlugin` is generally quite fast, even when there are a lot of files[1], there are situations in which it can slow things down a lot[2].

[1] #3769 (comment)
[2] vercel/next.js#26588 (comment)

There are a number of reasons for that, but one of them is that, in the current state, `@sentry/nextjs`'s use of the plugin is pretty indiscriminate - it just uploads anything and everything in the `.next` folder (which is where nextjs puts all of its built files), during both client and server builds. The lack of specificity leads us to upload files we don't need, and the fact that we don't distinguish between client and server builds means that whichever one happens second not only uploads its own unnecessary files, it also uploads all of the files from the other build (which have already been uploaded), both necessary and unnecessary. More details in #3769.

This change makes it so that we're much more specific in terms of what we upload, in order to fix both the specificity and the duplication problems.

Notes:

- There's discussion in the linked issue about which non-user sources/maps to upload (things like code from webpack, nextjs itself, etc). In the end, I chose to restrict it to user code (almost - see below), for two reasons. First, non-user code is unlikely to change much (or often) between releases, so if third-party code were included, we'd be uploading lots and lots of copies of the same files. Second, though it's occasionally helpful to be able to see such code in the stacktrace, the vast majority of the time the problem lies in user code. For both reasons, then, including third-party files didn't feel worth it , either in terms of time spent uploading them or space spent storing them.

  (I say it's "almost" restricted to user code because, among other files, the server build generates bundles which are labeled only by numbers (`1226.js` and such), some of which are user code and some of which are third-party code, and - in this PR at least - I didn't include a way to filter out the latter while making sure to still include the former. This is potentially still a worthwhile thing to do, but the fact that it would mean actually examining the contents of files (rather than just their paths) makes it a more complicated change, which will have to wait for a future PR.)

-  In that issue, the topic of HMR (hot module reloading) files also came up. For the moment I also chose to skip those (even though they contain user code), as a) the plugin isn't meant for use in a dev environment, where every change triggers a new build, potentially leading to hundreds of sets of release files being uploaded, and b) we'd face a similar issue of needing to examine more than just the path in order to know what to upload (to avoid uploading all previous iterations of the code at each rebuild).

- Another small issue I fixed, while I was in the relevant code, was to prevent the webpack plugin from injecting the release into bundles which don't need it (like third-party code, or any bundle with no `Sentry.init()` call). Since this lines up exactly with the files into which we need to inject `sentry.server.config.js` or `sentry.client.config.js`, I pulled it out into a function, `shouldAddSentryToEntryPoint()`.

Fixes #3769
Fixes vercel/next.js#26588

packages/nextjs/package.json

@@ -23,7 +23,7 @@
     "@sentry/react": "6.10.0",
     "@sentry/tracing": "6.10.0",
     "@sentry/utils": "6.10.0",
-    "@sentry/webpack-plugin": "1.15.1",
+    "@sentry/webpack-plugin": "1.17.1",
     "tslib": "^1.9.3"
   },
   "devDependencies": {

packages/nextjs/src/config/types.ts

@@ -1,4 +1,7 @@
-export { SentryCliPluginOptions as SentryWebpackPluginOptions } from '@sentry/webpack-plugin';
+import { SentryCliPluginOptions } from '@sentry/webpack-plugin';
+
+export type SentryWebpackPluginOptions = SentryCliPluginOptions;
+export type SentryWebpackPlugin = { options: SentryWebpackPluginOptions };
 
 /**
  * Overall Nextjs config
@@ -9,6 +12,8 @@ export type ExportedNextConfig = NextConfigObject | NextConfigFunction;
 export type NextConfigObject = {
   // custom webpack options
   webpack?: WebpackConfigFunction;
+  // whether to build serverless functions for all pages, not just API routes
+  target?: 'server' | 'experimental-serverless-trace';
   sentry?: {
     disableServerWebpackPlugin?: boolean;
     disableClientWebpackPlugin?: boolean;
@@ -43,7 +48,13 @@ export type WebpackConfigObject = {
 };
 
 // Information about the current build environment
-export type BuildContext = { dev: boolean; isServer: boolean; buildId: string; dir: string };
+export type BuildContext = {
+  dev: boolean;
+  isServer: boolean;
+  buildId: string;
+  dir: string;
+  config: Partial<NextConfigObject>;
+};
 
 /**
  * Webpack `entry` config

packages/nextjs/src/config/webpack.ts

@@ -21,18 +21,6 @@ export { SentryWebpackPlugin };
 // TODO: merge default SentryWebpackPlugin include with their SentryWebpackPlugin include
 // TODO: drop merged keys from override check? `includeDefaults` option?
 
-const defaultSentryWebpackPluginOptions = dropUndefinedKeys({
-  url: process.env.SENTRY_URL,
-  org: process.env.SENTRY_ORG,
-  project: process.env.SENTRY_PROJECT,
-  authToken: process.env.SENTRY_AUTH_TOKEN,
-  configFile: 'sentry.properties',
-  stripPrefix: ['webpack://_N_E/'],
-  urlPrefix: `~/_next`,
-  include: '.next/',
-  ignore: ['.next/cache', 'server/ssr-module-cache.js', 'static/*/_ssgManifest.js', 'static/*/_buildManifest.js'],
-});
-
 /**
  * Construct the function which will be used as the nextjs config's `webpack` value.
  *
@@ -88,18 +76,11 @@ export function constructWebpackConfigFunction(
         newConfig.devtool = 'source-map';
       }
 
-      checkWebpackPluginOverrides(userSentryWebpackPluginOptions);
-
       newConfig.plugins = newConfig.plugins || [];
       newConfig.plugins.push(
         // @ts-ignore Our types for the plugin are messed up somehow - TS wants this to be `SentryWebpackPlugin.default`,
         // but that's not actually a thing
-        new SentryWebpackPlugin({
-          dryRun: buildContext.dev,
-          release: getSentryRelease(buildContext.buildId),
-          ...defaultSentryWebpackPluginOptions,
-          ...userSentryWebpackPluginOptions,
-        }),
+        new SentryWebpackPlugin(getWebpackPluginOptions(buildContext, userSentryWebpackPluginOptions)),
       );
     }
 
@@ -136,7 +117,7 @@ async function addSentryToEntryProperty(
     : getUserConfigFile(buildContext.dir, 'client');
 
   for (const entryPointName in newEntryProperty) {
-    if (entryPointName === 'pages/_app' || entryPointName.includes('pages/api')) {
+    if (shouldAddSentryToEntryPoint(entryPointName)) {
       // we need to turn the filename into a path so webpack can find it
       addFileToExistingEntryPoint(newEntryProperty, entryPointName, `./${userConfigFile}`);
     }
@@ -221,13 +202,15 @@ function addFileToExistingEntryPoint(
  * our default options are getting overridden. (Note: If any of our default values is undefined, it won't be included in
  * the warning.)
  *
- * @param userSentryWebpackPluginOptions The user's SentryWebpackPlugin options
+ * @param defaultOptions Default SentryWebpackPlugin options
+ * @param userOptions The user's SentryWebpackPlugin options
  */
-function checkWebpackPluginOverrides(userSentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>): void {
+function checkWebpackPluginOverrides(
+  defaultOptions: SentryWebpackPluginOptions,
+  userOptions: Partial<SentryWebpackPluginOptions>,
+): void {
   // warn if any of the default options for the webpack plugin are getting overridden
-  const sentryWebpackPluginOptionOverrides = Object.keys(defaultSentryWebpackPluginOptions)
-    .concat('dryrun')
-    .filter(key => key in userSentryWebpackPluginOptions);
+  const sentryWebpackPluginOptionOverrides = Object.keys(defaultOptions).filter(key => key in userOptions);
   if (sentryWebpackPluginOptionOverrides.length > 0) {
     logger.warn(
       '[Sentry] You are overriding the following automatically-set SentryWebpackPlugin config options:\n' +
@@ -236,3 +219,58 @@ function checkWebpackPluginOverrides(userSentryWebpackPluginOptions: Partial<Sen
     );
   }
 }
+
+/**
+ * Determine if this is an entry point into which both `Sentry.init()` code and the release value should be injected
+ *
+ * @param entryPointName The name of the entry point in question
+ * @returns `true` if sentry code should be injected, and `false` otherwise
+ */
+function shouldAddSentryToEntryPoint(entryPointName: string): boolean {
+  return entryPointName === 'pages/_app' || entryPointName.includes('pages/api');
+}
+
+/**
+ * Combine default and user-provided SentryWebpackPlugin options, accounting for whether we're building server files or
+ * client files.
+ *
+ * @param buildContext Nexjs-provided data about the current build
+ * @param userPluginOptions User-provided SentryWebpackPlugin options
+ * @returns Final set of combined options
+ */
+function getWebpackPluginOptions(
+  buildContext: BuildContext,
+  userPluginOptions: Partial<SentryWebpackPluginOptions>,
+): SentryWebpackPluginOptions {
+  const { isServer, dir: projectDir, buildId, dev: isDev, config: nextConfig } = buildContext;
+
+  const isServerless = nextConfig.target === 'experimental-serverless-trace';
+  const hasSentryProperties = fs.existsSync(path.resolve(projectDir, 'sentry.properties'));
+
+  const serverInclude = isServerless
+    ? [{ paths: ['.next/serverless/'], urlPrefix: '~/_next/serverless' }]
+    : [
+        { paths: ['.next/server/chunks/'], urlPrefix: '~/_next/server/chunks' },
+        { paths: ['.next/server/pages/'], urlPrefix: '~/_next/server/pages' },
+      ];
+  const clientInclude = [{ paths: ['.next/static/chunks/pages'], urlPrefix: '~/_next/static/chunks/pages' }];
+
+  const defaultPluginOptions = dropUndefinedKeys({
+    include: isServer ? serverInclude : clientInclude,
+    ignore: [],
+    url: process.env.SENTRY_URL,
+    org: process.env.SENTRY_ORG,
+    project: process.env.SENTRY_PROJECT,
+    authToken: process.env.SENTRY_AUTH_TOKEN,
+    configFile: hasSentryProperties ? 'sentry.properties' : undefined,
+    stripPrefix: ['webpack://_N_E/'],
+    urlPrefix: `~/_next`,
+    entries: shouldAddSentryToEntryPoint,
+    release: getSentryRelease(buildId),
+    dryRun: isDev,
+  });
+
+  checkWebpackPluginOverrides(defaultPluginOptions, userPluginOptions);
+
+  return { ...defaultPluginOptions, ...userPluginOptions };
+}

packages/nextjs/test/config.test.ts

@@ -9,6 +9,7 @@ import {
   EntryPropertyFunction,
   ExportedNextConfig,
   NextConfigObject,
+  SentryWebpackPlugin as SentryWebpackPluginType,
   SentryWebpackPluginOptions,
   WebpackConfigObject,
 } from '../src/config/types';
@@ -44,7 +45,9 @@ const userNextConfig = {
       }),
   }),
 };
-const userSentryWebpackPluginConfig = { org: 'squirrelChasers', project: 'simulator', include: './thirdPartyMaps' };
+const userSentryWebpackPluginConfig = { org: 'squirrelChasers', project: 'simulator' };
+process.env.SENTRY_AUTH_TOKEN = 'dogsarebadatkeepingsecrets';
+process.env.SENTRY_RELEASE = 'doGsaREgReaT';
 
 /** Mocks of the arguments passed to the result of `withSentryConfig` (when it's a function). */
 const runtimePhase = 'ball-fetching';
@@ -80,10 +83,12 @@ const clientWebpackConfig = {
   target: 'web',
   context: '/Users/Maisey/projects/squirrelChasingSimulator',
 };
+
 const baseBuildContext = {
   dev: false,
-  buildId: 'doGsaREgReaT',
+  buildId: 'sItStAyLiEdOwN',
   dir: '/Users/Maisey/projects/squirrelChasingSimulator',
+  config: { target: 'server' as const },
 };
 const serverBuildContext = { isServer: true, ...baseBuildContext };
 const clientBuildContext = { isServer: false, ...baseBuildContext };
@@ -100,7 +105,7 @@ const clientBuildContext = { isServer: false, ...baseBuildContext };
  */
 function materializeFinalNextConfig(
   userNextConfig: ExportedNextConfig,
-  userSentryWebpackPluginConfig?: SentryWebpackPluginOptions,
+  userSentryWebpackPluginConfig?: Partial<SentryWebpackPluginOptions>,
 ): NextConfigObject {
   const sentrifiedConfig = withSentryConfig(userNextConfig, userSentryWebpackPluginConfig);
   let finalConfigValues = sentrifiedConfig;
@@ -131,7 +136,7 @@ function materializeFinalNextConfig(
  */
 async function materializeFinalWebpackConfig(options: {
   userNextConfig: ExportedNextConfig;
-  userSentryWebpackPluginConfig?: SentryWebpackPluginOptions;
+  userSentryWebpackPluginConfig?: Partial<SentryWebpackPluginOptions>;
   incomingWebpackConfig: WebpackConfigObject;
   incomingWebpackBuildContext: BuildContext;
 }): Promise<WebpackConfigObject> {
@@ -311,12 +316,40 @@ describe('webpack config', () => {
 });
 
 describe('Sentry webpack plugin config', () => {
-  it('includes expected properties', () => {
-    // TODO
+  it('includes expected properties', async () => {
+    // also, can pull from either env or user config (see notes on specific properties below)
+    const finalWebpackConfig = await materializeFinalWebpackConfig({
+      userNextConfig,
+      userSentryWebpackPluginConfig,
+      incomingWebpackConfig: serverWebpackConfig,
+      incomingWebpackBuildContext: serverBuildContext,
+    });
+
+    expect(finalWebpackConfig.plugins?.[0].options).toEqual(
+      expect.objectContaining({
+        include: expect.any(Array), // default, tested separately elsewhere
+        ignore: [], // default
+        org: 'squirrelChasers', // from user webpack plugin config
+        project: 'simulator', // from user webpack plugin config
+        authToken: 'dogsarebadatkeepingsecrets', // picked up from env
+        stripPrefix: ['webpack://_N_E/'], // default
+        urlPrefix: `~/_next`, // default
+        entries: expect.any(Function), // default, tested separately elsewhere
+        release: 'doGsaREgReaT', // picked up from env
+        dryRun: false, // based on buildContext.dev being false
+      }),
+    );
   });
 
-  it('preserves unrelated plugin config options', () => {
-    // TODO
+  it('preserves unrelated plugin config options', async () => {
+    const finalWebpackConfig = await materializeFinalWebpackConfig({
+      userNextConfig,
+      userSentryWebpackPluginConfig: { ...userSentryWebpackPluginConfig, debug: true },
+      incomingWebpackConfig: serverWebpackConfig,
+      incomingWebpackBuildContext: serverBuildContext,
+    });
+
+    expect((finalWebpackConfig.plugins?.[0].options as SentryWebpackPluginOptions).debug).toEqual(true);
   });
 
   it('warns when overriding certain default values', () => {
@@ -327,6 +360,51 @@ describe('Sentry webpack plugin config', () => {
     // do we even want to do this?
   });
 
+  describe('Sentry webpack plugin `include` option', () => {
+    it('has the correct value when building client bundles', async () => {
+      const finalWebpackConfig = await materializeFinalWebpackConfig({
+        userNextConfig,
+        incomingWebpackConfig: clientWebpackConfig,
+        incomingWebpackBuildContext: clientBuildContext,
+      });
+
+      const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
+
+      expect(sentryWebpackPlugin.options?.include).toEqual([
+        { paths: ['.next/static/chunks/pages'], urlPrefix: '~/_next/static/chunks/pages' },
+      ]);
+    });
+
+    it('has the correct value when building serverless server bundles', async () => {
+      const finalWebpackConfig = await materializeFinalWebpackConfig({
+        userNextConfig,
+        incomingWebpackConfig: serverWebpackConfig,
+        incomingWebpackBuildContext: { ...serverBuildContext, config: { target: 'experimental-serverless-trace' } },
+      });
+
+      const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
+
+      expect(sentryWebpackPlugin.options?.include).toEqual([
+        { paths: ['.next/serverless/'], urlPrefix: '~/_next/serverless' },
+      ]);
+    });
+
+    it('has the correct value when building serverful server bundles', async () => {
+      const finalWebpackConfig = await materializeFinalWebpackConfig({
+        userNextConfig,
+        incomingWebpackConfig: serverWebpackConfig,
+        incomingWebpackBuildContext: serverBuildContext,
+      });
+
+      const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
+
+      expect(sentryWebpackPlugin.options?.include).toEqual([
+        { paths: ['.next/server/chunks/'], urlPrefix: '~/_next/server/chunks' },
+        { paths: ['.next/server/pages/'], urlPrefix: '~/_next/server/pages' },
+      ]);
+    });
+  });
+
   it('allows SentryWebpackPlugin to be turned off for client code (independent of server code)', () => {
     const clientFinalNextConfig = materializeFinalNextConfig({
       ...userNextConfig,

yarn.lock

@@ -2624,10 +2624,10 @@
   resolved "https://registry.yarnpkg.com/@protobufjs/utf8/-/utf8-1.1.0.tgz#a777360b5b39a1a2e5106f8e858f2fd2d060c570"
   integrity sha1-p3c2C1s5oaLlEG+OhY8v0tBgxXA=
 
-"@sentry/cli@^1.64.1":
-  version "1.66.0"
-  resolved "https://registry.yarnpkg.com/@sentry/cli/-/cli-1.66.0.tgz#0526f1bc1c0570ce72ed817190af92f3b63a2e9a"
-  integrity sha512-2pZ+JHnvKqwyJWcGkKg/gCM/zURYronAnruBNllI+rH2g5IL0N90deMmjB1xcqXS66J222+MPTtWrGEK1Vl0/w==
+"@sentry/cli@^1.68.0":
+  version "1.68.0"
+  resolved "https://registry.yarnpkg.com/@sentry/cli/-/cli-1.68.0.tgz#2ced8fac67ee01e746a45e8ee45a518d4526937e"
+  integrity sha512-zc7+cxKDqpHLREGJKRH6KwE8fZW8bnczg3OLibJ0czleXoWPdAuOK1Xm1BTMcOnaXfg3VKAh0rI7S1PTdj+SrQ==
   dependencies:
     https-proxy-agent "^5.0.0"
     mkdirp "^0.5.5"
@@ -2636,12 +2636,12 @@
     progress "^2.0.3"
     proxy-from-env "^1.1.0"
 
-"@sentry/webpack-plugin@1.15.1":
-  version "1.15.1"
-  resolved "https://registry.yarnpkg.com/@sentry/webpack-plugin/-/webpack-plugin-1.15.1.tgz#deb014fce8c1b51811100f25ec9050dd03addd9b"
-  integrity sha512-/Z06MJDXyWcN2+CbeDTMDwVzySjgZWQajOke773TvpkgqdtkeT1eYBsA+pfsje+ZE1prEgrZdlH/L9HdM1odnQ==
+"@sentry/webpack-plugin@1.17.1":
+  version "1.17.1"
+  resolved "https://registry.yarnpkg.com/@sentry/webpack-plugin/-/webpack-plugin-1.17.1.tgz#1b3ebbe9991e4d77125ace2b24594059a088268a"
+  integrity sha512-L47a0hxano4a+9jbvQSBzHCT1Ph8fYAvGGUvFg8qc69yXS9si5lXRNIH/pavN6mqJjhQjAcEsEp+vxgvT4xZDQ==
   dependencies:
-    "@sentry/cli" "^1.64.1"
+    "@sentry/cli" "^1.68.0"
 
 "@simple-dom/interface@^1.4.0":
   version "1.4.0"