Skip to content

Commit c7550dd

Browse files
authored
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
1 parent 36c938c commit c7550dd

22 files changed

+1208
-17
lines changed

packages/nextjs/package.json

+4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"access": "public"
1818
},
1919
"dependencies": {
20+
"@babel/parser": "^7.18.10",
2021
"@sentry/core": "7.9.0",
2122
"@sentry/hub": "7.9.0",
2223
"@sentry/integrations": "7.9.0",
@@ -26,9 +27,12 @@
2627
"@sentry/types": "7.9.0",
2728
"@sentry/utils": "7.9.0",
2829
"@sentry/webpack-plugin": "1.19.0",
30+
"jscodeshift": "^0.13.1",
2931
"tslib": "^1.9.3"
3032
},
3133
"devDependencies": {
34+
"@babel/types": "7.18.10",
35+
"@types/jscodeshift": "^0.11.5",
3236
"@types/webpack": "^4.41.31",
3337
"next": "10.1.3"
3438
},

packages/nextjs/rollup.npm.config.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -14,32 +14,34 @@ export default [
1414
),
1515
...makeNPMConfigVariants(
1616
makeBaseNPMConfig({
17-
entrypoints: ['src/config/templates/prefixLoaderTemplate.ts'],
17+
entrypoints: [
18+
'src/config/templates/prefixLoaderTemplate.ts',
19+
'src/config/templates/dataFetchersLoaderTemplate.ts',
20+
],
1821

1922
packageSpecificConfig: {
2023
output: {
21-
// preserve the original file structure (i.e., so that everything is still relative to `src`)
24+
// Preserve the original file structure (i.e., so that everything is still relative to `src`). (Not entirely
25+
// clear why this is necessary here and not for other entrypoints in this file.)
2226
entryFileNames: 'config/templates/[name].js',
2327

2428
// this is going to be add-on code, so it doesn't need the trappings of a full module (and in fact actively
2529
// shouldn't have them, lest they muck with the module to which we're adding it)
2630
sourcemap: false,
2731
esModule: false,
2832
},
33+
external: ['@sentry/nextjs'],
2934
},
3035
}),
3136
),
3237
...makeNPMConfigVariants(
3338
makeBaseNPMConfig({
34-
entrypoints: ['src/config/loaders/prefixLoader.ts'],
39+
entrypoints: ['src/config/loaders/index.ts'],
3540

3641
packageSpecificConfig: {
3742
output: {
3843
// make it so Rollup calms down about the fact that we're doing `export { loader as default }`
39-
exports: 'default',
40-
41-
// preserve the original file structure (i.e., so that everything is still relative to `src`)
42-
entryFileNames: 'config/loaders/[name].js',
44+
exports: 'named',
4345
},
4446
},
4547
}),

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

+322
Large diffs are not rendered by default.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/**
2+
* This loader auto-wraps a user's page-level data-fetching functions (`getStaticPaths`, `getStaticProps`, and
3+
* `getServerSideProps`) in order to instrument them for tracing. At a high level, this is done by finding the relevant
4+
* functions, renaming them so as not to create a name collision, and then creating a new version of each function which
5+
* is a wrapped version of the original. We do this by parsing the user's code and some template code into ASTs,
6+
* manipulating them, and then turning them back into strings and appending our template code to the user's (modified)
7+
* page code. Greater detail and explanations can be found in situ in the functions below and in the helper functions in
8+
* `ast.ts`.
9+
*/
10+
11+
import { logger } from '@sentry/utils';
12+
import * as fs from 'fs';
13+
import * as path from 'path';
14+
15+
import { isESM } from '../../utils/isESM';
16+
import type { AST } from './ast';
17+
import { findDeclarations, findExports, makeAST, removeComments, renameIdentifiers } from './ast';
18+
import type { LoaderThis } from './types';
19+
20+
// Map to keep track of each function's placeholder in the template and what it should be replaced with. (The latter
21+
// 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
22+
// to a non-empty string later.)
23+
const DATA_FETCHING_FUNCTIONS = {
24+
getServerSideProps: { placeholder: '__ORIG_GSSP__', alias: '' },
25+
getStaticProps: { placeholder: '__ORIG_GSPROPS__', alias: '' },
26+
getStaticPaths: { placeholder: '__ORIG_GSPATHS__', alias: '' },
27+
};
28+
29+
type LoaderOptions = {
30+
projectDir: string;
31+
};
32+
33+
/**
34+
* Find any data-fetching functions the user's code contains and rename them to prevent clashes, then whittle the
35+
* template exporting wrapped versions instead down to only the functions found.
36+
*
37+
* @param userCode The source code of the current page file
38+
* @param templateCode The source code of the full template, including all functions
39+
* @param filepath The path to the current pagefile, within the project directory
40+
* @returns A tuple of modified user and template code
41+
*/
42+
function wrapFunctions(userCode: string, templateCode: string, filepath: string): string[] {
43+
let userAST: AST, templateAST: AST;
44+
const isTS = new RegExp('\\.tsx?$').test(filepath);
45+
46+
try {
47+
userAST = makeAST(userCode, isTS);
48+
templateAST = makeAST(templateCode, false);
49+
} catch (err) {
50+
logger.warn(`Couldn't add Sentry to ${filepath} because there was a parsing error: ${err}`);
51+
// Replace the template code with an empty string, so in the end the user code is untouched
52+
return [userCode, ''];
53+
}
54+
55+
// Comments are useful to have in the template for anyone reading it, but don't make sense to be injected into user
56+
// code, because they're about the template-i-ness of the template, not the code itself
57+
// TODO: Move this to our rollup build
58+
removeComments(templateAST);
59+
60+
for (const functionName of Object.keys(DATA_FETCHING_FUNCTIONS)) {
61+
// Find and rename all identifiers whose name is `functionName`
62+
const alias = renameIdentifiers(userAST, functionName);
63+
64+
// `alias` will be defined iff the user code contains the function in question and renaming has been done
65+
if (alias) {
66+
// We keep track of the alias for each function, so that later on we can fill it in for the placeholder in the
67+
// template. (Not doing that now because it's much more easily done once the template code has gone back to being
68+
// a string.)
69+
DATA_FETCHING_FUNCTIONS[functionName as keyof typeof DATA_FETCHING_FUNCTIONS].alias = alias;
70+
}
71+
72+
// Otherwise, if the current function doesn't exist anywhere in the user's code, delete the code in the template
73+
// wrapping that function
74+
//
75+
// Note: We start with all of the possible wrapper lines in the template and delete the ones we don't need (rather
76+
// than starting with none and adding in the ones we do need) because it allows them to live in our souce code as
77+
// *code*. If we added them in, they'd have to be strings containing code, and we'd lose all of the benefits of
78+
// syntax highlighting, linting, etc.
79+
else {
80+
// We have to look for declarations and exports separately because when we build the SDK, Rollup turns
81+
// export const XXX = ...
82+
// into
83+
// const XXX = ...
84+
// export { XXX }
85+
findExports(templateAST, functionName).remove();
86+
findDeclarations(templateAST, functionName).remove();
87+
}
88+
}
89+
90+
return [userAST.toSource(), templateAST.toSource()];
91+
}
92+
93+
/**
94+
* Wrap `getStaticPaths`, `getStaticProps`, and `getServerSideProps` (if they exist) in the given page code
95+
*/
96+
export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
97+
// We know one or the other will be defined, depending on the version of webpack being used
98+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
99+
const { projectDir } = this.getOptions ? this.getOptions() : this.query!;
100+
101+
// For now this loader only works for ESM code
102+
if (!isESM(userCode)) {
103+
return userCode;
104+
}
105+
106+
// 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
107+
// simple substring match (rather than waiting until we've parsed the code) because it's meant to be an
108+
// as-fast-as-possible fail-fast. It's possible for user code to pass this check, even if it contains none of the
109+
// functions in question, just by virtue of the correct string having been found, be it in a comment, as part of a
110+
// longer variable name, etc. That said, when we actually do the code manipulation we'll be working on the code's AST,
111+
// meaning we'll be able to differentiate between code we actually want to change and any false positives which might
112+
// come up here.)
113+
if (Object.keys(DATA_FETCHING_FUNCTIONS).every(functionName => !userCode.includes(functionName))) {
114+
return userCode;
115+
}
116+
117+
const templatePath = path.resolve(__dirname, '../templates/dataFetchersLoaderTemplate.js');
118+
// make sure the template is included when runing `webpack watch`
119+
this.addDependency(templatePath);
120+
121+
const templateCode = fs.readFileSync(templatePath).toString();
122+
123+
const [modifiedUserCode, modifiedTemplateCode] = wrapFunctions(
124+
userCode,
125+
templateCode,
126+
// Relative path to the page we're currently processing, for use in error messages
127+
path.relative(projectDir, this.resourcePath),
128+
);
129+
130+
// Fill in template placeholders
131+
let injectedCode = modifiedTemplateCode;
132+
for (const { placeholder, alias } of Object.values(DATA_FETCHING_FUNCTIONS)) {
133+
injectedCode = injectedCode.replace(placeholder, alias);
134+
}
135+
136+
return `${modifiedUserCode}\n${injectedCode}`;
137+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export { default as prefixLoader } from './prefixLoader';
2+
export { default as dataFetchersLoader } from './dataFetchersLoader';
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* Note: The implementation here is loosely based on the jsx and tsx parsers in 'jscodeshift'. It doesn't expose its
3+
* parsers, so we have to provide our own if we want to use anything besides the default. Fortunately, its parsers turn
4+
* out to just be wrappers around `babel.parse` with certain options set. The options chosen here are different from the
5+
* `jscodeshift` parsers in that a) unrecognized and deprecated options and options set to default values have been
6+
* removed, and b) all standard plugins are included, meaning the widest range of user code is able to be parsed.
7+
*/
8+
9+
import * as babel from '@babel/parser';
10+
import { File } from '@babel/types';
11+
12+
type Parser = {
13+
parse: (code: string) => babel.ParseResult<File>;
14+
};
15+
16+
const jsxOptions: babel.ParserOptions = {
17+
// Nextjs supports dynamic import, so this seems like a good idea
18+
allowImportExportEverywhere: true,
19+
// We're only supporting wrapping in ESM pages
20+
sourceType: 'module',
21+
// Without `tokens`, jsx parsing breaks
22+
tokens: true,
23+
// The maximal set of non-mutually-exclusive standard plugins, so as to support as much weird syntax in our users'
24+
// code as possible
25+
plugins: [
26+
'asyncDoExpressions',
27+
'decimal',
28+
['decorators', { decoratorsBeforeExport: false }],
29+
'decoratorAutoAccessors',
30+
'destructuringPrivate',
31+
'doExpressions',
32+
'estree',
33+
'exportDefaultFrom',
34+
'functionBind',
35+
'importMeta',
36+
'importAssertions',
37+
'jsx',
38+
'moduleBlocks',
39+
'partialApplication',
40+
['pipelineOperator', { proposal: 'hack', topicToken: '^' }],
41+
'regexpUnicodeSets',
42+
'throwExpressions',
43+
] as babel.ParserPlugin[],
44+
};
45+
46+
const tsxOptions = {
47+
...jsxOptions,
48+
// Because `jsxOptions` is typed as a `ParserOptions` object, TS doesn't discount the possibility of its `plugins`
49+
// property being undefined, even though it is, in fact, very clearly defined - hence the empty array.
50+
plugins: [...(jsxOptions.plugins || []), 'typescript'] as babel.ParserPlugin[],
51+
};
52+
53+
/**
54+
* Create either a jsx or tsx parser to be used by `jscodeshift`.
55+
*
56+
* @param type Either 'jsx' or 'tsx'
57+
* @returns An object with the appropriate `parse` method.
58+
*/
59+
export function makeParser(type: 'jsx' | 'tsx'): Parser {
60+
const options = type === 'jsx' ? jsxOptions : tsxOptions;
61+
return {
62+
parse: code => babel.parse(code, options),
63+
};
64+
}

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

