Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nextjs): Auto-wrap edge-routes and middleware #6746

Merged
merged 10 commits into from
Jan 16, 2023
8 changes: 6 additions & 2 deletions packages/nextjs/rollup.npm.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ export default [
),
...makeNPMConfigVariants(
makeBaseNPMConfig({
entrypoints: ['src/config/templates/pageWrapperTemplate.ts', 'src/config/templates/apiWrapperTemplate.ts'],
entrypoints: [
'src/config/templates/pageWrapperTemplate.ts',
'src/config/templates/apiWrapperTemplate.ts',
'src/config/templates/middlewareWrapperTemplate.ts',
],

packageSpecificConfig: {
output: {
Expand All @@ -29,7 +33,7 @@ export default [
// make it so Rollup calms down about the fact that we're combining default and named exports
exports: 'named',
},
external: ['@sentry/nextjs', '__SENTRY_WRAPPING_TARGET__'],
external: ['@sentry/nextjs', '__SENTRY_WRAPPING_TARGET_FILE__'],
},
}),
),
Expand Down
26 changes: 16 additions & 10 deletions packages/nextjs/src/config/loaders/wrappingLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,19 @@ const apiWrapperTemplateCode = fs.readFileSync(apiWrapperTemplatePath, { encodin
const pageWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'pageWrapperTemplate.js');
const pageWrapperTemplateCode = fs.readFileSync(pageWrapperTemplatePath, { encoding: 'utf8' });

const middlewareWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'middlewareWrapperTemplate.js');
const middlewareWrapperTemplateCode = fs.readFileSync(middlewareWrapperTemplatePath, { encoding: 'utf8' });

// Just a simple placeholder to make referencing module consistent
const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module';

// Needs to end in .cjs in order for the `commonjs` plugin to pick it up
const WRAPPING_TARGET_MODULE_NAME = '__SENTRY_WRAPPING_TARGET__.cjs';
const WRAPPING_TARGET_MODULE_NAME = '__SENTRY_WRAPPING_TARGET_FILE__.cjs';

type LoaderOptions = {
pagesDir: string;
pageExtensionRegex: string;
excludeServerRoutes: Array<RegExp | string>;
isEdgeRuntime: boolean;
};

/**
Expand All @@ -40,14 +42,8 @@ export default function wrappingLoader(
pagesDir,
pageExtensionRegex,
excludeServerRoutes = [],
isEdgeRuntime,
} = 'getOptions' in this ? this.getOptions() : this.query;

// We currently don't support the edge runtime
if (isEdgeRuntime) {
return userCode;
}

this.async();

// Get the parameterized route name from this page's filepath
Expand All @@ -71,13 +67,23 @@ export default function wrappingLoader(
return;
}

let templateCode = parameterizedRoute.startsWith('/api') ? apiWrapperTemplateCode : pageWrapperTemplateCode;
const middlewareJsPath = path.join(pagesDir, '..', 'middleware.js');
const middlewareTsPath = path.join(pagesDir, '..', 'middleware.js');

let templateCode: string;
if (parameterizedRoute.startsWith('/api')) {
templateCode = apiWrapperTemplateCode;
} else if (this.resourcePath === middlewareJsPath || this.resourcePath === middlewareTsPath) {
templateCode = middlewareWrapperTemplateCode;
} else {
templateCode = pageWrapperTemplateCode;
}

// Inject the route and the path to the file we're wrapping into the template
templateCode = templateCode.replace(/__ROUTE__/g, parameterizedRoute.replace(/\\/g, '\\\\'));

// Replace the import path of the wrapping target in the template with a path that the `wrapUserCode` function will understand.
templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET__/g, WRAPPING_TARGET_MODULE_NAME);
templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, WRAPPING_TARGET_MODULE_NAME);

// Run the proxy module code through Rollup, in order to split the `export * from '<wrapped file>'` out into
// individual exports (which nextjs seems to require).
Expand Down
8 changes: 4 additions & 4 deletions packages/nextjs/src/config/templates/apiWrapperTemplate.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
/**
/*
* This file is a template for the code which will be substituted when our webpack loader handles API files in the
* `pages/` directory.
*
* We use `__RESOURCE_PATH__` as a placeholder for the path to the file being wrapped. Because it's not a real package,
* We use `__SENTRY_WRAPPING_TARGET_FILE__` as a placeholder for the path to the file being wrapped. Because it's not a real package,
* this causes both TS and ESLint to complain, hence the pragma comments below.
*/

// @ts-ignore See above
// eslint-disable-next-line import/no-unresolved
import * as origModule from '__SENTRY_WRAPPING_TARGET__';
import * as origModule from '__SENTRY_WRAPPING_TARGET_FILE__';
// eslint-disable-next-line import/no-extraneous-dependencies
import * as Sentry from '@sentry/nextjs';
import type { PageConfig } from 'next';
Expand Down Expand Up @@ -60,4 +60,4 @@ export default userProvidedHandler ? Sentry.withSentryAPI(userProvidedHandler, '
// not include anything whose name matchs something we've explicitly exported above.
// @ts-ignore See above
// eslint-disable-next-line import/no-unresolved
export * from '__SENTRY_WRAPPING_TARGET__';
export * from '__SENTRY_WRAPPING_TARGET_FILE__';
50 changes: 50 additions & 0 deletions packages/nextjs/src/config/templates/middlewareWrapperTemplate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* This file is a template for the code which will be substituted when our webpack loader handles middleware files.
*
* We use `__SENTRY_WRAPPING_TARGET_FILE__` as a placeholder for the path to the file being wrapped. Because it's not a real package,
* this causes both TS and ESLint to complain, hence the pragma comments below.
*/

// @ts-ignore See above
// eslint-disable-next-line import/no-unresolved
import * as origModule from '__SENTRY_WRAPPING_TARGET_FILE__';
// eslint-disable-next-line import/no-extraneous-dependencies
import * as Sentry from '@sentry/nextjs';

import type { EdgeRouteHandler } from '../../edge/types';

type NextApiModule =
| {
// ESM export
default?: EdgeRouteHandler;
middleware?: EdgeRouteHandler;
}
// CJS export
| EdgeRouteHandler;

const userApiModule = origModule as NextApiModule;

// Default to undefined. It's possible for Next.js users to not define any exports/handlers in an API route. If that is
// the case Next.js wil crash during runtime but the Sentry SDK should definitely not crash so we need tohandle it.
let userProvidedNamedHandler: EdgeRouteHandler | undefined = undefined;
let userProvidedDefaultHandler: EdgeRouteHandler | undefined = undefined;

if ('middleware' in userApiModule && typeof userApiModule.middleware === 'function') {
// Handle when user defines via named ESM export: `export { middleware };`
userProvidedNamedHandler = userApiModule.middleware;
} else if ('default' in userApiModule && typeof userApiModule.default === 'function') {
// Handle when user defines via ESM export: `export default myFunction;`
userProvidedDefaultHandler = userApiModule.default;
} else if (typeof userApiModule === 'function') {
// Handle when user defines via CJS export: "module.exports = myFunction;"
userProvidedDefaultHandler = userApiModule;
}

export const middleware = userProvidedNamedHandler ? Sentry.withSentryMiddleware(userProvidedNamedHandler) : undefined;
export default userProvidedDefaultHandler ? Sentry.withSentryMiddleware(userProvidedDefaultHandler) : undefined;

// Re-export anything exported by the page module we're wrapping. When processing this code, Rollup is smart enough to
// not include anything whose name matchs something we've explicitly exported above.
// @ts-ignore See above
// eslint-disable-next-line import/no-unresolved
export * from '__SENTRY_WRAPPING_TARGET_FILE__';
8 changes: 4 additions & 4 deletions packages/nextjs/src/config/templates/pageWrapperTemplate.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
/**
/*
* This file is a template for the code which will be substituted when our webpack loader handles non-API files in the
* `pages/` directory.
*
* We use `__RESOURCE_PATH__` as a placeholder for the path to the file being wrapped. Because it's not a real package,
* We use `__SENTRY_WRAPPING_TARGET_FILE__` as a placeholder for the path to the file being wrapped. Because it's not a real package,
* this causes both TS and ESLint to complain, hence the pragma comments below.
*/

// @ts-ignore See above
// eslint-disable-next-line import/no-unresolved
import * as wrapee from '__SENTRY_WRAPPING_TARGET__';
import * as wrapee from '__SENTRY_WRAPPING_TARGET_FILE__';
// eslint-disable-next-line import/no-extraneous-dependencies
import * as Sentry from '@sentry/nextjs';
import type { GetServerSideProps, GetStaticProps, NextPage as NextPageComponent } from 'next';
Expand Down Expand Up @@ -54,4 +54,4 @@ export default pageComponent;
// not include anything whose name matchs something we've explicitly exported above.
// @ts-ignore See above
// eslint-disable-next-line import/no-unresolved
export * from '__SENTRY_WRAPPING_TARGET__';
export * from '__SENTRY_WRAPPING_TARGET_FILE__';
71 changes: 50 additions & 21 deletions packages/nextjs/src/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export type ExportedNextConfig = NextConfigObjectWithSentry | NextConfigFunction
export type NextConfigObjectWithSentry = NextConfigObject & {
sentry?: UserSentryOptions;
};

export type NextConfigFunctionWithSentry = (
phase: string,
defaults: { defaultConfig: NextConfigObject },
Expand Down Expand Up @@ -60,39 +61,67 @@ export type NextConfigObject = {
};

export type UserSentryOptions = {
// Override the SDK's default decision about whether or not to enable to the webpack plugin. Note that `false` forces
// the plugin to be enabled, even in situations where it's not recommended.
/**
* Override the SDK's default decision about whether or not to enable to the Sentry webpack plugin for server files.
* Note that `false` forces the plugin to be enabled, even in situations where it's not recommended.
*/
disableServerWebpackPlugin?: boolean;

/**
* Override the SDK's default decision about whether or not to enable to the Sentry webpack plugin for client files.
* Note that `false` forces the plugin to be enabled, even in situations where it's not recommended.
*/
disableClientWebpackPlugin?: boolean;

// Use `hidden-source-map` for webpack `devtool` option, which strips the `sourceMappingURL` from the bottom of built
// JS files
/**
* Use `hidden-source-map` for webpack `devtool` option, which strips the `sourceMappingURL` from the bottom of built
* JS files.
*/
hideSourceMaps?: boolean;

// Force webpack to apply the same transpilation rules to the SDK code as apply to user code. Helpful when targeting
// older browsers which don't support ES6 (or ES6+ features like object spread).
/**
* Instructs webpack to apply the same transpilation rules to the SDK code as apply to user code. Helpful when
* targeting older browsers which don't support ES6 (or ES6+ features like object spread).
*/
transpileClientSDK?: boolean;

// Upload files from `<distDir>/static/chunks` rather than `<distDir>/static/chunks/pages`. Usually files outside of
// `pages/` only contain third-party code, but in cases where they contain user code, restricting the webpack
// plugin's upload breaks sourcemaps for those user-code-containing files, because it keeps them from being
// 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
// up uploading too many files, which is why this defaults to `false`.
/**
* Instructs the Sentry webpack plugin to upload source files from `<distDir>/static/chunks` rather than
* `<distDir>/static/chunks/pages`. Usually files outside of `pages/` only contain third-party code, but in cases
* where they contain user code, restricting the webpack plugin's upload breaks sourcemaps for those
* user-code-containing files, because it keeps them from being uploaded. Defaults to `false`.
*/
// We don't want to widen the scope if we don't have to, because we're guaranteed to end up uploading too many files,
// which is why this defaults to`false`.
widenClientFileUpload?: boolean;

// Automatically instrument Next.js data fetching methods and Next.js API routes
/**
* Automatically instrument Next.js data fetching methods and Next.js API routes with error and performance monitoring.
* Defaults to `true`.
*/
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.
/**
* Automatically instrument Next.js middleware with error and performance monitoring. Defaults to `true`.
*/
autoInstrumentMiddleware?: 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>;

// Tunnel Sentry requests through this route on the Next.js server, to circumvent ad-blockers blocking Sentry events from being sent.
// This option should be a path (for example: '/error-monitoring').
/**
* Tunnel Sentry requests through this route on the Next.js server, to circumvent ad-blockers blocking Sentry events
* from being sent. This option should be a path (for example: '/error-monitoring').
*
* NOTE: This feature only works with Next.js 11+
*/
tunnelRoute?: string;
};

Expand Down Expand Up @@ -164,7 +193,7 @@ export type EntryPointObject = { import: string | Array<string> };
*/

export type WebpackModuleRule = {
test?: string | RegExp;
test?: string | RegExp | ((resourcePath: string) => boolean);
include?: Array<string | RegExp> | RegExp;
exclude?: (filepath: string) => boolean;
use?: ModuleRuleUseProperty | Array<ModuleRuleUseProperty>;
Expand Down
46 changes: 42 additions & 4 deletions packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,23 +99,61 @@ export function constructWebpackConfigFunction(

if (isServer) {
if (userSentryOptions.autoInstrumentServerFunctions !== false) {
const pagesDir = newConfig.resolve?.alias?.['private-next-pages'] as string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We looked at this really obscure field with a typecast here, and I am not sure if it is even part of the Next.js public API. Behavior around the pages and pages/src directory is pretty well defined here and it felt safer in the long run to just do the lookup ourselves. I guess it is not really required to be in this PR but snuck it in as a little cleanup.

let pagesDirPath: string;
if (
fs.existsSync(path.join(projectDir, 'pages')) &&
fs.lstatSync(path.join(projectDir, 'pages')).isDirectory()
) {
pagesDirPath = path.join(projectDir, 'pages');
} else {
pagesDirPath = path.join(projectDir, 'src', 'pages');
}

const middlewareJsPath = path.join(pagesDirPath, '..', 'middleware.js');
const middlewareTsPath = path.join(pagesDirPath, '..', 'middleware.ts');

// Default page extensions per https://github.com/vercel/next.js/blob/f1dbc9260d48c7995f6c52f8fbcc65f08e627992/packages/next/server/config-shared.ts#L161
const pageExtensions = userNextConfig.pageExtensions || ['tsx', 'ts', 'jsx', 'js'];
const dotPrefixedPageExtensions = pageExtensions.map(ext => `.${ext}`);
const pageExtensionRegex = pageExtensions.map(escapeStringForRegex).join('|');

// It is very important that we insert our loader at the beginning of the array because we expect any sort of transformations/transpilations (e.g. TS -> JS) to already have happened.
newConfig.module.rules.unshift({
test: new RegExp(`^${escapeStringForRegex(pagesDir)}.*\\.(${pageExtensionRegex})$`),
test: resourcePath => {
// We generally want to apply the loader to all API routes, pages and to the middleware file.

// `resourcePath` may be an absolute path or a path relative to the context of the webpack config
let absoluteResourcePath: string;
if (path.isAbsolute(resourcePath)) {
absoluteResourcePath = resourcePath;
} else {
absoluteResourcePath = path.join(projectDir, resourcePath);
}
const normalizedAbsoluteResourcePath = path.normalize(absoluteResourcePath);

if (
// Match everything inside pages/ with the appropriate file extension
normalizedAbsoluteResourcePath.startsWith(pagesDirPath) &&
dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext))
) {
return true;
} else if (
// Match middleware.js and middleware.ts
normalizedAbsoluteResourcePath === middlewareJsPath ||
normalizedAbsoluteResourcePath === middlewareTsPath
) {
return userSentryOptions.autoInstrumentMiddleware ?? true;
} else {
return false;
}
},
use: [
{
loader: path.resolve(__dirname, 'loaders/wrappingLoader.js'),
options: {
pagesDir,
pagesDir: pagesDirPath,
pageExtensionRegex,
excludeServerRoutes: userSentryOptions.excludeServerRoutes,
isEdgeRuntime: buildContext.nextRuntime === 'edge',
},
},
],
Expand Down
2 changes: 1 addition & 1 deletion packages/nextjs/src/edge/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// We cannot make any assumptions about what users define as their handler except maybe that it is a function
export interface EdgeRouteHandler {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(req: any): any | Promise<any>;
(...args: any[]): any;
}