From 2f6deeb183a6b1049898111547e20c5d5a14c8f6 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 17 Aug 2022 12:21:36 +0200 Subject: [PATCH] feat(nextjs): Create spans and route parameterization in server-side `getInitialProps` (#5587) --- .../src/config/loaders/dataFetchersLoader.ts | 58 +++++++++------ packages/nextjs/src/config/wrappers/types.ts | 35 --------- .../wrappers/withSentryGetInitialProps.ts | 24 ++++-- .../wrappers/withSentryGetServerSideProps.ts | 23 ++++-- .../wrappers/withSentryGetStaticProps.ts | 25 +++++-- .../src/config/wrappers/wrapperUtils.ts | 73 ++++++++----------- 6 files changed, 119 insertions(+), 119 deletions(-) delete mode 100644 packages/nextjs/src/config/wrappers/types.ts diff --git a/packages/nextjs/src/config/loaders/dataFetchersLoader.ts b/packages/nextjs/src/config/loaders/dataFetchersLoader.ts index 0d4c0d3becb6..d1608cbb8415 100644 --- a/packages/nextjs/src/config/loaders/dataFetchersLoader.ts +++ b/packages/nextjs/src/config/loaders/dataFetchersLoader.ts @@ -111,6 +111,21 @@ 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, pagesDir } = '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 // string. @@ -136,13 +151,26 @@ export default function wrapDataFetchersLoader(this: LoaderThis, 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 (parameterizedRouteName === '/_app') { + // getInitialProps signature is a bit different in _app.js so we need a different wrapper + // Currently a no-op + } else if (parameterizedRouteName === '/_error') { + // getInitialProps behaviour is a bit different in _error.js so we probably want different wrapper + // Currently a no-op + } else if (parameterizedRouteName === '/_document') { + // 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 +201,8 @@ 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); + + injectedCode = injectedCode.replace('__FILEPATH__', parameterizedRouteName); 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/wrappers/types.ts b/packages/nextjs/src/config/wrappers/types.ts deleted file mode 100644 index 85b5502703ca..000000000000 --- a/packages/nextjs/src/config/wrappers/types.ts +++ /dev/null @@ -1,35 +0,0 @@ -import type { - GetServerSideProps, - GetServerSidePropsContext, - GetServerSidePropsResult, - GetStaticProps, - GetStaticPropsContext, - GetStaticPropsResult, - NextPage, - NextPageContext, -} from 'next'; - -type Props = { [key: string]: unknown }; - -export type GSProps = { - fn: GetStaticProps; - wrappedFn: GetStaticProps; - context: GetStaticPropsContext; - result: GetStaticPropsResult; -}; - -export type GSSP = { - fn: GetServerSideProps; - wrappedFn: GetServerSideProps; - context: GetServerSidePropsContext; - result: GetServerSidePropsResult; -}; - -export type GIProps = { - fn: Required['getInitialProps']; - wrappedFn: NextPage['getInitialProps']; - context: NextPageContext; - result: unknown; -}; - -export type DataFetchingFunction = GSProps | GSSP | GIProps; diff --git a/packages/nextjs/src/config/wrappers/withSentryGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetInitialProps.ts index 7e5e209ae98a..9abf2c7a7f0a 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetInitialProps.ts @@ -1,14 +1,26 @@ -import { GIProps } from './types'; +import { NextPage } from 'next'; + +import { callDataFetcherTraced } from './wrapperUtils'; + +type GetInitialProps = Required>['getInitialProps']; /** * Create a wrapped version of the user's exported `getInitialProps` function * - * @param origGIProps: The user's `getInitialProps` function - * @param origGIPropsHost: The user's object on which `getInitialProps` lives (used for `this`) + * @param origGetInitialProps The user's `getInitialProps` function + * @param parameterizedRoute The page's parameterized route * @returns A wrapped version of the function */ -export function withSentryGetInitialProps(origGIProps: GIProps['fn']): GIProps['wrappedFn'] { - return async function (this: unknown, ...args: Parameters) { - return await origGIProps.call(this, ...args); +export function withSentryGetInitialProps( + origGetInitialProps: GetInitialProps, + parameterizedRoute: string, +): GetInitialProps { + return async function ( + ...getInitialPropsArguments: Parameters + ): Promise> { + return callDataFetcherTraced(origGetInitialProps, getInitialPropsArguments, { + parameterizedRoute, + dataFetchingMethodName: 'getInitialProps', + }); }; } diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts index 23e7050fdac0..dce840606e39 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts @@ -1,15 +1,24 @@ -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 * - * @param origGetServerSideProps: The user's `getServerSideProps` function - * @param route: The page's parameterized route + * @param origGetServerSideProps The user's `getServerSideProps` function + * @param parameterizedRoute 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 { - return wrapperCore(origGetServerSideProps, context, route); +export function withSentryGetServerSideProps( + origGetServerSideProps: GetServerSideProps, + parameterizedRoute: string, +): GetServerSideProps { + return async function ( + ...getServerSidePropsArguments: Parameters + ): ReturnType { + return callDataFetcherTraced(origGetServerSideProps, getServerSidePropsArguments, { + parameterizedRoute, + dataFetchingMethodName: 'getServerSideProps', + }); }; } diff --git a/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts index 00c286b1b500..0d2cc5e161dd 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts @@ -1,15 +1,26 @@ -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 * - * @param origGetStaticProps: The user's `getStaticProps` function - * @param route: The page's parameterized route + * @param origGetStaticProps The user's `getStaticProps` function + * @param parameterizedRoute 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 { - return wrapperCore(origGetStaticProps, context, route); +export function withSentryGetStaticProps( + origGetStaticProps: GetStaticProps, + parameterizedRoute: string, +): GetStaticProps { + return async function ( + ...getStaticPropsArguments: Parameters> + ): ReturnType> { + return callDataFetcherTraced(origGetStaticProps, getStaticPropsArguments, { + parameterizedRoute, + dataFetchingMethodName: 'getStaticProps', + }); }; } diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index feca9cf4adb7..ab216f34aeaf 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -1,58 +1,45 @@ import { captureException } from '@sentry/core'; import { getActiveTransaction } from '@sentry/tracing'; -import { Span } from '@sentry/types'; - -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 - * @param route The route currently being served - * @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( - origFunction: T['fn'], - context: T['context'], - route: string, -): Promise { - 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'; +export async function callDataFetcherTraced Promise | any>( + origFunction: F, + origFunctionArgs: Parameters, + options: { + parameterizedRoute: string; + dataFetchingMethodName: string; + }, +): Promise> { + const { parameterizedRoute, dataFetchingMethodName } = options; - // 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})` }); - - const props = await callOriginal(origFunction, context, span); - - span.finish(); + const transaction = getActiveTransaction(); - return props; + if (!transaction) { + return origFunction(...origFunctionArgs); } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return callOriginal(origFunction, 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. + transaction.name = parameterizedRoute; + 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: `${dataFetchingMethodName} (${parameterizedRoute})`, + }); -/** 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); + return await origFunction(...origFunctionArgs); } catch (err) { if (span) { span.finish();