+1-3
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ type LoaderOptions = {
1010
/**
1111
* Inject templated code into the beginning of a module.
1212
*/
13-
function prefixLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
13+
export default function prefixLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
1414
// We know one or the other will be defined, depending on the version of webpack being used
1515
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
1616
const { distDir } = this.getOptions ? this.getOptions() : this.query!;
@@ -25,5 +25,3 @@ function prefixLoader(this: LoaderThis<LoaderOptions>, userCode: string): string
2525

2626
return `${templateCode}\n${userCode}`;
2727
}
28-
29-
export { prefixLoader as default };

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

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
// TODO Use real webpack types
22
export type LoaderThis<Options> = {
3+
// Path to the file being loaded
4+
resourcePath: string;
5+
36
// Loader options in Webpack 4
47
query?: Options;
58
// Loader options in Webpack 5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import type {
2+
GetServerSideProps as GetServerSidePropsFunction,
3+
GetStaticPaths as GetStaticPathsFunction,
4+
GetStaticProps as GetStaticPropsFunction,
5+
} from 'next';
6+
7+
declare const __ORIG_GSSP__: GetServerSidePropsFunction;
8+
declare const __ORIG_GSPROPS__: GetStaticPropsFunction;
9+
declare const __ORIG_GSPATHS__: GetStaticPathsFunction;
10+
11+
// We import the SDK under a purposefully clunky name, to lessen to near zero the chances of a name collision in case
12+
// the user has also imported Sentry for some reason. (In the future, we could check for such a collision using the AST,
13+
// but this is a lot simpler.)
14+
//
15+
// TODO: This import line is here because it needs to be in the injected code, but it also would (ideally)
16+
// let us take advantage of typechecking, via the linter (both eslint and the TS linter), using intellisense, and when
17+
// building. Solving for all five simultaneously seems to be tricky, however, because of the circular dependency. This
18+
// is one of a number of possible compromise options, which seems to hit everything except eslint linting and
19+
// typechecking via `tsc`. (TS linting and intellisense both work, though, so we do get at least some type safety.) See
20+
// https://github.com/getsentry/sentry-javascript/pull/5503#discussion_r936827996 for more details.
21+
//
22+
// eslint-disable-next-line import/no-extraneous-dependencies, import/no-unresolved
23+
import * as ServerSideSentryNextjsSDK from '@sentry/nextjs';
24+
25+
export const getServerSideProps =
26+
typeof __ORIG_GSSP__ === 'function' ? ServerSideSentryNextjsSDK.withSentryGSSP(__ORIG_GSSP__) : __ORIG_GSSP__;
27+
export const getStaticProps =
28+
typeof __ORIG_GSPROPS__ === 'function'
29+
? ServerSideSentryNextjsSDK.withSentryGSProps(__ORIG_GSPROPS__)
30+
: __ORIG_GSPROPS__;
31+
export const getStaticPaths =
32+
typeof __ORIG_GSPATHS__ === 'function'
33+
? ServerSideSentryNextjsSDK.withSentryGSPaths(__ORIG_GSPATHS__)
34+
: __ORIG_GSPATHS__;

