ref(nextjs): Wrap server-side data-fetching methods during build (#5503)

This adds wrapping of the nextjs server-side data-fetching methods (`getStaticPaths`, `getStaticProps`, and `getServerSideProps`) to the webpack config changes we make during app build. Eventually, this will let us create spans for them, use them to improve route parameterization, and trace requests which aren't currently getting traced on Vercel-deployed nextjs apps. For the moment, though, all the wrappers do is call the functions they wrap, in order to separate out the wrapping into its own PR.

Notes:

-  For the moment, in order to allow us to iterate on it without fear of breaking people's apps, this feature is gated behind the `sentry.autoWrapDataFetchers` flag in `next.config.js`.

- To do the wrapping, we scan each page's AST for references to each of the three functions. If a given function is found, it is renamed (and all references to it similarly adjusted) so that its name starts with one or more underscores (so `getServerSideProps` becomes `_getServerSideProps`, for example). This then allows us to create a new, wrapped version of the (now-renamed) function, and export that wrapped version under the original name. (For example, we can then add `export const getServerSideProps = withSentryGSSP(_getServerSideProps)` to the page code using the loader, and nextjs will never know the difference.)

- Parsing of user code into an AST (and subsequent traversal of that AST) is done using `@babel/parser` and `jscodeshift`. (`@babel/parser` is used directly because `jscodeshift` neither exposes its parsers nor provides an option for specifying a parser by name (at least when used programmatically). Therefore, in order to use a jsx parser for JS pages and a tsx parser for TS pages, we need to supply them ourselves.)

Follow-up work:

- Add tests
- Make these wrappers do more than call the original functions

Author: lobsterkatie

packages/nextjs/package.json

@@ -17,6 +17,7 @@
     "access": "public"
   },
   "dependencies": {
+    "@babel/parser": "^7.18.10",
     "@sentry/core": "7.9.0",
     "@sentry/hub": "7.9.0",
     "@sentry/integrations": "7.9.0",
@@ -26,9 +27,12 @@
     "@sentry/types": "7.9.0",
     "@sentry/utils": "7.9.0",
     "@sentry/webpack-plugin": "1.19.0",
+    "jscodeshift": "^0.13.1",
     "tslib": "^1.9.3"
   },
   "devDependencies": {
+    "@babel/types": "7.18.10",
+    "@types/jscodeshift": "^0.11.5",
     "@types/webpack": "^4.41.31",
     "next": "10.1.3"
   },

packages/nextjs/rollup.npm.config.js

