Skip to content

Commit e861cc4

Browse files
authored
feat(nextjs): Add spans and route parameterization in data fetching wrappers (#5564)
This adds span creation and route parameterization to the experimental `getStaticProps` and `getServerSideProps` wrappers. In order to do the parameterization, we store the parameterized route( which is the same as the filepath of the page file) as a global variable in the code we add using the `dataFetchersLoader` and then pass it to the wrappers, which then replace the name of the active transaction with the parameterized route we've stored. It also adds basic error-capturing to the wrappers. Open issues/questions (not solved by this PR): - Because of prefetching, revalidation of static pages, and the like, the data fetching functions can run in the background, during handling of a route different from their own. (For example, if page A has a nextjs `Link` component pointing to page B, next will prefetch the data for B (and therefore run its data fetching functions) while handling the request for A. We need to make sure that we don't accidentally overwrite the transaction name in that case to B. - There's more we should do in terms of error handling. Specifically, we should port much of the logic in `withSentry` into these wrappers also. - Porting the error-handling logic will mean we need to deal with the domain question - how to keep reactivating the correct domain as we go into and out of data fetching and rendering functions. (There are other items listed in the meta issue linked below, but the above are the ones directly relevant to the work in this PR.) Ref: #5505
1 parent c2cdf92 commit e861cc4

File tree

6 files changed

+84
-23
lines changed

6 files changed

+84
-23
lines changed

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

+16-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const DATA_FETCHING_FUNCTIONS = {
3737

3838
type LoaderOptions = {
3939
projectDir: string;
40+
pagesDir: string;
4041
};
4142

4243
/**
@@ -108,7 +109,7 @@ export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>,
108109
}
109110

110111
// We know one or the other will be defined, depending on the version of webpack being used
111-
const { projectDir } = 'getOptions' in this ? this.getOptions() : this.query;
112+
const { projectDir, pagesDir } = 'getOptions' in this ? this.getOptions() : this.query;
112113

113114
// In the following branch we will proxy the user's file. This means we return code (basically an entirely new file)
114115
// 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<LoaderOptions>,
172173

173174
// Fill in template placeholders
174175
let injectedCode = modifiedTemplateCode;
176+
const route = path
177+
// Get the path of the file insde of the pages directory
178+
.relative(pagesDir, this.resourcePath)
179+
// Add a slash at the beginning
180+
.replace(/(.*)/, '/$1')
181+
// Pull off the file extension
182+
.replace(/\.(jsx?|tsx?)/, '')
183+
// Any page file named `index` corresponds to root of the directory its in, URL-wise, so turn `/xyz/index` into
184+
// just `/xyz`
185+
.replace(/\/index$/, '')
186+
// In case all of the above have left us with an empty string (which will happen if we're dealing with the
187+
// homepage), sub back in the root route
188+
.replace(/^$/, '/');
189+
injectedCode = injectedCode.replace('__FILEPATH__', route);
175190
for (const { placeholder, alias } of Object.values(DATA_FETCHING_FUNCTIONS)) {
176191
injectedCode = injectedCode.replace(new RegExp(placeholder, 'g'), alias);
177192
}

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,14 @@ declare const __ORIG_GSPROPS__: GetStaticPropsFunction;
1717
// eslint-disable-next-line import/no-extraneous-dependencies, import/no-unresolved
1818
import * as ServerSideSentryNextjsSDK from '@sentry/nextjs';
1919

20+
const PARAMETERIZED_ROUTE = '__FILEPATH__';
21+
2022
export const getServerSideProps =
2123
typeof __ORIG_GSSP__ === 'function'
22-
? ServerSideSentryNextjsSDK.withSentryGetServerSideProps(__ORIG_GSSP__)
24+
? ServerSideSentryNextjsSDK.withSentryGetServerSideProps(__ORIG_GSSP__, PARAMETERIZED_ROUTE)
2325
: __ORIG_GSSP__;
26+
2427
export const getStaticProps =
2528
typeof __ORIG_GSPROPS__ === 'function'
26-
? ServerSideSentryNextjsSDK.withSentryGetStaticProps(__ORIG_GSPROPS__)
29+
? ServerSideSentryNextjsSDK.withSentryGetStaticProps(__ORIG_GSPROPS__, PARAMETERIZED_ROUTE)
2730
: __ORIG_GSPROPS__;

packages/nextjs/src/config/webpack.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@ export function constructWebpackConfigFunction(
5656
newConfig = userNextConfig.webpack(newConfig, buildContext);
5757
}
5858

59-
const pageRegex = new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages(/.+)\\.(jsx?|tsx?)`);
60-
6159
if (isServer) {
6260
newConfig.module = {
6361
...newConfig.module,
@@ -81,12 +79,15 @@ export function constructWebpackConfigFunction(
8179
};
8280

8381
if (userSentryOptions.experiments?.autoWrapDataFetchers) {
82+
const pagesDir = newConfig.resolve?.alias?.['private-next-pages'] as string;
83+
8484
newConfig.module.rules.push({
85-
test: pageRegex,
85+
// Nextjs allows the `pages` folder to optionally live inside a `src` folder
86+
test: new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages/.*\\.(jsx?|tsx?)`),
8687
use: [
8788
{
8889
loader: path.resolve(__dirname, 'loaders/dataFetchersLoader.js'),
89-
options: { projectDir },
90+
options: { projectDir, pagesDir },
9091
},
9192
],
9293
});
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { GSSP } from './types';
2-
import { callOriginal } from './wrapperUtils';
2+
import { wrapperCore } from './wrapperUtils';
33

44
/**
55
* Create a wrapped version of the user's exported `getServerSideProps` function
66
*
77
* @param origGetServerSideProps: The user's `getServerSideProps` function
8+
* @param route: The page's parameterized route
89
* @returns A wrapped version of the function
910
*/
10-
export function withSentryGetServerSideProps(origGetServerSideProps: GSSP['fn']): GSSP['wrappedFn'] {
11+
export function withSentryGetServerSideProps(origGetServerSideProps: GSSP['fn'], route: string): GSSP['wrappedFn'] {
1112
return async function (context: GSSP['context']): Promise<GSSP['result']> {
12-
return callOriginal<GSSP>(origGetServerSideProps, context);
13+
return wrapperCore<GSSP>(origGetServerSideProps, context, route);
1314
};
1415
}
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { GSProps } from './types';
2-
import { callOriginal } from './wrapperUtils';
2+
import { wrapperCore } from './wrapperUtils';
33

44
/**
55
* Create a wrapped version of the user's exported `getStaticProps` function
66
*
77
* @param origGetStaticProps: The user's `getStaticProps` function
8+
* @param route: The page's parameterized route
89
* @returns A wrapped version of the function
910
*/
10-
export function withSentryGetStaticProps(origGetStaticProps: GSProps['fn']): GSProps['wrappedFn'] {
11+
export function withSentryGetStaticProps(origGetStaticProps: GSProps['fn'], route: string): GSProps['wrappedFn'] {
1112
return async function (context: GSProps['context']): Promise<GSProps['result']> {
12-
return callOriginal<GSProps>(origGetStaticProps, context);
13+
return wrapperCore<GSProps>(origGetStaticProps, context, route);
1314
};
1415
}
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,65 @@
1+
import { captureException } from '@sentry/core';
2+
import { getActiveTransaction } from '@sentry/tracing';
3+
import { Span } from '@sentry/types';
4+
15
import { DataFetchingFunction } from './types';
26

37
/**
4-
* Pass-through wrapper for the original function, used as a first step in eventually wrapping the data-fetching
5-
* functions with code for tracing.
8+
* Create a span to track the wrapped function and update transaction name with parameterized route.
69
*
710
* @template T Types for `getInitialProps`, `getStaticProps`, and `getServerSideProps`
811
* @param origFunction The user's exported `getInitialProps`, `getStaticProps`, or `getServerSideProps` function
912
* @param context The context object passed by nextjs to the function
13+
* @param route The route currently being served
1014
* @returns The result of calling the user's function
1115
*/
12-
export async function callOriginal<T extends DataFetchingFunction>(
16+
export async function wrapperCore<T extends DataFetchingFunction>(
1317
origFunction: T['fn'],
1418
context: T['context'],
19+
route: string,
1520
): Promise<T['result']> {
16-
let props;
21+
const transaction = getActiveTransaction();
22+
23+
if (transaction) {
24+
// Pull off any leading underscores we've added in the process of wrapping the function
25+
const wrappedFunctionName = origFunction.name.replace(/^_*/, '');
26+
27+
// TODO: Make sure that the given route matches the name of the active transaction (to prevent background data
28+
// fetching from switching the name to a completely other route)
29+
transaction.name = route;
30+
transaction.metadata.source = 'route';
31+
32+
// Capture the route, since pre-loading, revalidation, etc might mean that this span may happen during another
33+
// route's transaction
34+
const span = transaction.startChild({ op: 'nextjs.data', description: `${wrappedFunctionName} (${route})` });
35+
36+
const props = await callOriginal(origFunction, context, span);
37+
38+
span.finish();
1739

18-
// TODO: Can't figure out how to tell TS that the types are correlated - that a `GSPropsFunction` will only get passed
19-
// `GSPropsContext` and never, say, `GSSPContext`. That's what wrapping everything in objects and using the generic
20-
// and pulling the types from the generic rather than specifying them directly was supposed to do, but... no luck.
21-
// eslint-disable-next-line prefer-const, @typescript-eslint/no-explicit-any
22-
props = await (origFunction as any)(context);
40+
return props;
41+
}
42+
43+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
44+
return callOriginal(origFunction, context);
45+
}
46+
47+
/** Call the original function, capturing any errors and finishing the span (if any) in case of error */
48+
async function callOriginal<T extends DataFetchingFunction>(
49+
origFunction: T['fn'],
50+
context: T['context'],
51+
span?: Span,
52+
): Promise<T['result']> {
53+
try {
54+
// eslint-disable-next-line prefer-const, @typescript-eslint/no-explicit-any
55+
return (origFunction as any)(context);
56+
} catch (err) {
57+
if (span) {
58+
span.finish();
59+
}
2360

24-
return props;
61+
// TODO Copy more robust error handling over from `withSentry`
62+
captureException(err);
63+
throw err;
64+
}
2565
}

0 commit comments

Comments
 (0)