fix(nextjs): Support custom distDir values in default RewriteFrames integration (#4017)

This is a follow-up to #3990, which makes sure sourcemaps get uploaded from the correct spot when using the nextjs `distDir` option[1]. 

To make sure that the filenames  in stacktrace frames match the names of said sourcemaps, we use the `RewriteFrames` integration. Up until now, we've been hardcoding the replacement it should make, based on nextjs's default output directory. This makes it dynamic, by injecting code at build time which adds the relevant value into `global`, where it is accessible at runtime when we're instantiating the integration.

[1] https://nextjs.org/docs/api-reference/next.config.js/setting-a-custom-build-directory

Author: lobsterkatie

packages/nextjs/src/config/types.ts

@@ -15,6 +15,8 @@ export type NextConfigObject = {
   webpack: WebpackConfigFunction;
   // whether to build serverless functions for all pages, not just API routes
   target: 'server' | 'experimental-serverless-trace';
+  // the output directory for the built app (defaults to ".next")
+  distDir: string;
   sentry?: {
     disableServerWebpackPlugin?: boolean;
     disableClientWebpackPlugin?: boolean;

packages/nextjs/src/config/webpack.ts

@@ -2,11 +2,11 @@ import { getSentryRelease } from '@sentry/node';
 import { dropUndefinedKeys, logger } from '@sentry/utils';
 import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin';
 import * as fs from 'fs';
+import * as os from 'os';
 import * as path from 'path';
 
 import {
   BuildContext,
-  EntryPointValue,
   EntryPropertyObject,
   NextConfigObject,
   SentryWebpackPluginOptions,
@@ -128,14 +128,31 @@ async function addSentryToEntryProperty(
   const newEntryProperty =
     typeof currentEntryProperty === 'function' ? await currentEntryProperty() : { ...currentEntryProperty };
 
+  // `sentry.server.config.js` or `sentry.client.config.js` (or their TS equivalents)
   const userConfigFile = buildContext.isServer
     ? getUserConfigFile(buildContext.dir, 'server')
     : getUserConfigFile(buildContext.dir, 'client');
 
+  // we need to turn the filename into a path so webpack can find it
+  const filesToInject = [`./${userConfigFile}`];
+
+  // Support non-default output directories by making the output path (easy to get here at build-time) available to the
+  // server SDK's default `RewriteFrames` instance (which needs it at runtime).
+  if (buildContext.isServer) {
+    const rewriteFramesHelper = path.resolve(
+      fs.mkdtempSync(path.resolve(os.tmpdir(), 'sentry-')),
+      'rewriteFramesHelper.js',
+    );
+    fs.writeFileSync(rewriteFramesHelper, `global.__rewriteFramesDistDir__ = '${buildContext.config.distDir}';\n`);
+    // stick our helper file ahead of the user's config file so the value is in the global namespace *before*
+    // `Sentry.init()` is called
+    filesToInject.unshift(rewriteFramesHelper);
+  }
+
+  // inject into all entry points which might contain user's code
   for (const entryPointName in newEntryProperty) {
     if (shouldAddSentryToEntryPoint(entryPointName)) {
-      // we need to turn the filename into a path so webpack can find it
-      addFileToExistingEntryPoint(newEntryProperty, entryPointName, `./${userConfigFile}`);
+      addFilesToExistingEntryPoint(newEntryProperty, entryPointName, filesToInject);
     }
   }
 
@@ -163,51 +180,52 @@ export function getUserConfigFile(projectDir: string, platform: 'server' | 'clie
 }
 
 /**
- * Add a file to a specific element of the given `entry` webpack config property.
+ * Add files to a specific element of the given `entry` webpack config property.
  *
  * @param entryProperty The existing `entry` config object
  * @param entryPointName The key where the file should be injected
- * @param filepath The path to the injected file
+ * @param filepaths An array of paths to the injected files
  */
-function addFileToExistingEntryPoint(
+function addFilesToExistingEntryPoint(
   entryProperty: EntryPropertyObject,
   entryPointName: string,
-  filepath: string,
+  filepaths: string[],
 ): void {
   // can be a string, array of strings, or object whose `import` property is one of those two
   const currentEntryPoint = entryProperty[entryPointName];
-  let newEntryPoint: EntryPointValue;
+  let newEntryPoint = currentEntryPoint;
 
   if (typeof currentEntryPoint === 'string') {
-    newEntryPoint = [filepath, currentEntryPoint];
+    newEntryPoint = [...filepaths, currentEntryPoint];
   } else if (Array.isArray(currentEntryPoint)) {
-    newEntryPoint = [filepath, ...currentEntryPoint];
+    newEntryPoint = [...filepaths, ...currentEntryPoint];
   }
   // descriptor object (webpack 5+)
   else if (typeof currentEntryPoint === 'object' && 'import' in currentEntryPoint) {
     const currentImportValue = currentEntryPoint.import;
     let newImportValue;
 
     if (typeof currentImportValue === 'string') {
-      newImportValue = [filepath, currentImportValue];
+      newImportValue = [...filepaths, currentImportValue];
     } else {
-      newImportValue = [filepath, ...currentImportValue];
+      newImportValue = [...filepaths, ...currentImportValue];
     }
 
     newEntryPoint = {
       ...currentEntryPoint,
       import: newImportValue,
     };
-  } else {
-    // mimic the logger prefix in order to use `console.warn` (which will always be printed, regardless of SDK settings)
+  }
+  // malformed entry point (use `console.error` rather than `logger.error` because it will always be printed, regardless
+  // of SDK settings)
+  else {
     // eslint-disable-next-line no-console
     console.error(
       'Sentry Logger [Error]:',
-      `Could not inject SDK initialization code into entry point ${entryPointName}, as it is not a recognized format.\n`,
+      `Could not inject SDK initialization code into entry point ${entryPointName}, as its current value is not in a recognized format.\n`,
       `Expected: string | Array<string> | { [key:string]: any, import: string | Array<string> }\n`,
       `Got: ${currentEntryPoint}`,
     );
-    return;
   }
 
   entryProperty[entryPointName] = newEntryPoint;

packages/nextjs/src/index.server.ts

@@ -1,6 +1,7 @@
 import { RewriteFrames } from '@sentry/integrations';
 import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node';
-import { logger } from '@sentry/utils';
+import { escapeStringForRegex, logger } from '@sentry/utils';
+import * as path from 'path';
 
 import { instrumentServer } from './utils/instrumentServer';
 import { MetadataBuilder } from './utils/metadataBuilder';
@@ -13,6 +14,8 @@ export * from '@sentry/node';
 // because or SSR of next.js we can only use this.
 export { ErrorBoundary, withErrorBoundary } from '@sentry/react';
 
+type GlobalWithDistDir = typeof global & { __rewriteFramesDistDir__: string };
+
 /** Inits the Sentry NextJS SDK on node. */
 export function init(options: NextjsOptions): void {
   if (options.debug) {
@@ -29,7 +32,6 @@ export function init(options: NextjsOptions): void {
   const metadataBuilder = new MetadataBuilder(options, ['nextjs', 'node']);
   metadataBuilder.addSdkMetadata();
   options.environment = options.environment || process.env.NODE_ENV;
-  // TODO capture project root and store in an env var for RewriteFrames?
   addServerIntegrations(options);
   // Right now we only capture frontend sessions for Next.js
   options.autoSessionTracking = false;
@@ -47,25 +49,29 @@ function sdkAlreadyInitialized(): boolean {
   return !!hub.getClient();
 }
 
-const SOURCEMAP_FILENAME_REGEX = /^.*\/\.next\//;
-
-const defaultRewriteFramesIntegration = new RewriteFrames({
-  iteratee: frame => {
-    frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next/');
-    return frame;
-  },
-});
-
-const defaultHttpTracingIntegration = new Integrations.Http({ tracing: true });
-
 function addServerIntegrations(options: NextjsOptions): void {
+  // This value is injected at build time, based on the output directory specified in the build config
+  const distDirName = (global as GlobalWithDistDir).__rewriteFramesDistDir__ || '.next';
+  // nextjs always puts the build directory at the project root level, which is also where you run `next start` from, so
+  // we can read in the project directory from the currently running process
+  const distDirAbsPath = path.resolve(process.cwd(), distDirName);
+  const SOURCEMAP_FILENAME_REGEX = new RegExp(escapeStringForRegex(distDirAbsPath));
+
+  const defaultRewriteFramesIntegration = new RewriteFrames({
+    iteratee: frame => {
+      frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next');
+      return frame;
+    },
+  });
+
   if (options.integrations) {
     options.integrations = addIntegration(defaultRewriteFramesIntegration, options.integrations);
   } else {
     options.integrations = [defaultRewriteFramesIntegration];
   }
 
   if (options.tracesSampleRate !== undefined || options.tracesSampler !== undefined) {
+    const defaultHttpTracingIntegration = new Integrations.Http({ tracing: true });
     options.integrations = addIntegration(defaultHttpTracingIntegration, options.integrations, {
       Http: { keyPath: '_tracing', value: true },
     });

packages/nextjs/test/config.test.ts

@@ -37,6 +37,32 @@ const mockExistsSync = (path: fs.PathLike) => {
 };
 const exitsSync = jest.spyOn(fs, 'existsSync').mockImplementation(mockExistsSync);
 
+// Make it so that all temporary folders, either created directly by tests or by the code they're testing, will go into
+// one spot that we know about, which we can then clean up when we're done
+const realTmpdir = jest.requireActual('os').tmpdir;
+const TEMP_DIR_PATH = path.join(realTmpdir(), 'sentry-nextjs-test');
+jest.spyOn(os, 'tmpdir').mockReturnValue(TEMP_DIR_PATH);
+// In theory, we should always land in the `else` here, but this saves the cases where the prior run got interrupted and
+// the `afterAll` below didn't happen.
+if (fs.existsSync(TEMP_DIR_PATH)) {
+  rimraf.sync(path.join(TEMP_DIR_PATH, '*'));
+} else {
+  fs.mkdirSync(TEMP_DIR_PATH);
+}
+
+afterAll(() => {
+  rimraf.sync(TEMP_DIR_PATH);
+});
+
+// In order to know what to expect in the webpack config `entry` property, we need to know the path of the temporary
+// directory created when doing the file injection, so wrap the real `mkdtempSync` and store the resulting path where we
+// can access it
+const mkdtempSyncSpy = jest.spyOn(fs, 'mkdtempSync');
+
+afterEach(() => {
+  mkdtempSyncSpy.mockClear();
+});
+
 /** Mocks of the arguments passed to `withSentryConfig` */
 const userNextConfig: Partial<NextConfigObject> = {
   publicRuntimeConfig: { location: 'dogpark', activities: ['fetch', 'chasing', 'digging'] },
@@ -103,7 +129,12 @@ function getBuildContext(
     dev: false,
     buildId: 'sItStAyLiEdOwN',
     dir: '/Users/Maisey/projects/squirrelChasingSimulator',
-    config: { target: 'server', ...userNextConfig } as NextConfigObject,
+    config: {
+      // nextjs's default values
+      target: 'server',
+      distDir: '.next',
+      ...userNextConfig,
+    } as NextConfigObject,
     webpack: { version: webpackVersion },
     isServer: buildTarget === 'server',
   };
@@ -279,26 +310,35 @@ describe('webpack config', () => {
         incomingWebpackBuildContext: serverBuildContext,
       });
 
+      const tempDir = mkdtempSyncSpy.mock.results[0].value;
+      const rewriteFramesHelper = path.join(tempDir, 'rewriteFramesHelper.js');
+
       expect(finalWebpackConfig.entry).toEqual(
         expect.objectContaining({
           // original entry point value is a string
           // (was 'private-next-pages/api/dogs/[name].js')
-          'pages/api/dogs/[name]': [serverConfigFilePath, 'private-next-pages/api/dogs/[name].js'],
+          'pages/api/dogs/[name]': [rewriteFramesHelper, serverConfigFilePath, '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': [serverConfigFilePath, './node_modules/smellOVision/index.js', 'private-next-pages/_app.js'],
+          'pages/_app': [
+            rewriteFramesHelper,
+            serverConfigFilePath,
+            './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: [serverConfigFilePath, 'private-next-pages/api/simulator/dogStats/[name].js'],
+            import: [rewriteFramesHelper, serverConfigFilePath, '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: [
+              rewriteFramesHelper,
               serverConfigFilePath,
               './node_modules/dogPoints/converter.js',
               'private-next-pages/api/simulator/leaderboard.js',
@@ -308,14 +348,14 @@ describe('webpack config', () => {
           // original entry point value is an object containg properties besides `import`
           // (`dependOn` remains untouched)
           'pages/api/tricks/[trickName]': {
-            import: [serverConfigFilePath, 'private-next-pages/api/tricks/[trickName].js'],
+            import: [rewriteFramesHelper, serverConfigFilePath, 'private-next-pages/api/tricks/[trickName].js'],
             dependOn: 'treats',
           },
         }),
       );
     });
 
-    it('does not inject into non-_app, non-API routes', async () => {
+    it('does not inject anything into non-_app, non-API routes', async () => {
       const finalWebpackConfig = await materializeFinalWebpackConfig({
         userNextConfig,
         incomingWebpackConfig: clientWebpackConfig,
@@ -326,12 +366,62 @@ describe('webpack config', () => {
         expect.objectContaining({
           // no injected file
           main: './src/index.ts',
-          // was 'next-client-pages-loader?page=%2F_app'
+        }),
+      );
+    });
+
+    it('does not inject `RewriteFrames` helper into client routes', async () => {
+      const finalWebpackConfig = await materializeFinalWebpackConfig({
+        userNextConfig,
+        incomingWebpackConfig: clientWebpackConfig,
+        incomingWebpackBuildContext: clientBuildContext,
+      });
+
+      expect(finalWebpackConfig.entry).toEqual(
+        expect.objectContaining({
+          // was 'next-client-pages-loader?page=%2F_app', and now has client config but not`RewriteFrames` helper injected
           'pages/_app': [clientConfigFilePath, 'next-client-pages-loader?page=%2F_app'],
         }),
       );
     });
   });
+
+  describe('`distDir` value in default server-side `RewriteFrames` integration', () => {
+    it.each([
+      ['no custom `distDir`', undefined, '.next'],
+      ['custom `distDir`', 'dist', 'dist'],
+    ])(
+      'creates file injecting `distDir` value into `global` - %s',
+      async (_name, customDistDir, expectedInjectedValue) => {
+        // Note: the fact that the file tested here gets injected correctly is covered in the 'webpack `entry` property
+        // config' tests above
+
+        const userNextConfigDistDir = {
+          ...userNextConfig,
+          ...(customDistDir && { distDir: customDistDir }),
+        };
+        await materializeFinalWebpackConfig({
+          userNextConfig: userNextConfigDistDir,
+          incomingWebpackConfig: serverWebpackConfig,
+          incomingWebpackBuildContext: getBuildContext('server', userNextConfigDistDir),
+        });
+
+        const tempDir = mkdtempSyncSpy.mock.results[0].value;
+        const rewriteFramesHelper = path.join(tempDir, 'rewriteFramesHelper.js');
+
+        expect(fs.existsSync(rewriteFramesHelper)).toBe(true);
+
+        const injectedCode = fs.readFileSync(rewriteFramesHelper).toString();
+        expect(injectedCode).toEqual(`global.__rewriteFramesDistDir__ = '${expectedInjectedValue}';\n`);
+      },
+    );
+
+    describe('`RewriteFrames` ends up with correct `distDir` value', () => {
+      // TODO: this, along with any number of other parts of the build process, should be tested with an integration
+      // test which actually runs webpack and inspects the resulting bundles (and that integration test should test
+      // custom `distDir` values with and without a `.`, to make sure the regex escaping is working)
+    });
+  });
 });
 
 describe('Sentry webpack plugin config', () => {
@@ -587,12 +677,11 @@ describe('Sentry webpack plugin config', () => {
     });
 
     beforeEach(() => {
+      // these will get cleaned up by the file's overall `afterAll` function, and the `mkdtempSync` mock above ensures
+      // that the location of the created folder is stored in `tempDir`
       const tempDirPathPrefix = path.join(os.tmpdir(), 'sentry-nextjs-test-');
-      tempDir = fs.mkdtempSync(tempDirPathPrefix);
-    });
-
-    afterEach(() => {
-      rimraf.sync(tempDir);
+      fs.mkdtempSync(tempDirPathPrefix);
+      tempDir = mkdtempSyncSpy.mock.results[0].value;
     });
 
     afterAll(() => {

packages/nextjs/test/index.server.test.ts

@@ -10,6 +10,9 @@ const { Integrations } = SentryNode;
 
 const global = getGlobalObject();
 
+// normally this is set as part of the build process, so mock it here
+(global as typeof global & { __rewriteFramesDistDir__: string }).__rewriteFramesDistDir__ = '.next';
+
 let configureScopeCallback: (scope: Scope) => void = () => undefined;
 jest.spyOn(SentryNode, 'configureScope').mockImplementation(callback => (configureScopeCallback = callback));
 const nodeInit = jest.spyOn(SentryNode, 'init');