Skip to content

Commit

Permalink
feat(nextjs): Add excludeServerRoutes config option (#6207)
Browse files Browse the repository at this point in the history
Currently, in the nextjs SDK, we inject the user's `Sentry.init()` code (by way of their `sentry.server.config.js` file) into all serverside routes. This adds a new option to the `sentry` object in `next.config.js` which allows users to prevent specific routes from being instrumented in this way. In this option, excluded routes can be specified using either strings (which need to exactly match the route) or regexes.

Note: Heavily inspired by #6125. h/t to @lforst for his work there. Compared to that PR, this one allows non-API routes to be excluded and allows excluded pages to be specified as routes rather than filepaths. (Using routes a) obviates the need for users to add `pages/` to the beginning of every entry, b) abstracts away the differences between windows and POSIX paths, and c) futureproofs users' config values against underlying changes to project file organization.)

Docs for this feature are being added in getsentry/sentry-docs#5789.

Fixes #6119.
Fixes #5964.
  • Loading branch information
lobsterkatie committed Nov 15, 2022
1 parent 66bcbd7 commit fd6ae0f
Show file tree
Hide file tree
Showing 10 changed files with 163 additions and 8 deletions.
14 changes: 12 additions & 2 deletions packages/nextjs/src/config/loaders/proxyLoader.ts
@@ -1,4 +1,4 @@
import { escapeStringForRegex, logger } from '@sentry/utils';
import { escapeStringForRegex, logger, stringMatchesSomePattern } from '@sentry/utils';
import * as fs from 'fs';
import * as path from 'path';

Expand All @@ -8,6 +8,7 @@ import { LoaderThis } from './types';
type LoaderOptions = {
pagesDir: string;
pageExtensionRegex: string;
excludeServerRoutes: Array<RegExp | string>;
};

/**
Expand All @@ -17,7 +18,11 @@ type LoaderOptions = {
*/
export default async function proxyLoader(this: LoaderThis<LoaderOptions>, userCode: string): Promise<string> {
// We know one or the other will be defined, depending on the version of webpack being used
const { pagesDir, pageExtensionRegex } = 'getOptions' in this ? this.getOptions() : this.query;
const {
pagesDir,
pageExtensionRegex,
excludeServerRoutes = [],
} = 'getOptions' in this ? this.getOptions() : this.query;

// Get the parameterized route name from this page's filepath
const parameterizedRoute = path
Expand All @@ -34,6 +39,11 @@ export default async function proxyLoader(this: LoaderThis<LoaderOptions>, userC
// homepage), sub back in the root route
.replace(/^$/, '/');

// Skip explicitly-ignored pages
if (stringMatchesSomePattern(parameterizedRoute, excludeServerRoutes, true)) {
return userCode;
}

// We don't want to wrap twice (or infinitely), so in the proxy we add this query string onto references to the
// wrapped file, so that we know that it's already been processed. (Adding this query string is also necessary to
// convince webpack that it's a different file than the one it's in the middle of loading now, so that the originals
Expand Down
8 changes: 8 additions & 0 deletions packages/nextjs/src/config/types.ts
Expand Up @@ -59,6 +59,14 @@ export type UserSentryOptions = {

// Automatically instrument Next.js data fetching methods and Next.js API routes
autoInstrumentServerFunctions?: boolean;

// Exclude certain serverside API routes or pages from being instrumented with Sentry. This option takes an array of
// strings or regular expressions.
//
// NOTE: Pages should be specified as routes (`/animals` or `/api/animals/[animalType]/habitat`), not filepaths
// (`pages/animals/index.js` or `.\src\pages\api\animals\[animalType]\habitat.tsx`), and strings must be be a full,
// exact match.
excludeServerRoutes?: Array<RegExp | string>;
};

export type NextConfigFunction = (phase: string, defaults: { defaultConfig: NextConfigObject }) => NextConfigObject;
Expand Down
36 changes: 30 additions & 6 deletions packages/nextjs/src/config/webpack.ts
@@ -1,6 +1,6 @@
/* eslint-disable max-lines */
import { getSentryRelease } from '@sentry/node';
import { arrayify, dropUndefinedKeys, escapeStringForRegex, logger } from '@sentry/utils';
import { arrayify, dropUndefinedKeys, escapeStringForRegex, logger, stringMatchesSomePattern } from '@sentry/utils';
import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin';
import * as chalk from 'chalk';
import * as fs from 'fs';
Expand Down Expand Up @@ -91,7 +91,11 @@ export function constructWebpackConfigFunction(
use: [
{
loader: path.resolve(__dirname, 'loaders/proxyLoader.js'),
options: { pagesDir, pageExtensionRegex },
options: {
pagesDir,
pageExtensionRegex,
excludeServerRoutes: userSentryOptions.excludeServerRoutes,
},
},
],
});
Expand Down Expand Up @@ -135,7 +139,7 @@ export function constructWebpackConfigFunction(
// will call the callback which will call `f` which will call `x.y`... and on and on. Theoretically this could also
// be fixed by using `bind`, but this is way simpler.)
const origEntryProperty = newConfig.entry;
newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, buildContext);
newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, buildContext, userSentryOptions);

// Enable the Sentry plugin (which uploads source maps to Sentry when not in dev) by default
if (shouldEnableWebpackPlugin(buildContext, userSentryOptions)) {
Expand Down Expand Up @@ -248,6 +252,7 @@ function findTranspilationRules(rules: WebpackModuleRule[] | undefined, projectD
async function addSentryToEntryProperty(
currentEntryProperty: WebpackEntryProperty,
buildContext: BuildContext,
userSentryOptions: UserSentryOptions,
): Promise<EntryPropertyObject> {
// The `entry` entry in a webpack config can be a string, array of strings, object, or function. By default, nextjs
// sets it to an async function which returns the promise of an object of string arrays. Because we don't know whether
Expand All @@ -268,8 +273,18 @@ async function addSentryToEntryProperty(

// inject into all entry points which might contain user's code
for (const entryPointName in newEntryProperty) {
if (shouldAddSentryToEntryPoint(entryPointName, isServer)) {
if (shouldAddSentryToEntryPoint(entryPointName, isServer, userSentryOptions.excludeServerRoutes)) {
addFilesToExistingEntryPoint(newEntryProperty, entryPointName, filesToInject);
} else {
if (
isServer &&
// If the user has asked to exclude pages, confirm for them that it's worked
userSentryOptions.excludeServerRoutes &&
// We always skip these, so it's not worth telling the user that we've done so
!['pages/_app', 'pages/_document'].includes(entryPointName)
) {
__DEBUG_BUILD__ && logger.log(`Skipping Sentry injection for ${entryPointName.replace(/^pages/, '')}`);
}
}
}

Expand Down Expand Up @@ -377,13 +392,21 @@ function checkWebpackPluginOverrides(
*
* @param entryPointName The name of the entry point in question
* @param isServer Whether or not this function is being called in the context of a server build
* @param excludeServerRoutes A list of excluded serverside entrypoints provided by the user
* @returns `true` if sentry code should be injected, and `false` otherwise
*/
function shouldAddSentryToEntryPoint(entryPointName: string, isServer: boolean): boolean {
function shouldAddSentryToEntryPoint(
entryPointName: string,
isServer: boolean,
excludeServerRoutes: Array<string | RegExp> = [],
): boolean {
// On the server side, by default we inject the `Sentry.init()` code into every page (with a few exceptions).
if (isServer) {
const entryPointRoute = entryPointName.replace(/^pages/, '');
if (
// User-specified pages to skip. (Note: For ease of use, `excludeServerRoutes` is specified in terms of routes,
// which don't have the `pages` prefix.)
stringMatchesSomePattern(entryPointRoute, excludeServerRoutes, true) ||
// All non-API pages contain both of these components, and we don't want to inject more than once, so as long as
// we're doing the individual pages, it's fine to skip these. (Note: Even if a given user doesn't have either or
// both of these in their `pages/` folder, they'll exist as entrypoints because nextjs will supply default
Expand Down Expand Up @@ -462,7 +485,8 @@ export function getWebpackPluginOptions(
configFile: hasSentryProperties ? 'sentry.properties' : undefined,
stripPrefix: ['webpack://_N_E/'],
urlPrefix,
entries: (entryPointName: string) => shouldAddSentryToEntryPoint(entryPointName, isServer),
entries: (entryPointName: string) =>
shouldAddSentryToEntryPoint(entryPointName, isServer, userSentryOptions.excludeServerRoutes),
release: getSentryRelease(buildId),
dryRun: isDev,
});
Expand Down
25 changes: 25 additions & 0 deletions packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts
Expand Up @@ -241,5 +241,30 @@ describe('constructWebpackConfigFunction()', () => {
simulatorBundle: './src/simulator/index.ts',
});
});

it('does not inject into routes included in `excludeServerRoutes`', async () => {
const nextConfigWithExcludedRoutes = {
...exportedNextConfig,
sentry: {
excludeServerRoutes: [/simulator/],
},
};
const finalWebpackConfig = await materializeFinalWebpackConfig({
exportedNextConfig: nextConfigWithExcludedRoutes,
incomingWebpackConfig: serverWebpackConfig,
incomingWebpackBuildContext: serverBuildContext,
});

expect(finalWebpackConfig.entry).toEqual(
expect.objectContaining({
'pages/simulator/leaderboard': {
import: expect.not.arrayContaining([serverConfigFilePath]),
},
'pages/api/simulator/dogStats/[name]': {
import: expect.not.arrayContaining([serverConfigFilePath]),
},
}),
);
});
});
});
1 change: 1 addition & 0 deletions packages/nextjs/test/integration/next.config.js
Expand Up @@ -9,6 +9,7 @@ const moduleExports = {
// Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts`
// TODO (v8): This can come out in v8, because this option will get a default value
hideSourceMaps: false,
excludeServerRoutes: ['/api/excludedEndpoints/excludedWithString', /\/api\/excludedEndpoints\/excludedWithRegExp/],
},
};
const SentryWebpackPluginOptions = {
Expand Down
4 changes: 4 additions & 0 deletions packages/nextjs/test/integration/next10.config.template
Expand Up @@ -10,6 +10,10 @@ const moduleExports = {
// Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts`
// TODO (v8): This can come out in v8, because this option will get a default value
hideSourceMaps: false,
excludeServerRoutes: [
'/api/excludedEndpoints/excludedWithString',
/\/api\/excludedEndpoints\/excludedWithRegExp/,
],
},
};

Expand Down
4 changes: 4 additions & 0 deletions packages/nextjs/test/integration/next11.config.template
Expand Up @@ -11,6 +11,10 @@ const moduleExports = {
// Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts`
// TODO (v8): This can come out in v8, because this option will get a default value
hideSourceMaps: false,
excludeServerRoutes: [
'/api/excludedEndpoints/excludedWithString',
/\/api\/excludedEndpoints\/excludedWithRegExp/,
],
},
};

Expand Down
@@ -0,0 +1,6 @@
// This file will test the `excludeServerRoutes` option when a route is provided as a RegExp.
const handler = async (): Promise<void> => {
throw new Error('API Error');
};

export default handler;
@@ -0,0 +1,6 @@
// This file will test the `excludeServerRoutes` option when a route is provided as a string.
const handler = async (): Promise<void> => {
throw new Error('API Error');
};

export default handler;
@@ -0,0 +1,67 @@
const assert = require('assert');

const { sleep } = require('../utils/common');
const { getAsync, interceptEventRequest, interceptTracingRequest } = require('../utils/server');

module.exports = async ({ url: urlBase, argv }) => {
const regExpUrl = `${urlBase}/api/excludedEndpoints/excludedWithRegExp`;
const stringUrl = `${urlBase}/api/excludedEndpoints/excludedWithString`;

const capturedRegExpErrorRequest = interceptEventRequest(
{
exception: {
values: [
{
type: 'Error',
value: 'API Error',
},
],
},
tags: {
runtime: 'node',
},
request: {
url: regExpUrl,
method: 'GET',
},
transaction: 'GET /api/excludedEndpoints/excludedWithRegExp',
},
argv,
'excluded API endpoint via RegExp',
);

const capturedStringErrorRequest = interceptEventRequest(
{
exception: {
values: [
{
type: 'Error',
value: 'API Error',
},
],
},
tags: {
runtime: 'node',
},
request: {
url: regExpUrl,
method: 'GET',
},
transaction: 'GET /api/excludedEndpoints/excludedWithString',
},
argv,
'excluded API endpoint via String',
);

await Promise.all([getAsync(regExpUrl), getAsync(stringUrl)]);
await sleep(250);

assert.ok(
!capturedRegExpErrorRequest.isDone(),
'Did intercept error request even though route should be excluded (RegExp)',
);
assert.ok(
!capturedStringErrorRequest.isDone(),
'Did intercept error request even though route should be excluded (String)',
);
};

0 comments on commit fd6ae0f

Please sign in to comment.