@@ -14,32 +14,34 @@ export default [
   ),
   ...makeNPMConfigVariants(
     makeBaseNPMConfig({
-      entrypoints: ['src/config/templates/prefixLoaderTemplate.ts'],
+      entrypoints: [
+        'src/config/templates/prefixLoaderTemplate.ts',
+        'src/config/templates/dataFetchersLoaderTemplate.ts',
+      ],
 
       packageSpecificConfig: {
         output: {
-          // preserve the original file structure (i.e., so that everything is still relative to `src`)
+          // Preserve the original file structure (i.e., so that everything is still relative to `src`). (Not entirely
+          // clear why this is necessary here and not for other entrypoints in this file.)
           entryFileNames: 'config/templates/[name].js',
 
           // this is going to be add-on code, so it doesn't need the trappings of a full module (and in fact actively
           // shouldn't have them, lest they muck with the module to which we're adding it)
           sourcemap: false,
           esModule: false,
         },
+        external: ['@sentry/nextjs'],
       },
     }),
   ),
   ...makeNPMConfigVariants(
     makeBaseNPMConfig({
-      entrypoints: ['src/config/loaders/prefixLoader.ts'],
+      entrypoints: ['src/config/loaders/index.ts'],
 
       packageSpecificConfig: {
         output: {
           // make it so Rollup calms down about the fact that we're doing `export { loader as default }`
-          exports: 'default',
-
-          // preserve the original file structure (i.e., so that everything is still relative to `src`)
-          entryFileNames: 'config/loaders/[name].js',
+          exports: 'named',
         },
       },
     }),

packages/nextjs/src/config/loaders/ast.ts

@@ -0,0 +1,137 @@
+/**
+ * This loader auto-wraps a user's page-level data-fetching functions (`getStaticPaths`, `getStaticProps`, and
+ * `getServerSideProps`) in order to instrument them for tracing. At a high level, this is done by finding the relevant
+ * functions, renaming them so as not to create a name collision, and then creating a new version of each function which
+ * is a wrapped version of the original. We do this by parsing the user's code and some template code into ASTs,
+ * manipulating them, and then turning them back into strings and appending our template code to the user's (modified)
+ * page code. Greater detail and explanations can be found in situ in the functions below and in the helper functions in
+ * `ast.ts`.
+ */
+
+import { logger } from '@sentry/utils';
+import * as fs from 'fs';
+import * as path from 'path';
+
+import { isESM } from '../../utils/isESM';
+import type { AST } from './ast';
+import { findDeclarations, findExports, makeAST, removeComments, renameIdentifiers } from './ast';
+import type { LoaderThis } from './types';
+
+// Map to keep track of each function's placeholder in the template and what it should be replaced with. (The latter
+// will get added as we process the user code. Setting it to an empty string here means TS won't complain when we set it
+// to a non-empty string later.)
+const DATA_FETCHING_FUNCTIONS = {
+  getServerSideProps: { placeholder: '__ORIG_GSSP__', alias: '' },
+  getStaticProps: { placeholder: '__ORIG_GSPROPS__', alias: '' },
+  getStaticPaths: { placeholder: '__ORIG_GSPATHS__', alias: '' },
+};
+
+type LoaderOptions = {
+  projectDir: string;
+};
+
+/**
+ * Find any data-fetching functions the user's code contains and rename them to prevent clashes, then whittle the
+ * template exporting wrapped versions instead down to only the functions found.
+ *
+ * @param userCode The source code of the current page file
+ * @param templateCode The source code of the full template, including all functions
+ * @param filepath The path to the current pagefile, within the project directory
+ * @returns A tuple of modified user and template code
+ */
+function wrapFunctions(userCode: string, templateCode: string, filepath: string): string[] {
+  let userAST: AST, templateAST: AST;
+  const isTS = new RegExp('\\.tsx?$').test(filepath);
+
+  try {
+    userAST = makeAST(userCode, isTS);
+    templateAST = makeAST(templateCode, false);
+  } catch (err) {
+    logger.warn(`Couldn't add Sentry to ${filepath} because there was a parsing error: ${err}`);
+    // Replace the template code with an empty string, so in the end the user code is untouched
+    return [userCode, ''];
+  }
+
+  // Comments are useful to have in the template for anyone reading it, but don't make sense to be injected into user
+  // code, because they're about the template-i-ness of the template, not the code itself
+  // TODO: Move this to our rollup build
+  removeComments(templateAST);
+
+  for (const functionName of Object.keys(DATA_FETCHING_FUNCTIONS)) {
+    // Find and rename all identifiers whose name is `functionName`
+    const alias = renameIdentifiers(userAST, functionName);
+
+    // `alias` will be defined iff the user code contains the function in question and renaming has been done
+    if (alias) {
+      // We keep track of the alias for each function, so that later on we can fill it in for the placeholder in the
+      // template. (Not doing that now because it's much more easily done once the template code has gone back to being
+      // a string.)
+      DATA_FETCHING_FUNCTIONS[functionName as keyof typeof DATA_FETCHING_FUNCTIONS].alias = alias;
+    }
+
+    // Otherwise, if the current function doesn't exist anywhere in the user's code, delete the code in the template
+    // wrapping that function
+    //
+    // Note: We start with all of the possible wrapper lines in the template and delete the ones we don't need (rather
+    // than starting with none and adding in the ones we do need) because it allows them to live in our souce code as
+    // *code*. If we added them in, they'd have to be strings containing code, and we'd lose all of the benefits of
+    // syntax highlighting, linting, etc.
+    else {
+      // We have to look for declarations and exports separately because when we build the SDK, Rollup turns
+      //     export const XXX = ...
+      // into
+      //     const XXX = ...
+      //     export { XXX }
+      findExports(templateAST, functionName).remove();
+      findDeclarations(templateAST, functionName).remove();
+    }
+  }
+
+  return [userAST.toSource(), templateAST.toSource()];
+}
+
+/**
+ * Wrap `getStaticPaths`, `getStaticProps`, and `getServerSideProps` (if they exist) in the given page code
+ */
+export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
+  // We know one or the other will be defined, depending on the version of webpack being used
+  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+  const { projectDir } = this.getOptions ? this.getOptions() : this.query!;
+
+  // For now this loader only works for ESM code
+  if (!isESM(userCode)) {
+    return userCode;
+  }
+
+  // If none of the functions we want to wrap appears in the page's code, there's nothing to do. (Note: We do this as a
+  // simple substring match (rather than waiting until we've parsed the code) because it's meant to be an
+  // as-fast-as-possible fail-fast. It's possible for user code to pass this check, even if it contains none of the
+  // functions in question, just by virtue of the correct string having been found, be it in a comment, as part of a
+  // longer variable name, etc. That said, when we actually do the code manipulation we'll be working on the code's AST,
+  // meaning we'll be able to differentiate between code we actually want to change and any false positives which might
+  // come up here.)
+  if (Object.keys(DATA_FETCHING_FUNCTIONS).every(functionName => !userCode.includes(functionName))) {
+    return userCode;
+  }
+
+  const templatePath = path.resolve(__dirname, '../templates/dataFetchersLoaderTemplate.js');
+  // make sure the template is included when runing `webpack watch`
+  this.addDependency(templatePath);
+
+  const templateCode = fs.readFileSync(templatePath).toString();
+
+  const [modifiedUserCode, modifiedTemplateCode] = wrapFunctions(
+    userCode,
+    templateCode,
+    // Relative path to the page we're currently processing, for use in error messages
+    path.relative(projectDir, this.resourcePath),
+  );
+
+  // Fill in template placeholders
+  let injectedCode = modifiedTemplateCode;
+  for (const { placeholder, alias } of Object.values(DATA_FETCHING_FUNCTIONS)) {
+    injectedCode = injectedCode.replace(placeholder, alias);
+  }
+
+  return `${modifiedUserCode}\n${injectedCode}`;
+}

