ref(nextjs): Inject init code in _app and API routes (#3786)

lobsterkatie · web-flow · commit 8063fbb4ffb9 · 2021-07-09T08:37:01.000-07:00
Change  how we include the user's `sentry.server.config.js` and `sentry.client.config.js` code (which is what calls `Sentry.init()`) in server and browser bundles, in order both to fix apps deployed to Vercel and to eliminate some semi-gross hacks included in the current implementation.

A great deal more detail in the PR description.
diff --git a/packages/nextjs/src/config/utils.ts b/packages/nextjs/src/config/utils.ts
diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts
@@ -4,7 +4,6 @@ import * as SentryWebpackPlugin from '@sentry/webpack-plugin';
 
 import {
   BuildContext,
-  EntryPointObject,
   EntryPointValue,
   EntryPropertyObject,
   NextConfigObject,
@@ -13,16 +12,15 @@ import {
   WebpackConfigObject,
   WebpackEntryProperty,
 } from './types';
-import { SERVER_SDK_INIT_PATH, storeServerConfigFileLocation } from './utils';
 
 export { SentryWebpackPlugin };
 
 // TODO: merge default SentryWebpackPlugin ignore with their SentryWebpackPlugin ignore or ignoreFile
 // TODO: merge default SentryWebpackPlugin include with their SentryWebpackPlugin include
 // TODO: drop merged keys from override check? `includeDefaults` option?
 
-const CLIENT_SDK_CONFIG_FILE = './sentry.client.config.js';
-const SERVER_SDK_CONFIG_FILE = './sentry.server.config.js';
+export const CLIENT_SDK_CONFIG_FILE = './sentry.client.config.js';
+export const SERVER_SDK_CONFIG_FILE = './sentry.server.config.js';
 
 const defaultSentryWebpackPluginOptions = dropUndefinedKeys({
   url: process.env.SENTRY_URL,
@@ -58,12 +56,6 @@ export function constructWebpackConfigFunction(
   const newWebpackFunction = (incomingConfig: WebpackConfigObject, buildContext: BuildContext): WebpackConfigObject => {
     let newConfig = { ...incomingConfig };
 
-    // if we're building server code, store the webpack output path as an env variable, so we know where to look for the
-    // webpack-processed version of `sentry.server.config.js` when we need it
-    if (newConfig.target === 'node') {
-      storeServerConfigFileLocation(newConfig);
-    }
-
     // if user has custom webpack config (which always takes the form of a function), run it so we have actual values to
     // work with
     if ('webpack' in userNextConfig && typeof userNextConfig.webpack === 'function') {
@@ -140,39 +132,11 @@ async function addSentryToEntryProperty(
   const newEntryProperty =
     typeof currentEntryProperty === 'function' ? await currentEntryProperty() : { ...currentEntryProperty };
 
-  // Add a new element to the `entry` array, we force webpack to create a bundle out of the user's
-  // `sentry.server.config.js` file and output it to `SERVER_INIT_LOCATION`. (See
-  // https://webpack.js.org/guides/code-splitting/#entry-points.) We do this so that the user's config file is run
-  // through babel (and any other processors through which next runs the rest of the user-provided code - pages, API
-  // routes, etc.). Specifically, we need any ESM-style `import` code to get transpiled into ES5, so that we can call
-  // `require()` on the resulting file when we're instrumenting the sesrver. (We can't use a dynamic import there
-  // because that then forces the user into a particular TS config.)
-
-  // On the server, create a separate bundle, as there's no one entry point depended on by all the others
-  if (buildContext.isServer) {
-    // slice off the final `.js` since webpack is going to add it back in for us, and we don't want to end up with
-    // `.js.js` as the extension
-    newEntryProperty[SERVER_SDK_INIT_PATH.slice(0, -3)] = SERVER_SDK_CONFIG_FILE;
-  }
-  // On the client, it's sufficient to inject it into the `main` JS code, which is included in every browser page.
-  else {
-    addFileToExistingEntryPoint(newEntryProperty, 'main', CLIENT_SDK_CONFIG_FILE);
-
-    // To work around a bug in nextjs, we need to ensure that the `main.js` entry is empty (otherwise it'll choose that
-    // over `main` and we'll lose the change we just made). In case some other library has put something into it, copy
-    // its contents over before emptying it out. See
-    // https://github.com/getsentry/sentry-javascript/pull/3696#issuecomment-863363803.)
-    const mainjsValue = newEntryProperty['main.js'];
-    if (Array.isArray(mainjsValue) && mainjsValue.length > 0) {
-      const mainValue = newEntryProperty.main;
-
-      // copy the `main.js` entries over
-      newEntryProperty.main = Array.isArray(mainValue)
-        ? [...mainjsValue, ...mainValue]
-        : { ...(mainValue as EntryPointObject), import: [...mainjsValue, ...(mainValue as EntryPointObject).import] };
-
-      // nuke the entries
-      newEntryProperty['main.js'] = [];
+  const userConfigFile = buildContext.isServer ? SERVER_SDK_CONFIG_FILE : CLIENT_SDK_CONFIG_FILE;
+
+  for (const entryPointName in newEntryProperty) {
+    if (entryPointName === 'pages/_app' || entryPointName.includes('pages/api')) {
+      addFileToExistingEntryPoint(newEntryProperty, entryPointName, userConfigFile);
     }
   }
 
@@ -195,22 +159,20 @@ function addFileToExistingEntryPoint(
   const currentEntryPoint = entryProperty[entryPointName];
   let newEntryPoint: EntryPointValue;
 
-  // We inject the user's client config file after the existing code so that the config file has access to
-  // `publicRuntimeConfig`. See https://github.com/getsentry/sentry-javascript/issues/3485
   if (typeof currentEntryPoint === 'string') {
-    newEntryPoint = [currentEntryPoint, filepath];
+    newEntryPoint = [filepath, currentEntryPoint];
   } else if (Array.isArray(currentEntryPoint)) {
-    newEntryPoint = [...currentEntryPoint, filepath];
+    newEntryPoint = [filepath, ...currentEntryPoint];
   }
   // descriptor object (webpack 5+)
   else if (typeof currentEntryPoint === 'object' && 'import' in currentEntryPoint) {
     const currentImportValue = currentEntryPoint.import;
-    let newImportValue: string | string[];
+    let newImportValue;
 
     if (typeof currentImportValue === 'string') {
-      newImportValue = [currentImportValue, filepath];
+      newImportValue = [filepath, currentImportValue];
     } else {
-      newImportValue = [...currentImportValue, filepath];
+      newImportValue = [filepath, ...currentImportValue];
     }
 
     newEntryPoint = {
diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts
@@ -5,7 +5,6 @@ import { fill, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'
 import * as domain from 'domain';
 import * as http from 'http';
 import { default as createNextServer } from 'next';
-import * as path from 'path';
 import * as querystring from 'querystring';
 import * as url from 'url';
 
@@ -111,18 +110,6 @@ function makeWrappedHandlerGetter(origHandlerGetter: HandlerGetter): WrappedHand
   // Otherwise, it's just a pass-through to the original method.
   const wrappedHandlerGetter = async function(this: NextServer): Promise<ReqHandler> {
     if (!sdkSetupComplete) {
-      try {
-        // `SENTRY_SERVER_INIT_PATH` is set at build time, and points to a webpack-processed version of the user's
-        // `sentry.server.config.js`. Requiring it starts the SDK.
-        require(path.resolve(process.env.SENTRY_SERVER_INIT_PATH as string));
-      } catch (err) {
-        // Log the error but don't bail - we still want the wrapping to happen, in case the user is doing something weird
-        // and manually calling `Sentry.init()` somewhere else. We log to console instead of using logger from utils
-        // because Sentry is not initialized.
-        // eslint-disable-next-line no-console
-        console.error(`[Sentry] Could not initialize SDK. Received error:\n${err}`);
-      }
-
       // stash this in the closure so that `makeWrappedReqHandler` can use it
       liveServer = this.server;
       const serverPrototype = Object.getPrototypeOf(liveServer);
diff --git a/packages/nextjs/test/config.test.ts b/packages/nextjs/test/config.test.ts
@@ -7,20 +7,14 @@ import {
   SentryWebpackPluginOptions,
   WebpackConfigObject,
 } from '../src/config/types';
-import { SENTRY_SERVER_CONFIG_FILE, SERVER_SDK_INIT_PATH } from '../src/config/utils';
-import { constructWebpackConfigFunction, SentryWebpackPlugin } from '../src/config/webpack';
-
-// mock `storeServerConfigFileLocation` in order to make it a no-op when necessary
-jest.mock('../src/config/utils', () => {
-  const original = jest.requireActual('../src/config/utils');
-  return {
-    ...original,
-    // nuke this so it won't try to look for our dummy paths
-    storeServerConfigFileLocation: jest.fn(),
-  };
-});
+import {
+  CLIENT_SDK_CONFIG_FILE,
+  constructWebpackConfigFunction,
+  SentryWebpackPlugin,
+  SERVER_SDK_CONFIG_FILE,
+} from '../src/config/webpack';
 
-/** mocks of the arguments passed to `withSentryConfig` */
+/** Mocks of the arguments passed to `withSentryConfig` */
 const userNextConfig = {
   publicRuntimeConfig: { location: 'dogpark', activities: ['fetch', 'chasing', 'digging'] },
   webpack: (config: WebpackConfigObject, _options: BuildContext) => ({
@@ -35,19 +29,36 @@ const userNextConfig = {
 };
 const userSentryWebpackPluginConfig = { org: 'squirrelChasers', project: 'simulator', include: './thirdPartyMaps' };
 
-/** mocks of the arguments passed to the result of `withSentryConfig` (when it's a function) */
-const runtimePhase = 'puppy-phase-chew-everything-in-sight';
+/** Mocks of the arguments passed to the result of `withSentryConfig` (when it's a function). */
+const runtimePhase = 'ball-fetching';
 const defaultNextConfig = { nappingHoursPerDay: 20, oversizeFeet: true, shouldChaseTail: true };
 
 /** mocks of the arguments passed to `nextConfig.webpack` */
 const serverWebpackConfig = {
-  entry: () => Promise.resolve({ 'pages/api/dogs/[name]': 'private-next-pages/api/dogs/[name].js' }),
+  entry: () =>
+    Promise.resolve({
+      'pages/api/dogs/[name]': 'private-next-pages/api/dogs/[name].js',
+      'pages/_app': ['./node_modules/smellOVision/index.js', 'private-next-pages/_app.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/api/tricks/[trickName]': {
+        import: 'private-next-pages/api/tricks/[trickName].js',
+        dependOn: 'treats',
+      },
+      treats: './node_modules/dogTreats/treatProvider.js',
+    }),
   output: { filename: '[name].js', path: '/Users/Maisey/projects/squirrelChasingSimulator/.next' },
   target: 'node',
   context: '/Users/Maisey/projects/squirrelChasingSimulator',
 };
 const clientWebpackConfig = {
-  entry: () => Promise.resolve({ main: './src/index.ts' }),
+  entry: () =>
+    Promise.resolve({
+      main: './src/index.ts',
+      'pages/_app': 'next-client-pages-loader?page=%2F_app',
+    }),
   output: { filename: 'static/chunks/[name].js', path: '/Users/Maisey/projects/squirrelChasingSimulator/.next' },
   target: 'web',
   context: '/Users/Maisey/projects/squirrelChasingSimulator',
@@ -212,7 +223,7 @@ describe('webpack config', () => {
   });
 
   describe('webpack `entry` property config', () => {
-    it('injects correct code when building server bundle', async () => {
+    it('handles various entrypoint shapes', async () => {
       const finalWebpackConfig = await materializeFinalWebpackConfig({
         userNextConfig,
         incomingWebpackConfig: serverWebpackConfig,
@@ -221,38 +232,53 @@ describe('webpack config', () => {
 
       expect(finalWebpackConfig.entry).toEqual(
         expect.objectContaining({
-          [SERVER_SDK_INIT_PATH.slice(0, -3)]: SENTRY_SERVER_CONFIG_FILE,
+          // original entry point value is a string
+          // (was 'private-next-pages/api/dogs/[name].js')
+          'pages/api/dogs/[name]': [SERVER_SDK_CONFIG_FILE, 'private-next-pages/api/dogs/[name].js'],
+
+          // original entry point value is a string array
+          // (was ['./node_modules/smellOVision/index.js', 'private-next-pages/_app.js'])
+          'pages/_app': [SERVER_SDK_CONFIG_FILE, './node_modules/smellOVision/index.js', 'private-next-pages/_app.js'],
+
+          // original entry point value is an object containing a string `import` value
+          // (`import` was 'private-next-pages/api/simulator/dogStats/[name].js')
+          'pages/api/simulator/dogStats/[name]': {
+            import: [SERVER_SDK_CONFIG_FILE, 'private-next-pages/api/simulator/dogStats/[name].js'],
+          },
+
+          // original entry point value is an object containing a string array `import` value
+          // (`import` was ['./node_modules/dogPoints/converter.js', 'private-next-pages/api/simulator/leaderboard.js'])
+          'pages/api/simulator/leaderboard': {
+            import: [
+              SERVER_SDK_CONFIG_FILE,
+              './node_modules/dogPoints/converter.js',
+              'private-next-pages/api/simulator/leaderboard.js',
+            ],
+          },
+
+          // original entry point value is an object containg properties besides `import`
+          // (`dependOn` remains untouched)
+          'pages/api/tricks/[trickName]': {
+            import: [SERVER_SDK_CONFIG_FILE, 'private-next-pages/api/tricks/[trickName].js'],
+            dependOn: 'treats',
+          },
         }),
       );
     });
 
-    it('injects correct code when building client bundle', async () => {
+    it('does not inject into non-_app, non-API routes', async () => {
       const finalWebpackConfig = await materializeFinalWebpackConfig({
         userNextConfig,
         incomingWebpackConfig: clientWebpackConfig,
         incomingWebpackBuildContext: clientBuildContext,
       });
 
-      expect(finalWebpackConfig.entry).toEqual(
-        expect.objectContaining({ main: ['./src/index.ts', './sentry.client.config.js'] }),
-      );
-    });
-
-    // see https://github.com/getsentry/sentry-javascript/pull/3696#issuecomment-863363803
-    it('handles non-empty `main.js` entry point', async () => {
-      const finalWebpackConfig = await materializeFinalWebpackConfig({
-        userNextConfig,
-        incomingWebpackConfig: {
-          ...clientWebpackConfig,
-          entry: () => Promise.resolve({ main: './src/index.ts', 'main.js': ['sitLieDownRollOver.config.js'] }),
-        },
-        incomingWebpackBuildContext: clientBuildContext,
-      });
-
       expect(finalWebpackConfig.entry).toEqual(
         expect.objectContaining({
-          main: ['sitLieDownRollOver.config.js', './src/index.ts', './sentry.client.config.js'],
-          'main.js': [],
+          // no injected file
+          main: './src/index.ts',
+          // was 'next-client-pages-loader?page=%2F_app'
+          'pages/_app': [CLIENT_SDK_CONFIG_FILE, 'next-client-pages-loader?page=%2F_app'],
         }),
       );
     });
