feat(nextjs): Create spans in serverside getInitialProps

Author: Luca Forstner

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

@@ -38,6 +38,9 @@ const DATA_FETCHING_FUNCTIONS = {
 type LoaderOptions = {
   projectDir: string;
   pagesDir: string;
+  underscoreAppRegex: RegExp;
+  underscoreErrorRegex: RegExp;
+  underscoreDocumentRegex: RegExp;
 };
 
 /**
@@ -109,7 +112,23 @@ export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>,
   }
 
   // We know one or the other will be defined, depending on the version of webpack being used
-  const { projectDir, pagesDir } = 'getOptions' in this ? this.getOptions() : this.query;
+  const { projectDir, pagesDir, underscoreAppRegex, underscoreDocumentRegex, underscoreErrorRegex } =
+    'getOptions' in this ? this.getOptions() : this.query;
+
+  // Get the parameterized route name from this page's filepath
+  const parameterizedRouteName = path
+    // Get the path of the file insde of the pages directory
+    .relative(pagesDir, this.resourcePath)
+    // Add a slash at the beginning
+    .replace(/(.*)/, '/$1')
+    // Pull off the file extension
+    .replace(/\.(jsx?|tsx?)/, '')
+    // Any page file named `index` corresponds to root of the directory its in, URL-wise, so turn `/xyz/index` into
+    // just `/xyz`
+    .replace(/\/index$/, '')
+    // In case all of the above have left us with an empty string (which will happen if we're dealing with the
+    // homepage), sub back in the root route
+    .replace(/^$/, '/');
 
   // In the following branch we will proxy the user's file. This means we return code (basically an entirely new file)
   // that re - exports all the user file's originial export, but with a "sentry-proxy-loader" query in the module
@@ -136,13 +155,26 @@ export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>,
     if (hasDefaultExport(ast)) {
       outputFileContent += `
         import { default as _sentry_default } from "${this.resourcePath}?sentry-proxy-loader";
-        import { withSentryGetInitialProps } from "@sentry/nextjs";
-
-        if (typeof _sentry_default.getInitialProps === 'function') {
-          _sentry_default.getInitialProps = withSentryGetInitialProps(_sentry_default.getInitialProps);
-        }
-
-        export default _sentry_default;`;
+        import { withSentryGetInitialProps } from "@sentry/nextjs";`;
+
+      if (this.resourcePath.match(underscoreAppRegex)) {
+        // getInitialProps signature is a bit different in _app.js so we need a different wrapper
+        // Currently a no-op
+      } else if (this.resourcePath.match(underscoreErrorRegex)) {
+        // getInitialProps behaviour is a bit different in _error.js so we probably want different wrapper
+        // Currently a no-op
+      } else if (this.resourcePath.match(underscoreDocumentRegex)) {
+        // getInitialProps signature is a bit different in _document.js so we need a different wrapper
+        // Currently a no-op
+      } else {
+        // We enter this branch for any "normal" Next.js page
+        outputFileContent += `
+          if (typeof _sentry_default.getInitialProps === 'function') {
+            _sentry_default.getInitialProps = withSentryGetInitialProps(_sentry_default.getInitialProps, '${parameterizedRouteName}');
+          }`;
+      }
+
+      outputFileContent += 'export default _sentry_default;';
     }
 
     return outputFileContent;
@@ -173,20 +205,8 @@ export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>,
 
     // Fill in template placeholders
     let injectedCode = modifiedTemplateCode;
-    const route = path
-      // Get the path of the file insde of the pages directory
-      .relative(pagesDir, this.resourcePath)
-      // Add a slash at the beginning
-      .replace(/(.*)/, '/$1')
-      // Pull off the file extension
-      .replace(/\.(jsx?|tsx?)/, '')
-      // Any page file named `index` corresponds to root of the directory its in, URL-wise, so turn `/xyz/index` into
-      // just `/xyz`
-      .replace(/\/index$/, '')
-      // In case all of the above have left us with an empty string (which will happen if we're dealing with the
-      // homepage), sub back in the root route
-      .replace(/^$/, '/');
-    injectedCode = injectedCode.replace('__FILEPATH__', route);
+
+    injectedCode = injectedCode.replace('__FILEPATH__', parameterizedRouteName);
     for (const { placeholder, alias } of Object.values(DATA_FETCHING_FUNCTIONS)) {
       injectedCode = injectedCode.replace(new RegExp(placeholder, 'g'), alias);
     }

packages/nextjs/src/config/webpack.ts

@@ -78,6 +78,12 @@ export function constructWebpackConfigFunction(
         ],
       };
 
+      const underscoreAppRegex = new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages/_app\\.(jsx?|tsx?)`);
+      const underscoreErrorRegex = new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages/_error\\.(jsx?|tsx?)`);
+      const underscoreDocumentRegex = new RegExp(
+        `${escapeStringForRegex(projectDir)}(/src)?/pages/_document\\.(jsx?|tsx?)`,
+      );
+
       if (userSentryOptions.experiments?.autoWrapDataFetchers) {
         const pagesDir = newConfig.resolve?.alias?.['private-next-pages'] as string;
 
@@ -87,7 +93,13 @@ export function constructWebpackConfigFunction(
           use: [
             {
               loader: path.resolve(__dirname, 'loaders/dataFetchersLoader.js'),
-              options: { projectDir, pagesDir },
+              options: {
+                projectDir,
+                pagesDir,
+                underscoreAppRegex,
+                underscoreErrorRegex,
+                underscoreDocumentRegex,
+              },
             },
           ],
         });

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

@@ -1,14 +1,18 @@
-import { GIProps } from './types';
+import { NextPage } from 'next';
+
+import { callDataFetcherTraced } from './wrapperUtils';
+
+type GetInitialProps = Required<NextPage<unknown>>['getInitialProps'];
 
 /**
  * Create a wrapped version of the user's exported `getInitialProps` function
  *
- * @param origGIProps: The user's `getInitialProps` function
+ * @param origGetInitialProps: The user's `getInitialProps` function
  * @param origGIPropsHost: The user's object on which `getInitialProps` lives (used for `this`)
  * @returns A wrapped version of the function
  */
-export function withSentryGetInitialProps(origGIProps: GIProps['fn']): GIProps['wrappedFn'] {
-  return async function (this: unknown, ...args: Parameters<GIProps['fn']>) {
-    return await origGIProps.call(this, ...args);
+export function withSentryGetInitialProps(origGetInitialProps: GetInitialProps, route: string): GetInitialProps {
+  return function (...getInitialPropsArguments: Parameters<GetInitialProps>): ReturnType<GetInitialProps> {
+    return callDataFetcherTraced(origGetInitialProps, getInitialPropsArguments, { route, op: 'getInitialProps' });
   };
 }

packages/nextjs/src/config/wrappers/withSentryGetInitialProps.ts

@@ -1,5 +1,6 @@
-import { GSSP } from './types';
-import { wrapperCore } from './wrapperUtils';
+import { GetServerSideProps } from 'next';
+
+import { callDataFetcherTraced } from './wrapperUtils';
 
 /**
  * Create a wrapped version of the user's exported `getServerSideProps` function
@@ -8,8 +9,14 @@ import { wrapperCore } from './wrapperUtils';
  * @param route: The page's parameterized route
  * @returns A wrapped version of the function
  */
-export function withSentryGetServerSideProps(origGetServerSideProps: GSSP['fn'], route: string): GSSP['wrappedFn'] {
-  return async function (context: GSSP['context']): Promise<GSSP['result']> {
-    return wrapperCore<GSSP>({ origFunction: origGetServerSideProps, context, route, op: 'getServerSideProps' });
+export function withSentryGetServerSideProps(
+  origGetServerSideProps: GetServerSideProps,
+  route: string,
+): GetServerSideProps {
+  return function (...getServerSidePropsArguments: Parameters<GetServerSideProps>): ReturnType<GetServerSideProps> {
+    return callDataFetcherTraced(origGetServerSideProps, getServerSidePropsArguments, {
+      route,
+      op: 'getServerSideProps',
+    });
   };
 }

packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts

@@ -1,5 +1,8 @@
-import { GSProps } from './types';
-import { wrapperCore } from './wrapperUtils';
+import { GetStaticProps } from 'next';
+
+import { callDataFetcherTraced } from './wrapperUtils';
+
+type Props = { [key: string]: unknown };
 
 /**
  * Create a wrapped version of the user's exported `getStaticProps` function
@@ -8,8 +11,11 @@ import { wrapperCore } from './wrapperUtils';
  * @param route: The page's parameterized route
  * @returns A wrapped version of the function
  */
-export function withSentryGetStaticProps(origGetStaticProps: GSProps['fn'], route: string): GSProps['wrappedFn'] {
-  return async function (context: GSProps['context']): Promise<GSProps['result']> {
-    return wrapperCore<GSProps>({ origFunction: origGetStaticProps, context, route, op: 'getStaticProps' });
+export function withSentryGetStaticProps(
+  origGetStaticProps: GetStaticProps<Props>,
+  route: string,
+): GetStaticProps<Props> {
+  return function (...getStaticPropsArguments: Parameters<GetStaticProps<Props>>): ReturnType<GetStaticProps<Props>> {
+    return callDataFetcherTraced(origGetStaticProps, getStaticPropsArguments, { route, op: 'getStaticProps' });
   };
 }

packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts

@@ -1,46 +1,58 @@
 import { getActiveTransaction } from '@sentry/tracing';
 
-import { DataFetchingFunction } from './types';
-
 /**
- * Create a span to track the wrapped function and update transaction name with parameterized route.
+ * Call a data fetcher and trace it. Only traces the function if there is an active transaction on the scope.
  *
- * @template T Types for `getInitialProps`, `getStaticProps`, and `getServerSideProps`
- * @param origFunction The user's exported `getInitialProps`, `getStaticProps`, or `getServerSideProps` function
- * @param context The context object passed by nextjs to the function
- * @returns The result of calling the user's function
+ * We only do the following until we move transaction creation into this function: When called, the wrapped function
+ * will also update the name of the active transaction with a parameterized route provided via the `options` argument.
  */
-export async function wrapperCore<T extends DataFetchingFunction>(options: {
-  origFunction: T['fn'];
-  context: T['context'];
-  route: string;
-  op: string;
-}): Promise<T['result']> {
-  const { origFunction, context, route, op } = options;
+export function callDataFetcherTraced<F extends (...args: any[]) => Promise<any> | any>(
+  origFunction: F,
+  origFunctionArgs: Parameters<F>,
+  options: {
+    route: string;
+    op: string;
+  },
+): ReturnType<F> {
+  const { route, op } = options;
 
   const transaction = getActiveTransaction();
 
-  if (transaction) {
-    // TODO: Make sure that the given route matches the name of the active transaction (to prevent background data
-    // fetching from switching the name to a completely other route)
-    transaction.name = route;
-    transaction.metadata.source = 'route';
-
-    // Capture the route, since pre-loading, revalidation, etc might mean that this span may happen during another
-    // route's transaction
-    const span = transaction.startChild({ op, data: { route } });
-
-    // TODO: Can't figure out how to tell TS that the types are correlated - that a `GSPropsFunction` will only get passed
-    // `GSPropsContext` and never, say, `GSSPContext`. That's what wrapping everything in objects and using the generic
-    // and pulling the types from the generic rather than specifying them directly was supposed to do, but... no luck.
-    // eslint-disable-next-line prefer-const, @typescript-eslint/no-explicit-any
-    const props = await (origFunction as any)(context);
-
-    span.finish();
-
-    return props;
+  if (!transaction) {
+    return origFunction(...origFunctionArgs);
   }
 
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  return (origFunction as any)(context);
+  // TODO: Make sure that the given route matches the name of the active transaction (to prevent background data
+  // fetching from switching the name to a completely other route) -- We'll probably switch to creating a transaction
+  // right here so making that check will probabably not even be necessary.
+  // Logic will be: If there is no active transaction, start one with correct name and source. If there is an active
+  // transaction, create a child span with correct name and source.
+  // We will probably need to put
+  transaction.name = route;
+  transaction.metadata.source = 'route';
+
+  // Capture the route, since pre-loading, revalidation, etc might mean that this span may happen during another
+  // route's transaction
+  const span = transaction.startChild({ op, data: { route } });
+
+  const result = origFunction(...origFunctionArgs);
+
+  // We do the following instead of `await`-ing the return value of `origFunction`, because that would require us to
+  // make this function async which might in turn create a mismatch of function signatures between the original
+  // function and the wrapped one.
+  // This wraps `result`, which is potentially a Promise, into a Promise.
+  // If `result` is a non-Promise, the callback of `then` is immediately called and the span is finished.
+  // If `result` is a Promise, the callback of `then` is only called when `result` resolves
+  void Promise.resolve(result).then(
+    () => {
+      span.finish();
+    },
+    err => {
+      // TODO: Can we somehow associate the error with the span?
+      span.finish();
+      throw err;
+    },
+  );
+
+  return result;
 }