ref(nextjs): Invert serverside injection criteria (#6206)

In nextjs, all non-API pages (other than sometimes `_error`)  include the `_app` component. Up until now, we've leveraged this fact when deciding which webpack entrypoints should have `sentry.server.config.js` injected during serverside build; specifically, we inject into `_app`, `_error`, and all API handlers, but not any other non-API pages.

This works fine, but it means that if we want to be able to pick and choose _which_ non-API pages get the config file injected, we're out of luck. Either we inject into `_app` (and therefore all non-API pages) or we don't (and therefore no non-API pages). In order to allow selective injection (which will be included in an upcoming PR), this inverts the logic: instead of `_app` being the only non-API page into which we inject, it is now one of the only pages we _don't_ inject into. (The other is `_document`, which plays a similar role as `_app` does.) Given that `_app` and `_document` can't stand on their own (without the context of a page component inside of them), they don't need to have Sentry injected separately. Having it injected into the pages `_app` and `_document` wrap is sufficient.

(Note that this change only applies to serverside injection. Client-side, we still only inject into `_app`, as we can't be selective about which pages get instrumented on the front end given all of the global monkeypatching we do.)

Author: lobsterkatie

packages/nextjs/src/config/webpack.ts

@@ -380,11 +380,37 @@ function checkWebpackPluginOverrides(
  * @returns `true` if sentry code should be injected, and `false` otherwise
  */
 function shouldAddSentryToEntryPoint(entryPointName: string, isServer: boolean): boolean {
-  return (
-    entryPointName === 'pages/_app' ||
-    (entryPointName.includes('pages/api') && !entryPointName.includes('_middleware')) ||
-    (isServer && entryPointName === 'pages/_error')
-  );
+  // On the server side, by default we inject the `Sentry.init()` code into every page (with a few exceptions).
+  if (isServer) {
+    const entryPointRoute = entryPointName.replace(/^pages/, '');
+    if (
+      // All non-API pages contain both of these components, and we don't want to inject more than once, so as long as
+      // we're doing the individual pages, it's fine to skip these. (Note: Even if a given user doesn't have either or
+      // both of these in their `pages/` folder, they'll exist as entrypoints because nextjs will supply default
+      // versions.)
+      entryPointRoute === '/_app' ||
+      entryPointRoute === '/_document' ||
+      // While middleware was in beta, it could be anywhere (at any level) in the `pages` directory, and would be called
+      // `_middleware.js`. Until the SDK runs successfully in the lambda edge environment, we have to exclude these.
+      entryPointName.includes('_middleware') ||
+      // Newer versions of nextjs are starting to introduce things outside the `pages/` folder (middleware, an `app/`
+      // directory, etc), but until those features are stable and we know how we want to support them, the safest bet is
+      // not to inject anywhere but inside `pages/`.
+      !entryPointName.startsWith('pages/')
+    ) {
+      return false;
+    }
+
+    // We want to inject Sentry into all other pages
+    return true;
+  }
+
+  // On the client side, we only want to inject into `_app`, because that guarantees there'll be only one copy of the
+  // SDK in the eventual bundle. Since `_app` is the (effectively) the root component for every nextjs app, inclusing
+  // Sentry there means it will be available for every front end page.
+  else {
+    return entryPointName === 'pages/_app';
+  }
 }
 
 /**

packages/nextjs/test/config/fixtures.ts

@@ -41,11 +41,12 @@ export const serverWebpackConfig: WebpackConfigObject = {
   entry: () =>
     Promise.resolve({
       'pages/_error': 'private-next-pages/_error.js',
-      'pages/_app': ['./node_modules/smellOVision/index.js', 'private-next-pages/_app.js'],
+      'pages/_app': 'private-next-pages/_app.js',
+      'pages/sniffTour': ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js'],
       'pages/api/_middleware': 'private-next-pages/api/_middleware.js',
       'pages/api/simulator/dogStats/[name]': { import: 'private-next-pages/api/simulator/dogStats/[name].js' },
-      'pages/api/simulator/leaderboard': {
-        import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/api/simulator/leaderboard.js'],
+      'pages/simulator/leaderboard': {
+        import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'],
       },
       'pages/api/tricks/[trickName]': {
         import: 'private-next-pages/api/tricks/[trickName].js',
@@ -64,6 +65,10 @@ export const clientWebpackConfig: WebpackConfigObject = {
       main: './src/index.ts',
       'pages/_app': 'next-client-pages-loader?page=%2F_app',
       'pages/_error': 'next-client-pages-loader?page=%2F_error',
+      'pages/sniffTour': ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js'],
+      'pages/simulator/leaderboard': {
+        import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'],
+      },
     }),
   output: { filename: 'static/chunks/[name].js', path: '/Users/Maisey/projects/squirrelChasingSimulator/.next' },
   target: 'web',

packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts

@@ -102,8 +102,12 @@ describe('constructWebpackConfigFunction()', () => {
           'pages/_error': [serverConfigFilePath, 'private-next-pages/_error.js'],
 
           // original entrypoint value is a string array
-          // (was ['./node_modules/smellOVision/index.js', 'private-next-pages/_app.js'])
-          'pages/_app': [serverConfigFilePath, './node_modules/smellOVision/index.js', 'private-next-pages/_app.js'],
+          // (was ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js'])
+          'pages/sniffTour': [
+            serverConfigFilePath,
+            './node_modules/smellOVision/index.js',
+            'private-next-pages/sniffTour.js',
+          ],
 
           // original entrypoint value is an object containing a string `import` value
           // (was { import: 'private-next-pages/api/simulator/dogStats/[name].js' })
@@ -112,12 +116,12 @@ describe('constructWebpackConfigFunction()', () => {
           },
 
           // original entrypoint value is an object containing a string array `import` value
-          // (was { import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/api/simulator/leaderboard.js'] })
-          'pages/api/simulator/leaderboard': {
+          // (was { import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'] })
+          'pages/simulator/leaderboard': {
             import: [
               serverConfigFilePath,
               './node_modules/dogPoints/converter.js',
-              'private-next-pages/api/simulator/leaderboard.js',
+              'private-next-pages/simulator/leaderboard.js',
             ],
           },
 
@@ -131,7 +135,7 @@ describe('constructWebpackConfigFunction()', () => {
       );
     });
 
-    it('injects user config file into `_app` in both server and client bundles', async () => {
+    it('injects user config file into `_app` in client bundle but not in server bundle', async () => {
       const finalServerWebpackConfig = await materializeFinalWebpackConfig({
         exportedNextConfig,
         incomingWebpackConfig: serverWebpackConfig,
@@ -145,7 +149,7 @@ describe('constructWebpackConfigFunction()', () => {
 
       expect(finalServerWebpackConfig.entry).toEqual(
         expect.objectContaining({
-          'pages/_app': expect.arrayContaining([serverConfigFilePath]),
+          'pages/_app': expect.not.arrayContaining([serverConfigFilePath]),
         }),
       );
       expect(finalClientWebpackConfig.entry).toEqual(
@@ -179,7 +183,7 @@ describe('constructWebpackConfigFunction()', () => {
       );
     });
 
-    it('injects user config file into API routes', async () => {
+    it('injects user config file into both API routes and non-API routes', async () => {
       const finalWebpackConfig = await materializeFinalWebpackConfig({
         exportedNextConfig,
         incomingWebpackConfig: serverWebpackConfig,
@@ -192,13 +196,13 @@ describe('constructWebpackConfigFunction()', () => {
             import: expect.arrayContaining([serverConfigFilePath]),
           },
 
-          'pages/api/simulator/leaderboard': {
-            import: expect.arrayContaining([serverConfigFilePath]),
-          },
-
           'pages/api/tricks/[trickName]': expect.objectContaining({
             import: expect.arrayContaining([serverConfigFilePath]),
           }),
+
+          'pages/simulator/leaderboard': {
+            import: expect.arrayContaining([serverConfigFilePath]),
+          },
         }),
       );
     });
@@ -218,19 +222,24 @@ describe('constructWebpackConfigFunction()', () => {
       );
     });
 
-    it('does not inject anything into non-_app, non-_error, non-API routes', async () => {
+    it('does not inject anything into non-_app pages during client build', async () => {
       const finalWebpackConfig = await materializeFinalWebpackConfig({
         exportedNextConfig,
         incomingWebpackConfig: clientWebpackConfig,
         incomingWebpackBuildContext: clientBuildContext,
       });
 
-      expect(finalWebpackConfig.entry).toEqual(
-        expect.objectContaining({
-          // no injected file
-          main: './src/index.ts',
-        }),
-      );
+      expect(finalWebpackConfig.entry).toEqual({
+        main: './src/index.ts',
+        // only _app has config file injected
+        'pages/_app': [clientConfigFilePath, 'next-client-pages-loader?page=%2F_app'],
+        'pages/_error': 'next-client-pages-loader?page=%2F_error',
+        'pages/sniffTour': ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js'],
+        'pages/simulator/leaderboard': {
+          import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'],
+        },
+        simulatorBundle: './src/simulator/index.ts',
+      });
     });
   });
 });