packages/nextjs/src/config/types.ts

+4
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ export type UserSentryOptions = {
4949
// 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
5050
// up uploading too many files, which is why this defaults to `false`.
5151
widenClientFileUpload?: boolean;
52+
53+
// Automatically wrap `getServerSideProps`, `getStaticProps`, and `getStaticPaths` in order to instrument them for
54+
// tracing.
55+
autoWrapDataFetchers?: boolean;
5256
};
5357

5458
export type NextConfigFunction = (phase: string, defaults: { defaultConfig: NextConfigObject }) => NextConfigObject;

packages/nextjs/src/config/webpack.ts

+15-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable max-lines */
22
import { getSentryRelease } from '@sentry/node';
3-
import { dropUndefinedKeys, logger } from '@sentry/utils';
3+
import { dropUndefinedKeys, escapeStringForRegex, logger } from '@sentry/utils';
44
import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin';
55
import * as fs from 'fs';
66
import * as path from 'path';
@@ -53,6 +53,8 @@ export function constructWebpackConfigFunction(
5353
newConfig = userNextConfig.webpack(newConfig, buildContext);
5454
}
5555

56+
const pageRegex = new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages(/.+)\\.(jsx?|tsx?)`);
57+
5658
if (isServer) {
5759
newConfig.module = {
5860
...newConfig.module,
@@ -74,6 +76,18 @@ export function constructWebpackConfigFunction(
7476
},
7577
],
7678
};
79+
80+
if (userSentryOptions.autoWrapDataFetchers) {
81+
newConfig.module.rules.push({
82+
test: pageRegex,
83+
use: [
84+
{
85+
loader: path.resolve(__dirname, 'loaders/dataFetchersLoader.js'),
86+
options: { projectDir },
87+
},
88+
],
89+
});
90+
}
7791
}
7892

7993
// The SDK uses syntax (ES6 and ES6+ features like object spread) which isn't supported by older browsers. For users
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export { withSentryGSPaths } from './withSentryGSPaths';
2+
export { withSentryGSProps } from './withSentryGSProps';
3+
export { withSentryGSSP } from './withSentryGSSP';

0 commit comments

Comments
 (0)