Skip to content

ref(nextjs): Inject init code in _app and API routes#3786

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.

Sign up for GitHub

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
lobsterkatie merged 3 commits into from
Jul 9, 2021
Merged

ref(nextjs): Inject init code in _app and API routes #3786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3dbdc45
Switch to injecting into API routes and _app for both server and client
lobsterkatie Jul 7, 2021
06879d7
switch injected file location to be first rather than last
lobsterkatie Jul 8, 2021
23c7797
fix tests
lobsterkatie Jul 8, 2021
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
60 changes: 0 additions & 60 deletions packages/nextjs/src/config/utils.ts
View file
Open in desktop

This file was deleted.

62 changes: 12 additions & 50 deletions packages/nextjs/src/config/webpack.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import * as SentryWebpackPlugin from '@sentry/webpack-plugin';

import {
BuildContext,
EntryPointObject,
EntryPointValue,
EntryPropertyObject,
NextConfigObject,
Expand All@@ -13,16 +12,15 @@ import {
WebpackConfigObject,
WebpackEntryProperty,
} from './types';
import { SERVER_SDK_INIT_PATH, storeServerConfigFileLocation } from './utils';

export { SentryWebpackPlugin };

// TODO: merge default SentryWebpackPlugin ignore with their SentryWebpackPlugin ignore or ignoreFile
// TODO: merge default SentryWebpackPlugin include with their SentryWebpackPlugin include
// TODO: drop merged keys from override check? `includeDefaults` option?

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

const defaultSentryWebpackPluginOptions = dropUndefinedKeys({
url: process.env.SENTRY_URL,
Expand DownExpand Up@@ -58,12 +56,6 @@ export function constructWebpackConfigFunction(
const newWebpackFunction = (incomingConfig: WebpackConfigObject, buildContext: BuildContext): WebpackConfigObject => {
let newConfig = { ...incomingConfig };

// if we're building server code, store the webpack output path as an env variable, so we know where to look for the
// webpack-processed version of `sentry.server.config.js` when we need it
if (newConfig.target === 'node') {
storeServerConfigFileLocation(newConfig);
Comment thread
AbhiPrasad marked this conversation as resolved.
}

// if user has custom webpack config (which always takes the form of a function), run it so we have actual values to
// work with
if ('webpack' in userNextConfig && typeof userNextConfig.webpack === 'function') {
Expand DownExpand Up@@ -140,39 +132,11 @@ async function addSentryToEntryProperty(
const newEntryProperty =
typeof currentEntryProperty === 'function' ? await currentEntryProperty() : { ...currentEntryProperty };

// Add a new element to the `entry` array, we force webpack to create a bundle out of the user's
// `sentry.server.config.js` file and output it to `SERVER_INIT_LOCATION`. (See
// https://webpack.js.org/guides/code-splitting/#entry-points.) We do this so that the user's config file is run
// through babel (and any other processors through which next runs the rest of the user-provided code - pages, API
// routes, etc.). Specifically, we need any ESM-style `import` code to get transpiled into ES5, so that we can call
// `require()` on the resulting file when we're instrumenting the sesrver. (We can't use a dynamic import there
// because that then forces the user into a particular TS config.)

// On the server, create a separate bundle, as there's no one entry point depended on by all the others
if (buildContext.isServer) {
// slice off the final `.js` since webpack is going to add it back in for us, and we don't want to end up with
// `.js.js` as the extension
newEntryProperty[SERVER_SDK_INIT_PATH.slice(0, -3)] = SERVER_SDK_CONFIG_FILE;
}
// On the client, it's sufficient to inject it into the `main` JS code, which is included in every browser page.
else {
addFileToExistingEntryPoint(newEntryProperty, 'main', CLIENT_SDK_CONFIG_FILE);

// To work around a bug in nextjs, we need to ensure that the `main.js` entry is empty (otherwise it'll choose that
// over `main` and we'll lose the change we just made). In case some other library has put something into it, copy
// its contents over before emptying it out. See
// https://github.com/getsentry/sentry-javascript/pull/3696#issuecomment-863363803.)
const mainjsValue = newEntryProperty['main.js'];
if (Array.isArray(mainjsValue) && mainjsValue.length > 0) {
const mainValue = newEntryProperty.main;

// copy the `main.js` entries over
newEntryProperty.main = Array.isArray(mainValue)
? [...mainjsValue, ...mainValue]
: { ...(mainValue as EntryPointObject), import: [...mainjsValue, ...(mainValue as EntryPointObject).import] };

// nuke the entries
newEntryProperty['main.js'] = [];
const userConfigFile = buildContext.isServer ? SERVER_SDK_CONFIG_FILE : CLIENT_SDK_CONFIG_FILE;

for (const entryPointName in newEntryProperty) {
if (entryPointName === 'pages/_app' || entryPointName.includes('pages/api')) {
addFileToExistingEntryPoint(newEntryProperty, entryPointName, userConfigFile);
}
}

Expand All@@ -195,22 +159,20 @@ function addFileToExistingEntryPoint(
const currentEntryPoint = entryProperty[entryPointName];
let newEntryPoint: EntryPointValue;

// We inject the user's client config file after the existing code so that the config file has access to
// `publicRuntimeConfig`. See https://github.com/getsentry/sentry-javascript/issues/3485
if (typeof currentEntryPoint === 'string') {
newEntryPoint = [currentEntryPoint, filepath];
newEntryPoint = [filepath, currentEntryPoint];
Comment thread
iker-barriocanal marked this conversation as resolved.
} else if (Array.isArray(currentEntryPoint)) {
newEntryPoint = [...currentEntryPoint, filepath];
newEntryPoint = [filepath, ...currentEntryPoint];
Comment thread
lobsterkatie marked this conversation as resolved.
}
// descriptor object (webpack 5+)
else if (typeof currentEntryPoint === 'object' && 'import' in currentEntryPoint) {
const currentImportValue = currentEntryPoint.import;
let newImportValue: string | string[];
let newImportValue;
Comment thread
lobsterkatie marked this conversation as resolved.

if (typeof currentImportValue === 'string') {
newImportValue = [currentImportValue, filepath];
newImportValue = [filepath, currentImportValue];
} else {
newImportValue = [...currentImportValue, filepath];
newImportValue = [filepath, ...currentImportValue];
}

newEntryPoint = {
Expand Down
13 changes: 0 additions & 13 deletions packages/nextjs/src/utils/instrumentServer.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,7 +5,6 @@ import { fill, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'
import * as domain from 'domain';
import * as http from 'http';
import { default as createNextServer } from 'next';
import * as path from 'path';
import * as querystring from 'querystring';
import * as url from 'url';

Expand DownExpand Up@@ -111,18 +110,6 @@ function makeWrappedHandlerGetter(origHandlerGetter: HandlerGetter): WrappedHand
// Otherwise, it's just a pass-through to the original method.
const wrappedHandlerGetter = async function(this: NextServer): Promise<ReqHandler> {
if (!sdkSetupComplete) {
try {
// `SENTRY_SERVER_INIT_PATH` is set at build time, and points to a webpack-processed version of the user's
// `sentry.server.config.js`. Requiring it starts the SDK.
require(path.resolve(process.env.SENTRY_SERVER_INIT_PATH as string));
Comment thread
lobsterkatie marked this conversation as resolved.
} catch (err) {
// Log the error but don't bail - we still want the wrapping to happen, in case the user is doing something weird
// and manually calling `Sentry.init()` somewhere else. We log to console instead of using logger from utils
// because Sentry is not initialized.
// eslint-disable-next-line no-console
console.error(`[Sentry] Could not initialize SDK. Received error:\n${err}`);
}

// stash this in the closure so that `makeWrappedReqHandler` can use it
liveServer = this.server;
const serverPrototype = Object.getPrototypeOf(liveServer);
Expand Down
102 changes: 64 additions & 38 deletions packages/nextjs/test/config.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,20 +7,14 @@ import {
SentryWebpackPluginOptions,
WebpackConfigObject,
} from '../src/config/types';
import { SENTRY_SERVER_CONFIG_FILE, SERVER_SDK_INIT_PATH } from '../src/config/utils';
import { constructWebpackConfigFunction, SentryWebpackPlugin } from '../src/config/webpack';

// mock `storeServerConfigFileLocation` in order to make it a no-op when necessary
jest.mock('../src/config/utils', () => {
const original = jest.requireActual('../src/config/utils');
return {
...original,
// nuke this so it won't try to look for our dummy paths
storeServerConfigFileLocation: jest.fn(),
};
});
import {
CLIENT_SDK_CONFIG_FILE,
constructWebpackConfigFunction,
SentryWebpackPlugin,
SERVER_SDK_CONFIG_FILE,
} from '../src/config/webpack';

/** mocks of the arguments passed to `withSentryConfig` */
/** Mocks of the arguments passed to `withSentryConfig` */
const userNextConfig = {
publicRuntimeConfig: { location: 'dogpark', activities: ['fetch', 'chasing', 'digging'] },
webpack: (config: WebpackConfigObject, _options: BuildContext) => ({
Expand All@@ -35,19 +29,36 @@ const userNextConfig = {
};
const userSentryWebpackPluginConfig = { org: 'squirrelChasers', project: 'simulator', include: './thirdPartyMaps' };

/** mocks of the arguments passed to the result of `withSentryConfig` (when it's a function) */
const runtimePhase = 'puppy-phase-chew-everything-in-sight';
/** Mocks of the arguments passed to the result of `withSentryConfig` (when it's a function). */
const runtimePhase = 'ball-fetching';
const defaultNextConfig = { nappingHoursPerDay: 20, oversizeFeet: true, shouldChaseTail: true };

/** mocks of the arguments passed to `nextConfig.webpack` */
const serverWebpackConfig = {
entry: () => Promise.resolve({ 'pages/api/dogs/[name]': 'private-next-pages/api/dogs/[name].js' }),
entry: () =>
Promise.resolve({
'pages/api/dogs/[name]': 'private-next-pages/api/dogs/[name].js',
'pages/_app': ['./node_modules/smellOVision/index.js', 'private-next-pages/_app.js'],
'pages/api/simulator/dogStats/[name]': { import: 'private-next-pages/api/simulator/dogStats/[name].js' },
'pages/api/simulator/leaderboard': {
import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/api/simulator/leaderboard.js'],
},
'pages/api/tricks/[trickName]': {
import: 'private-next-pages/api/tricks/[trickName].js',
dependOn: 'treats',
},
treats: './node_modules/dogTreats/treatProvider.js',
}),
output: { filename: '[name].js', path: '/Users/Maisey/projects/squirrelChasingSimulator/.next' },
target: 'node',
context: '/Users/Maisey/projects/squirrelChasingSimulator',
};
const clientWebpackConfig = {
entry: () => Promise.resolve({ main: './src/index.ts' }),
entry: () =>
Promise.resolve({
main: './src/index.ts',
'pages/_app': 'next-client-pages-loader?page=%2F_app',
}),
output: { filename: 'static/chunks/[name].js', path: '/Users/Maisey/projects/squirrelChasingSimulator/.next' },
target: 'web',
context: '/Users/Maisey/projects/squirrelChasingSimulator',
Expand DownExpand Up@@ -212,7 +223,7 @@ describe('webpack config', () => {
});

describe('webpack `entry` property config', () => {
it('injects correct code when building server bundle', async () => {
it('handles various entrypoint shapes', async () => {
const finalWebpackConfig = await materializeFinalWebpackConfig({
userNextConfig,
incomingWebpackConfig: serverWebpackConfig,
Expand All@@ -221,38 +232,53 @@ describe('webpack config', () => {

expect(finalWebpackConfig.entry).toEqual(
expect.objectContaining({
[SERVER_SDK_INIT_PATH.slice(0, -3)]: SENTRY_SERVER_CONFIG_FILE,
// original entry point value is a string
// (was 'private-next-pages/api/dogs/[name].js')
'pages/api/dogs/[name]': [SERVER_SDK_CONFIG_FILE, 'private-next-pages/api/dogs/[name].js'],

// original entry point value is a string array
// (was ['./node_modules/smellOVision/index.js', 'private-next-pages/_app.js'])
'pages/_app': [SERVER_SDK_CONFIG_FILE, './node_modules/smellOVision/index.js', 'private-next-pages/_app.js'],

// original entry point value is an object containing a string `import` value
// (`import` was 'private-next-pages/api/simulator/dogStats/[name].js')
'pages/api/simulator/dogStats/[name]': {
import: [SERVER_SDK_CONFIG_FILE, 'private-next-pages/api/simulator/dogStats/[name].js'],
},

// original entry point value is an object containing a string array `import` value
// (`import` was ['./node_modules/dogPoints/converter.js', 'private-next-pages/api/simulator/leaderboard.js'])
'pages/api/simulator/leaderboard': {
import: [
SERVER_SDK_CONFIG_FILE,
'./node_modules/dogPoints/converter.js',
'private-next-pages/api/simulator/leaderboard.js',
],
},

// original entry point value is an object containg properties besides `import`
// (`dependOn` remains untouched)
'pages/api/tricks/[trickName]': {
import: [SERVER_SDK_CONFIG_FILE, 'private-next-pages/api/tricks/[trickName].js'],
dependOn: 'treats',
},
}),
);
});

it('injects correct code when building client bundle', async () => {
it('does not inject into non-_app, non-API routes', async () => {
const finalWebpackConfig = await materializeFinalWebpackConfig({
userNextConfig,
incomingWebpackConfig: clientWebpackConfig,
incomingWebpackBuildContext: clientBuildContext,
});

expect(finalWebpackConfig.entry).toEqual(
expect.objectContaining({ main: ['./src/index.ts', './sentry.client.config.js'] }),
);
});

// see https://github.com/getsentry/sentry-javascript/pull/3696#issuecomment-863363803
it('handles non-empty `main.js` entry point', async () => {
const finalWebpackConfig = await materializeFinalWebpackConfig({
userNextConfig,
incomingWebpackConfig: {
...clientWebpackConfig,
entry: () => Promise.resolve({ main: './src/index.ts', 'main.js': ['sitLieDownRollOver.config.js'] }),
},
incomingWebpackBuildContext: clientBuildContext,
});

expect(finalWebpackConfig.entry).toEqual(
expect.objectContaining({
main: ['sitLieDownRollOver.config.js', './src/index.ts', './sentry.client.config.js'],
'main.js': [],
// no injected file
main: './src/index.ts',
// was 'next-client-pages-loader?page=%2F_app'
'pages/_app': [CLIENT_SDK_CONFIG_FILE, 'next-client-pages-loader?page=%2F_app'],
}),
);
});
Expand Down