From 11721ab4768f90118aae6b28f8cb0b945098517f Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 10 Aug 2022 18:56:37 -0700 Subject: [PATCH 1/4] inject route as global variable --- .../src/config/loaders/dataFetchersLoader.ts | 17 ++++++++++++++++- .../templates/dataFetchersLoaderTemplate.ts | 2 ++ packages/nextjs/src/config/webpack.ts | 9 +++++---- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/config/loaders/dataFetchersLoader.ts b/packages/nextjs/src/config/loaders/dataFetchersLoader.ts index 9f42e86ab6e9..0d4c0d3becb6 100644 --- a/packages/nextjs/src/config/loaders/dataFetchersLoader.ts +++ b/packages/nextjs/src/config/loaders/dataFetchersLoader.ts @@ -37,6 +37,7 @@ const DATA_FETCHING_FUNCTIONS = { type LoaderOptions = { projectDir: string; + pagesDir: string; }; /** @@ -108,7 +109,7 @@ export default function wrapDataFetchersLoader(this: LoaderThis, } // We know one or the other will be defined, depending on the version of webpack being used - const { projectDir } = 'getOptions' in this ? this.getOptions() : this.query; + const { projectDir, pagesDir } = 'getOptions' in this ? this.getOptions() : this.query; // 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 @@ -172,6 +173,20 @@ export default function wrapDataFetchersLoader(this: LoaderThis, // 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); for (const { placeholder, alias } of Object.values(DATA_FETCHING_FUNCTIONS)) { injectedCode = injectedCode.replace(new RegExp(placeholder, 'g'), alias); } diff --git a/packages/nextjs/src/config/templates/dataFetchersLoaderTemplate.ts b/packages/nextjs/src/config/templates/dataFetchersLoaderTemplate.ts index 78788d6826ac..66a75c599148 100644 --- a/packages/nextjs/src/config/templates/dataFetchersLoaderTemplate.ts +++ b/packages/nextjs/src/config/templates/dataFetchersLoaderTemplate.ts @@ -17,6 +17,8 @@ declare const __ORIG_GSPROPS__: GetStaticPropsFunction; // eslint-disable-next-line import/no-extraneous-dependencies, import/no-unresolved import * as ServerSideSentryNextjsSDK from '@sentry/nextjs'; +const PARAMETERIZED_ROUTE = '__FILEPATH__'; + export const getServerSideProps = typeof __ORIG_GSSP__ === 'function' ? ServerSideSentryNextjsSDK.withSentryGetServerSideProps(__ORIG_GSSP__) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index a636609a9a7b..93b79a1c7313 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -56,8 +56,6 @@ export function constructWebpackConfigFunction( newConfig = userNextConfig.webpack(newConfig, buildContext); } - const pageRegex = new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages(/.+)\\.(jsx?|tsx?)`); - if (isServer) { newConfig.module = { ...newConfig.module, @@ -81,12 +79,15 @@ export function constructWebpackConfigFunction( }; if (userSentryOptions.experiments?.autoWrapDataFetchers) { + const pagesDir = newConfig.resolve?.alias?.['private-next-pages'] as string; + newConfig.module.rules.push({ - test: pageRegex, + // Nextjs allows the `pages` folder to optionally live inside a `src` folder + test: new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages/.*\\.(jsx?|tsx?)`), use: [ { loader: path.resolve(__dirname, 'loaders/dataFetchersLoader.js'), - options: { projectDir }, + options: { projectDir, pagesDir }, }, ], }); From e03682c2d70ab366f7792f4d2ac66cffb8a46756 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 16 Aug 2022 12:06:13 -0700 Subject: [PATCH 2/4] pass parameterized route to wrapper functions --- .../src/config/templates/dataFetchersLoaderTemplate.ts | 5 +++-- .../src/config/wrappers/withSentryGetServerSideProps.ts | 3 ++- .../nextjs/src/config/wrappers/withSentryGetStaticProps.ts | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/config/templates/dataFetchersLoaderTemplate.ts b/packages/nextjs/src/config/templates/dataFetchersLoaderTemplate.ts index 66a75c599148..4f9984f3f136 100644 --- a/packages/nextjs/src/config/templates/dataFetchersLoaderTemplate.ts +++ b/packages/nextjs/src/config/templates/dataFetchersLoaderTemplate.ts @@ -21,9 +21,10 @@ const PARAMETERIZED_ROUTE = '__FILEPATH__'; export const getServerSideProps = typeof __ORIG_GSSP__ === 'function' - ? ServerSideSentryNextjsSDK.withSentryGetServerSideProps(__ORIG_GSSP__) + ? ServerSideSentryNextjsSDK.withSentryGetServerSideProps(__ORIG_GSSP__, PARAMETERIZED_ROUTE) : __ORIG_GSSP__; + export const getStaticProps = typeof __ORIG_GSPROPS__ === 'function' - ? ServerSideSentryNextjsSDK.withSentryGetStaticProps(__ORIG_GSPROPS__) + ? ServerSideSentryNextjsSDK.withSentryGetStaticProps(__ORIG_GSPROPS__, PARAMETERIZED_ROUTE) : __ORIG_GSPROPS__; diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts index 0293ad999c45..bad402023ab3 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts @@ -5,9 +5,10 @@ import { callOriginal } from './wrapperUtils'; * Create a wrapped version of the user's exported `getServerSideProps` function * * @param origGetServerSideProps: The user's `getServerSideProps` function + * @param route: The page's parameterized route * @returns A wrapped version of the function */ -export function withSentryGetServerSideProps(origGetServerSideProps: GSSP['fn']): GSSP['wrappedFn'] { +export function withSentryGetServerSideProps(origGetServerSideProps: GSSP['fn'], route: string): GSSP['wrappedFn'] { return async function (context: GSSP['context']): Promise { return callOriginal(origGetServerSideProps, context); }; diff --git a/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts index 23d6023a25bf..88cda4890b86 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts @@ -5,9 +5,10 @@ import { callOriginal } from './wrapperUtils'; * Create a wrapped version of the user's exported `getStaticProps` function * * @param origGetStaticProps: The user's `getStaticProps` function + * @param route: The page's parameterized route * @returns A wrapped version of the function */ -export function withSentryGetStaticProps(origGetStaticProps: GSProps['fn']): GSProps['wrappedFn'] { +export function withSentryGetStaticProps(origGetStaticProps: GSProps['fn'], route: string): GSProps['wrappedFn'] { return async function (context: GSProps['context']): Promise { return callOriginal(origGetStaticProps, context); }; From 20abb1f1267bbb2afd1c4b730358739f67b2281a Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 16 Aug 2022 12:07:40 -0700 Subject: [PATCH 3/4] add span creation and transaction name replacement to wrapper core function --- .../wrappers/withSentryGetServerSideProps.ts | 4 +- .../wrappers/withSentryGetStaticProps.ts | 4 +- .../src/config/wrappers/wrapperUtils.ts | 42 ++++++++++++++----- 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts index bad402023ab3..23e7050fdac0 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts @@ -1,5 +1,5 @@ import { GSSP } from './types'; -import { callOriginal } from './wrapperUtils'; +import { wrapperCore } from './wrapperUtils'; /** * Create a wrapped version of the user's exported `getServerSideProps` function @@ -10,6 +10,6 @@ import { callOriginal } from './wrapperUtils'; */ export function withSentryGetServerSideProps(origGetServerSideProps: GSSP['fn'], route: string): GSSP['wrappedFn'] { return async function (context: GSSP['context']): Promise { - return callOriginal(origGetServerSideProps, context); + return wrapperCore(origGetServerSideProps, context, route); }; } diff --git a/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts index 88cda4890b86..00c286b1b500 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts @@ -1,5 +1,5 @@ import { GSProps } from './types'; -import { callOriginal } from './wrapperUtils'; +import { wrapperCore } from './wrapperUtils'; /** * Create a wrapped version of the user's exported `getStaticProps` function @@ -10,6 +10,6 @@ import { callOriginal } from './wrapperUtils'; */ export function withSentryGetStaticProps(origGetStaticProps: GSProps['fn'], route: string): GSProps['wrappedFn'] { return async function (context: GSProps['context']): Promise { - return callOriginal(origGetStaticProps, context); + return wrapperCore(origGetStaticProps, context, route); }; } diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index d7a466cb8221..a66b4ee7c452 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -1,25 +1,47 @@ +import { getActiveTransaction } from '@sentry/tracing'; + import { DataFetchingFunction } from './types'; /** - * Pass-through wrapper for the original function, used as a first step in eventually wrapping the data-fetching - * functions with code for tracing. + * Create a span to track the wrapped function and update transaction name with parameterized route. * * @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 + * @param route The route currently being served * @returns The result of calling the user's function */ -export async function callOriginal( +export async function wrapperCore( origFunction: T['fn'], context: T['context'], + route: string, ): Promise { - let props; + const transaction = getActiveTransaction(); + + if (transaction) { + // Pull off any leading underscores we've added in the process of wrapping the function + const wrappedFunctionName = origFunction.name.replace(/^_*/, ''); + + // 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: 'nextjs.data', description: `${wrappedFunctionName} (${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(); - // 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 - props = await (origFunction as any)(context); + return props; + } - return props; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (origFunction as any)(context); } From 84cd8376c1b1bf8a8ef3b99515243eb9643f9c5a Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 16 Aug 2022 12:57:38 -0700 Subject: [PATCH 4/4] add basic error handling --- .../src/config/wrappers/wrapperUtils.ts | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index a66b4ee7c452..feca9cf4adb7 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -1,4 +1,6 @@ +import { captureException } from '@sentry/core'; import { getActiveTransaction } from '@sentry/tracing'; +import { Span } from '@sentry/types'; import { DataFetchingFunction } from './types'; @@ -31,11 +33,7 @@ export async function wrapperCore( // route's transaction const span = transaction.startChild({ op: 'nextjs.data', description: `${wrappedFunctionName} (${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); + const props = await callOriginal(origFunction, context, span); span.finish(); @@ -43,5 +41,25 @@ export async function wrapperCore( } // eslint-disable-next-line @typescript-eslint/no-explicit-any - return (origFunction as any)(context); + return callOriginal(origFunction, context); +} + +/** Call the original function, capturing any errors and finishing the span (if any) in case of error */ +async function callOriginal( + origFunction: T['fn'], + context: T['context'], + span?: Span, +): Promise { + try { + // eslint-disable-next-line prefer-const, @typescript-eslint/no-explicit-any + return (origFunction as any)(context); + } catch (err) { + if (span) { + span.finish(); + } + + // TODO Copy more robust error handling over from `withSentry` + captureException(err); + throw err; + } }