Skip to content

feat(nextjs): Use Edge SDK for Edge bundles #6753

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

Merged
merged 4 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
},
"main": "build/cjs/index.js",
"module": "build/esm/index.js",
"browser": "build/esm/client/index.js",
"types": "build/types/index.types.d.ts",
"publishConfig": {
"access": "public"
Expand Down
1 change: 1 addition & 0 deletions packages/nextjs/src/config/loaders/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { default as valueInjectionLoader } from './valueInjectionLoader';
export { default as prefixLoader } from './prefixLoader';
export { default as wrappingLoader } from './wrappingLoader';
export { default as sdkMultiplexerLoader } from './sdkMultiplexerLoader';
24 changes: 24 additions & 0 deletions packages/nextjs/src/config/loaders/sdkMultiplexerLoader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type { LoaderThis } from './types';

type LoaderOptions = {
importTarget: string;
};

/**
* This loader allows us to multiplex SDKs depending on what is passed to the `importTarget` loader option.
* If this loader encounters a file that contains the string "__SENTRY_SDK_MULTIPLEXER__" it will replace it's entire
* content with an "export all"-statement that points to `importTarget`.
*
* In our case we use this to multiplex different SDKs depending on whether we're bundling browser code, server code,
* or edge-runtime code.
*/
export default function sdkMultiplexerLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
if (!userCode.includes('__SENTRY_SDK_MULTIPLEXER__')) {
return userCode;
}

// We know one or the other will be defined, depending on the version of webpack being used
const { importTarget } = 'getOptions' in this ? this.getOptions() : this.query;

return `export * from "${importTarget}";`;
}
73 changes: 37 additions & 36 deletions packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,17 @@ export function constructWebpackConfigFunction(
// Add a loader which will inject code that sets global values
addValueInjectionLoader(newConfig, userNextConfig, userSentryOptions);

if (buildContext.nextRuntime === 'edge') {
// eslint-disable-next-line no-console
console.warn(
'[@sentry/nextjs] You are using edge functions or middleware. Please note that Sentry does not yet support error monitoring for these features.',
);
}
newConfig.module.rules.push({
test: /node_modules\/@sentry\/nextjs/,
use: [
{
loader: path.resolve(__dirname, 'loaders/sdkMultiplexerLoader.js'),
options: {
importTarget: buildContext.nextRuntime === 'edge' ? './edge' : './client',
},
},
],
});

if (isServer) {
if (userSentryOptions.autoInstrumentServerFunctions !== false) {
Expand Down Expand Up @@ -301,28 +306,25 @@ async function addSentryToEntryProperty(
// we know is that it won't have gotten *simpler* in form, so we only need to worry about the object and function
// options. See https://webpack.js.org/configuration/entry-context/#entry.

const { isServer, dir: projectDir, dev: isDev } = buildContext;
const { isServer, dir: projectDir, dev: isDev, nextRuntime } = buildContext;

const newEntryProperty =
typeof currentEntryProperty === 'function' ? await currentEntryProperty() : { ...currentEntryProperty };

// `sentry.server.config.js` or `sentry.client.config.js` (or their TS equivalents)
const userConfigFile = isServer ? getUserConfigFile(projectDir, 'server') : getUserConfigFile(projectDir, 'client');
const userConfigFile =
nextRuntime === 'edge'
? getUserConfigFile(projectDir, 'edge')
: isServer
? getUserConfigFile(projectDir, 'server')
: getUserConfigFile(projectDir, 'client');

// we need to turn the filename into a path so webpack can find it
const filesToInject = [`./${userConfigFile}`];
const filesToInject = userConfigFile ? [`./${userConfigFile}`] : [];

// inject into all entry points which might contain user's code
for (const entryPointName in newEntryProperty) {
if (
shouldAddSentryToEntryPoint(
entryPointName,
isServer,
userSentryOptions.excludeServerRoutes,
isDev,
buildContext.nextRuntime === 'edge',
)
) {
if (shouldAddSentryToEntryPoint(entryPointName, isServer, userSentryOptions.excludeServerRoutes, isDev)) {
addFilesToExistingEntryPoint(newEntryProperty, entryPointName, filesToInject);
} else {
if (
Expand All @@ -345,10 +347,11 @@ async function addSentryToEntryProperty(
* TypeScript or JavaScript file.
*
* @param projectDir The root directory of the project, where the file should be located
* @param platform Either "server" or "client", so that we know which file to look for
* @returns The name of the relevant file. If no file is found, this method throws an error.
* @param platform Either "server", "client" or "edge", so that we know which file to look for
* @returns The name of the relevant file. If the server or client file is not found, this method throws an error. The
* edge file is optional, if it is not found this function will return `undefined`.
*/
export function getUserConfigFile(projectDir: string, platform: 'server' | 'client'): string {
export function getUserConfigFile(projectDir: string, platform: 'server' | 'client' | 'edge'): string | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

does this mean users have to define a sentry.edge.config.js?

we have to update the docstring if this is the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does this mean users have to define a sentry.edge.config.js?

They don't have to in the sense that something will crash, but they should if they want to capture events. We're even logging a warning that they should add the file in case they're using edge stuff.

we have to update the docstring if this is the case.

Thanks for pointing that out. I updated the docstring.

const possibilities = [`sentry.${platform}.config.ts`, `sentry.${platform}.config.js`];

for (const filename of possibilities) {
Expand All @@ -357,7 +360,16 @@ export function getUserConfigFile(projectDir: string, platform: 'server' | 'clie
}
}

throw new Error(`Cannot find '${possibilities[0]}' or '${possibilities[1]}' in '${projectDir}'.`);
// Edge config file is optional
if (platform === 'edge') {
// eslint-disable-next-line no-console
console.warn(
'[@sentry/nextjs] You are using Next.js features that run on the Edge Runtime. Please add a "sentry.edge.config.js" or a "sentry.edge.config.ts" file to your project root in which you initialize the Sentry SDK with "Sentry.init()".',
);
return;
} else {
throw new Error(`Cannot find '${possibilities[0]}' or '${possibilities[1]}' in '${projectDir}'.`);
}
}

/**
Expand Down Expand Up @@ -449,11 +461,9 @@ function shouldAddSentryToEntryPoint(
isServer: boolean,
excludeServerRoutes: Array<string | RegExp> = [],
isDev: boolean,
isEdgeRuntime: boolean,
): boolean {
// We don't support the Edge runtime yet
if (isEdgeRuntime) {
return false;
if (entryPointName === 'middleware') {
return true;
}

// On the server side, by default we inject the `Sentry.init()` code into every page (with a few exceptions).
Expand All @@ -479,9 +489,6 @@ function shouldAddSentryToEntryPoint(
// versions.)
entryPointRoute === '/_app' ||
entryPointRoute === '/_document' ||
// While middleware was in beta, it could be anywhere (at any level) in the `pages` directory, and would be called
// `_middleware.js`. Until the SDK runs successfully in the lambda edge environment, we have to exclude these.
entryPointName.includes('_middleware') ||
// Newer versions of nextjs are starting to introduce things outside the `pages/` folder (middleware, an `app/`
// directory, etc), but until those features are stable and we know how we want to support them, the safest bet is
// not to inject anywhere but inside `pages/`.
Expand Down Expand Up @@ -552,13 +559,7 @@ export function getWebpackPluginOptions(
stripPrefix: ['webpack://_N_E/'],
urlPrefix,
entries: (entryPointName: string) =>
shouldAddSentryToEntryPoint(
entryPointName,
isServer,
userSentryOptions.excludeServerRoutes,
isDev,
buildContext.nextRuntime === 'edge',
),
shouldAddSentryToEntryPoint(entryPointName, isServer, userSentryOptions.excludeServerRoutes, isDev),
release: getSentryRelease(buildId),
dryRun: isDev,
});
Expand Down
2 changes: 2 additions & 0 deletions packages/nextjs/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
export * from './config';
export * from './server';

// __SENTRY_SDK_MULTIPLEXER__
9 changes: 6 additions & 3 deletions packages/nextjs/test/config/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {

export const SERVER_SDK_CONFIG_FILE = 'sentry.server.config.js';
export const CLIENT_SDK_CONFIG_FILE = 'sentry.client.config.js';
export const EDGE_SDK_CONFIG_FILE = 'sentry.edge.config.js';

/** Mock next config object */
export const userNextConfig: NextConfigObject = {
Expand Down Expand Up @@ -43,7 +44,7 @@ export const serverWebpackConfig: WebpackConfigObject = {
'pages/_error': 'private-next-pages/_error.js',
'pages/_app': 'private-next-pages/_app.js',
'pages/sniffTour': ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js'],
'pages/api/_middleware': 'private-next-pages/api/_middleware.js',
middleware: 'private-next-pages/middleware.js',
'pages/api/simulator/dogStats/[name]': { import: 'private-next-pages/api/simulator/dogStats/[name].js' },
'pages/simulator/leaderboard': {
import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'],
Expand Down Expand Up @@ -84,7 +85,7 @@ export const clientWebpackConfig: WebpackConfigObject = {
* @returns A mock build context for the given target
*/
export function getBuildContext(
buildTarget: 'server' | 'client',
buildTarget: 'server' | 'client' | 'edge',
materializedNextConfig: ExportedNextConfig,
webpackVersion: string = '5.4.15',
): BuildContext {
Expand All @@ -101,9 +102,11 @@ export function getBuildContext(
webpack: { version: webpackVersion },
defaultLoaders: true,
totalPages: 2,
isServer: buildTarget === 'server',
isServer: buildTarget === 'server' || buildTarget === 'edge',
nextRuntime: ({ server: 'nodejs', client: undefined, edge: 'edge' } as const)[buildTarget],
};
}

export const serverBuildContext = getBuildContext('server', exportedNextConfig);
export const clientBuildContext = getBuildContext('client', exportedNextConfig);
export const edgeBuildContext = getBuildContext('edge', exportedNextConfig);
8 changes: 6 additions & 2 deletions packages/nextjs/test/config/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@ import * as os from 'os';
import * as path from 'path';
import * as rimraf from 'rimraf';

import { CLIENT_SDK_CONFIG_FILE, SERVER_SDK_CONFIG_FILE } from './fixtures';
import { CLIENT_SDK_CONFIG_FILE, EDGE_SDK_CONFIG_FILE, SERVER_SDK_CONFIG_FILE } from './fixtures';

// We use `fs.existsSync()` in `getUserConfigFile()`. When we're not testing `getUserConfigFile()` specifically, all we
// need is for it to give us any valid answer, so make it always find what it's looking for. Since this is a core node
// built-in, though, which jest itself uses, otherwise let it do the normal thing. Storing the real version of the
// function also lets us restore the original when we do want to test `getUserConfigFile()`.
export const realExistsSync = jest.requireActual('fs').existsSync;
export const mockExistsSync = (path: fs.PathLike): ReturnType<typeof realExistsSync> => {
if ((path as string).endsWith(SERVER_SDK_CONFIG_FILE) || (path as string).endsWith(CLIENT_SDK_CONFIG_FILE)) {
if (
(path as string).endsWith(SERVER_SDK_CONFIG_FILE) ||
(path as string).endsWith(CLIENT_SDK_CONFIG_FILE) ||
(path as string).endsWith(EDGE_SDK_CONFIG_FILE)
) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
CLIENT_SDK_CONFIG_FILE,
clientBuildContext,
clientWebpackConfig,
EDGE_SDK_CONFIG_FILE,
edgeBuildContext,
exportedNextConfig,
SERVER_SDK_CONFIG_FILE,
serverBuildContext,
Expand Down Expand Up @@ -87,6 +89,7 @@ describe('constructWebpackConfigFunction()', () => {
describe('webpack `entry` property config', () => {
const serverConfigFilePath = `./${SERVER_SDK_CONFIG_FILE}`;
const clientConfigFilePath = `./${CLIENT_SDK_CONFIG_FILE}`;
const edgeConfigFilePath = `./${EDGE_SDK_CONFIG_FILE}`;

it('handles various entrypoint shapes', async () => {
const finalWebpackConfig = await materializeFinalWebpackConfig({
Expand Down Expand Up @@ -207,17 +210,16 @@ describe('constructWebpackConfigFunction()', () => {
);
});

it('does not inject user config file into API middleware', async () => {
it('injects user config file into API middleware', async () => {
const finalWebpackConfig = await materializeFinalWebpackConfig({
exportedNextConfig,
incomingWebpackConfig: serverWebpackConfig,
incomingWebpackBuildContext: serverBuildContext,
incomingWebpackBuildContext: edgeBuildContext,
});

expect(finalWebpackConfig.entry).toEqual(
expect.objectContaining({
// no injected file
'pages/api/_middleware': 'private-next-pages/api/_middleware.js',
middleware: [edgeConfigFilePath, 'private-next-pages/middleware.js'],
}),
);
});
Expand Down