Skip to content

Commit

Permalink
ref(nextjs): Inject init code in _app and API routes (#3786)
Browse files Browse the repository at this point in the history 
Change  how we include the user's `sentry.server.config.js` and `sentry.client.config.js` code (which is what calls `Sentry.init()`) in server and browser bundles, in order both to fix apps deployed to Vercel and to eliminate some semi-gross hacks included in the current implementation.

A great deal more detail in the PR description.
  • Loading branch information
@lobsterkatie
lobsterkatie committed Jul 9, 2021
1 parent f964a69 commit 8063fbb
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 161 deletions.
60 changes: 0 additions & 60 deletions packages/nextjs/src/config/utils.ts
View file

This file was deleted.

62 changes: 12 additions & 50 deletions packages/nextjs/src/config/webpack.ts
View file
Original file line number Diff line number Diff 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 Down Expand 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);
}

// 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 Down Expand 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];
} else if (Array.isArray(currentEntryPoint)) {
newEntryPoint = [...currentEntryPoint, filepath];
newEntryPoint = [filepath, ...currentEntryPoint];
}
// descriptor object (webpack 5+)
else if (typeof currentEntryPoint === 'object' && 'import' in currentEntryPoint) {
const currentImportValue = currentEntryPoint.import;
let newImportValue: string | string[];
let newImportValue;

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
Original file line number Diff line number Diff 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 Down Expand 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));
} 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
Original file line number Diff line number Diff 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 Down Expand 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

0 comments on commit 8063fbb

Please sign in to comment.