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

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');