packages/nextjs/src/config/loaders/dataFetchersLoader.ts

@@ -0,0 +1,2 @@
+export { default as prefixLoader } from './prefixLoader';
+export { default as dataFetchersLoader } from './dataFetchersLoader';

packages/nextjs/src/config/loaders/index.ts

@@ -0,0 +1,64 @@
+/**
+ * Note: The implementation here is loosely based on the jsx and tsx parsers in 'jscodeshift'. It doesn't expose its
+ * parsers, so we have to provide our own if we want to use anything besides the default. Fortunately, its parsers turn
+ * out to just be wrappers around `babel.parse` with certain options set. The options chosen here are different from the
+ * `jscodeshift` parsers in that a) unrecognized and deprecated options and options set to default values have been
+ * removed, and b) all standard plugins are included, meaning the widest range of user code is able to be parsed.
+ */
+
+import * as babel from '@babel/parser';
+import { File } from '@babel/types';
+
+type Parser = {
+  parse: (code: string) => babel.ParseResult<File>;
+};
+
+const jsxOptions: babel.ParserOptions = {
+  // Nextjs supports dynamic import, so this seems like a good idea
+  allowImportExportEverywhere: true,
+  // We're only supporting wrapping in ESM pages
+  sourceType: 'module',
+  // Without `tokens`, jsx parsing breaks
+  tokens: true,
+  // The maximal set of non-mutually-exclusive standard plugins, so as to support as much weird syntax in our users'
+  // code as possible
+  plugins: [
+    'asyncDoExpressions',
+    'decimal',
+    ['decorators', { decoratorsBeforeExport: false }],
+    'decoratorAutoAccessors',
+    'destructuringPrivate',
+    'doExpressions',
+    'estree',
+    'exportDefaultFrom',
+    'functionBind',
+    'importMeta',
+    'importAssertions',
+    'jsx',
+    'moduleBlocks',
+    'partialApplication',
+    ['pipelineOperator', { proposal: 'hack', topicToken: '^' }],
+    'regexpUnicodeSets',
+    'throwExpressions',
+  ] as babel.ParserPlugin[],
+};
+
+const tsxOptions = {
+  ...jsxOptions,
+  // Because `jsxOptions` is typed as a `ParserOptions` object, TS doesn't discount the possibility of its `plugins`
+  // property being undefined, even though it is, in fact, very clearly defined - hence the empty array.
+  plugins: [...(jsxOptions.plugins || []), 'typescript'] as babel.ParserPlugin[],
+};
+
+/**
+ * Create either a jsx or tsx parser to be used by `jscodeshift`.
+ *
+ * @param type Either 'jsx' or 'tsx'
+ * @returns An object with the appropriate `parse` method.
+ */
+export function makeParser(type: 'jsx' | 'tsx'): Parser {
+  const options = type === 'jsx' ? jsxOptions : tsxOptions;
+  return {
+    parse: code => babel.parse(code, options),
+  };
+}

packages/nextjs/src/config/loaders/parsers.ts

@@ -10,7 +10,7 @@ type LoaderOptions = {
 /**
  * Inject templated code into the beginning of a module.
  */
-function prefixLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
+export default function prefixLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
   // We know one or the other will be defined, depending on the version of webpack being used
   // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
   const { distDir } = this.getOptions ? this.getOptions() : this.query!;
@@ -25,5 +25,3 @@ function prefixLoader(this: LoaderThis<LoaderOptions>, userCode: string): string
 
   return `${templateCode}\n${userCode}`;
 }
-
-export { prefixLoader as default };

packages/nextjs/src/config/loaders/prefixLoader.ts

@@ -1,5 +1,8 @@
 // TODO Use real webpack types
 export type LoaderThis<Options> = {
+  // Path to the file being loaded
+  resourcePath: string;
+
   // Loader options in Webpack 4
   query?: Options;
   // Loader options in Webpack 5

packages/nextjs/src/config/loaders/types.ts

@@ -0,0 +1,34 @@
+import type {
+  GetServerSideProps as GetServerSidePropsFunction,
+  GetStaticPaths as GetStaticPathsFunction,
+  GetStaticProps as GetStaticPropsFunction,
+} from 'next';
+
+declare const __ORIG_GSSP__: GetServerSidePropsFunction;
+declare const __ORIG_GSPROPS__: GetStaticPropsFunction;
+declare const __ORIG_GSPATHS__: GetStaticPathsFunction;
+
+// We import the SDK under a purposefully clunky name, to lessen to near zero the chances of a name collision in case
+// the user has also imported Sentry for some reason. (In the future, we could check for such a collision using the AST,
+// but this is a lot simpler.)
+//
+// TODO: This import line is here because it needs to be in the injected code, but it also would (ideally)
+// let us take advantage of typechecking, via the linter (both eslint and the TS linter), using intellisense, and when
+// building. Solving for all five simultaneously seems to be tricky, however, because of the circular dependency. This
+// is one of a number of possible compromise options, which seems to hit everything except eslint linting and
+// typechecking via `tsc`. (TS linting and intellisense both work, though, so we do get at least some type safety.) See
+// https://github.com/getsentry/sentry-javascript/pull/5503#discussion_r936827996 for more details.
+//
+// eslint-disable-next-line import/no-extraneous-dependencies, import/no-unresolved
+import * as ServerSideSentryNextjsSDK from '@sentry/nextjs';
+
+export const getServerSideProps =
+  typeof __ORIG_GSSP__ === 'function' ? ServerSideSentryNextjsSDK.withSentryGSSP(__ORIG_GSSP__) : __ORIG_GSSP__;
+export const getStaticProps =
+  typeof __ORIG_GSPROPS__ === 'function'
+    ? ServerSideSentryNextjsSDK.withSentryGSProps(__ORIG_GSPROPS__)
+    : __ORIG_GSPROPS__;
+export const getStaticPaths =
+  typeof __ORIG_GSPATHS__ === 'function'
+    ? ServerSideSentryNextjsSDK.withSentryGSPaths(__ORIG_GSPATHS__)
+    : __ORIG_GSPATHS__;

packages/nextjs/src/config/templates/dataFetchersLoaderTemplate.ts

@@ -49,6 +49,10 @@ export type UserSentryOptions = {
   // uploaded. At the same time, we don't want to widen the scope if we don't have to, because we're guaranteed to end
   // up uploading too many files, which is why this defaults to `false`.
   widenClientFileUpload?: boolean;
+
+  // Automatically wrap `getServerSideProps`, `getStaticProps`, and `getStaticPaths` in order to instrument them for
+  // tracing.
+  autoWrapDataFetchers?: boolean;
 };
 
 export type NextConfigFunction = (phase: string, defaults: { defaultConfig: NextConfigObject }) => NextConfigObject;