From 379e99dbb0f920396e398b95b96b77fb2e1ac595 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:04:23 -0700 Subject: [PATCH 01/16] pull config file location constants into webpack file --- packages/nextjs/src/config/webpack.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 5b4e0dd58012..8617197101ac 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -13,8 +13,6 @@ import { WebpackEntryProperty, } from './types'; import { - SENTRY_CLIENT_CONFIG_FILE, - SENTRY_SERVER_CONFIG_FILE, SERVER_SDK_INIT_PATH, storeServerConfigFileLocation, } from './utils'; @@ -25,6 +23,9 @@ export { SentryWebpackPlugin }; // 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'; + const defaultSentryWebpackPluginOptions = dropUndefinedKeys({ url: process.env.SENTRY_URL, org: process.env.SENTRY_ORG, @@ -155,11 +156,11 @@ async function addSentryToEntryProperty( if (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)] = SENTRY_SERVER_CONFIG_FILE; + 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', SENTRY_CLIENT_CONFIG_FILE); + 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 From 58eb07363f9e8e0d1a62fbaef72920414441a6e0 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:11:06 -0700 Subject: [PATCH 02/16] remove obsolete guard against entry point not existing --- packages/nextjs/src/config/webpack.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 8617197101ac..202a26de2149 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -198,13 +198,6 @@ function addFileToExistingEntryPoint( // can be a string, array of strings, or object whose `import` property is one of those two let injectedInto = entryProperty[entryPointName]; - // Sometimes especially for older next.js versions it happens we don't have an entry point - if (!injectedInto) { - // eslint-disable-next-line no-console - console.error(`[Sentry] Can't inject ${filepath}, no entrypoint is defined.`); - return; - } - // 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 injectedInto === 'string') { From b3937fa0165e8c8748fffd0bb7dc7f0f0c503d0b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:15:04 -0700 Subject: [PATCH 03/16] s/options/buildContext in webpack function --- packages/nextjs/src/config/webpack.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 202a26de2149..f3e62d79f4aa 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -54,7 +54,7 @@ export function constructWebpackConfigFunction( userNextConfig: NextConfigObject = {}, userSentryWebpackPluginOptions: Partial = {}, ): WebpackConfigFunction { - const newWebpackFunction = (config: WebpackConfigObject, options: BuildContext): WebpackConfigObject => { + const newWebpackFunction = (config: WebpackConfigObject, buildContext: BuildContext): WebpackConfigObject => { // clone to avoid mutability issues let newConfig = { ...config }; @@ -67,7 +67,7 @@ export function constructWebpackConfigFunction( // 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') { - newConfig = userNextConfig.webpack(newConfig, options); + newConfig = userNextConfig.webpack(newConfig, buildContext); } // Tell webpack to inject user config files (containing the two `Sentry.init()` calls) into the appropriate output @@ -79,10 +79,10 @@ 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, options.isServer); + newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, buildContext.isServer); // Enable the Sentry plugin (which uploads source maps to Sentry when not in dev) by default - const enableWebpackPlugin = options.isServer + const enableWebpackPlugin = buildContext.isServer ? !userNextConfig.sentry?.disableServerWebpackPlugin : !userNextConfig.sentry?.disableClientWebpackPlugin; @@ -93,7 +93,7 @@ export function constructWebpackConfigFunction( // Next doesn't let you change this is dev even if you want to - see // https://github.com/vercel/next.js/blob/master/errors/improper-devtool.md - if (!options.dev) { + if (!buildContext.dev) { newConfig.devtool = 'source-map'; } @@ -104,8 +104,8 @@ export function constructWebpackConfigFunction( // @ts-ignore Our types for the plugin are messed up somehow - TS wants this to be `SentryWebpackPlugin.default`, // but that's not actually a thing new SentryWebpackPlugin({ - dryRun: options.dev, - release: getSentryRelease(options.buildId), + dryRun: buildContext.dev, + release: getSentryRelease(buildContext.buildId), ...defaultSentryWebpackPluginOptions, ...userSentryWebpackPluginOptions, }), From 5fba7dba890836e5acab40c4b1e6c02aaccd82a9 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:18:49 -0700 Subject: [PATCH 04/16] s/config/incomingConfig in webpack function --- packages/nextjs/src/config/webpack.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index f3e62d79f4aa..48f07ea0541f 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -54,9 +54,12 @@ export function constructWebpackConfigFunction( userNextConfig: NextConfigObject = {}, userSentryWebpackPluginOptions: Partial = {}, ): WebpackConfigFunction { - const newWebpackFunction = (config: WebpackConfigObject, buildContext: BuildContext): WebpackConfigObject => { + // Will be called by nextjs and passed its default webpack configuration and context data about the build (whether + // we're building server or client, whether we're in dev, what version of webpack we're using, etc). Note that + // `currentWebpackConfig` and `buildContext` are referred to as `config` and `options` in the nextjs docs. + const newWebpackFunction = (incomingConfig: WebpackConfigObject, buildContext: BuildContext): WebpackConfigObject => { // clone to avoid mutability issues - let newConfig = { ...config }; + 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 From b4302874c6d943ebd95bb3077274ec80caa85ae8 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:21:54 -0700 Subject: [PATCH 05/16] s/importVal/newImportValue --- packages/nextjs/src/config/webpack.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 48f07ea0541f..59344b9bf7f6 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -208,17 +208,17 @@ function addFileToExistingEntryPoint( } else if (Array.isArray(injectedInto)) { injectedInto = [...injectedInto, filepath]; } else { - let importVal: string | string[]; + let newImportValue: string | string[]; if (typeof injectedInto.import === 'string') { - importVal = [injectedInto.import, filepath]; + newImportValue = [injectedInto.import, filepath]; } else { - importVal = [...injectedInto.import, filepath]; + newImportValue = [...injectedInto.import, filepath]; } injectedInto = { ...injectedInto, - import: importVal, + import: newImportValue, }; } From 0854b10705b3131c77f95a6d5dd1218c41102517 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:24:52 -0700 Subject: [PATCH 06/16] s/injectedInto/newEntryPoint --- packages/nextjs/src/config/webpack.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 59344b9bf7f6..e1c5b0078430 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -199,30 +199,30 @@ function addFileToExistingEntryPoint( filepath: string, ): void { // can be a string, array of strings, or object whose `import` property is one of those two - let injectedInto = entryProperty[entryPointName]; + let newEntryPoint = entryProperty[entryPointName]; // 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 injectedInto === 'string') { - injectedInto = [injectedInto, filepath]; - } else if (Array.isArray(injectedInto)) { - injectedInto = [...injectedInto, filepath]; + if (typeof newEntryPoint === 'string') { + newEntryPoint = [newEntryPoint, filepath]; + } else if (Array.isArray(newEntryPoint)) { + newEntryPoint = [...newEntryPoint, filepath]; } else { let newImportValue: string | string[]; - if (typeof injectedInto.import === 'string') { - newImportValue = [injectedInto.import, filepath]; + if (typeof newEntryPoint.import === 'string') { + newImportValue = [newEntryPoint.import, filepath]; } else { - newImportValue = [...injectedInto.import, filepath]; + newImportValue = [...newEntryPoint.import, filepath]; } - injectedInto = { - ...injectedInto, + newEntryPoint = { + ...newEntryPoint, import: newImportValue, }; } - entryProperty[entryPointName] = injectedInto; + entryProperty[entryPointName] = newEntryPoint; } /** From ad0d974afd98c5b55cce1928405574d52e37044b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:35:07 -0700 Subject: [PATCH 07/16] pass full build context --- packages/nextjs/src/config/webpack.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index e1c5b0078430..376959b31103 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -82,7 +82,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.isServer); + newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, buildContext); // Enable the Sentry plugin (which uploads source maps to Sentry when not in dev) by default const enableWebpackPlugin = buildContext.isServer @@ -126,14 +126,13 @@ export function constructWebpackConfigFunction( * included in the the necessary bundles. * * @param origEntryProperty The value of the property before Sentry code has been injected - * @param isServer A boolean provided by nextjs indicating whether we're handling the server bundles or the browser - * bundles + * @param buildContext Object passed by nextjs containing metadata about the build * @returns The value which the new `entry` property (which will be a function) will return (TODO: this should return * the function, rather than the function's return value) */ async function addSentryToEntryProperty( origEntryProperty: WebpackEntryProperty, - isServer: boolean, + buildContext: BuildContext, ): Promise { // 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 @@ -156,7 +155,7 @@ async function addSentryToEntryProperty( // 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 (isServer) { + 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; From ed44187ffca947ba1d805946c52f4b9d6993456c Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:42:22 -0700 Subject: [PATCH 08/16] s/origEntryProperty/currentEntryProperty --- packages/nextjs/src/config/webpack.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 376959b31103..497f9f4a7d0a 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -125,13 +125,13 @@ export function constructWebpackConfigFunction( * Modify the webpack `entry` property so that the code in `sentry.server.config.js` and `sentry.client.config.js` is * included in the the necessary bundles. * - * @param origEntryProperty The value of the property before Sentry code has been injected + * @param currentEntryProperty The value of the property before Sentry code has been injected * @param buildContext Object passed by nextjs containing metadata about the build * @returns The value which the new `entry` property (which will be a function) will return (TODO: this should return * the function, rather than the function's return value) */ async function addSentryToEntryProperty( - origEntryProperty: WebpackEntryProperty, + currentEntryProperty: WebpackEntryProperty, buildContext: BuildContext, ): Promise { // The `entry` entry in a webpack config can be a string, array of strings, object, or function. By default, nextjs @@ -140,9 +140,9 @@ 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. - let newEntryProperty = origEntryProperty; - if (typeof origEntryProperty === 'function') { - newEntryProperty = await origEntryProperty(); + let newEntryProperty = currentEntryProperty; + if (typeof currentEntryProperty === 'function') { + newEntryProperty = await currentEntryProperty(); } newEntryProperty = newEntryProperty as EntryPropertyObject; From 8bdab4a70f16f6929f559caf36eafcd2924ae4da Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:51:45 -0700 Subject: [PATCH 09/16] store current import value --- packages/nextjs/src/config/webpack.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 497f9f4a7d0a..a46a47a7c65a 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -207,12 +207,13 @@ function addFileToExistingEntryPoint( } else if (Array.isArray(newEntryPoint)) { newEntryPoint = [...newEntryPoint, filepath]; } else { + const currentImportValue = newEntryPoint.import; let newImportValue: string | string[]; - if (typeof newEntryPoint.import === 'string') { - newImportValue = [newEntryPoint.import, filepath]; + if (typeof currentImportValue === 'string') { + newImportValue = [currentImportValue, filepath]; } else { - newImportValue = [...newEntryPoint.import, filepath]; + newImportValue = [...currentImportValue, filepath]; } newEntryPoint = { From 39aadb22bc03b98536df8339487cd3a4716d4b97 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:58:30 -0700 Subject: [PATCH 10/16] separate current and new entry points --- packages/nextjs/src/config/webpack.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index a46a47a7c65a..68d1e5deaa0c 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -5,6 +5,7 @@ import * as SentryWebpackPlugin from '@sentry/webpack-plugin'; import { BuildContext, EntryPointObject, + EntryPointValue, EntryPropertyObject, NextConfigObject, SentryWebpackPluginOptions, @@ -198,16 +199,17 @@ function addFileToExistingEntryPoint( filepath: string, ): void { // can be a string, array of strings, or object whose `import` property is one of those two - let newEntryPoint = entryProperty[entryPointName]; + 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 newEntryPoint === 'string') { - newEntryPoint = [newEntryPoint, filepath]; - } else if (Array.isArray(newEntryPoint)) { - newEntryPoint = [...newEntryPoint, filepath]; + if (typeof currentEntryPoint === 'string') { + newEntryPoint = [currentEntryPoint, filepath]; + } else if (Array.isArray(currentEntryPoint)) { + newEntryPoint = [...currentEntryPoint, filepath]; } else { - const currentImportValue = newEntryPoint.import; + const currentImportValue = currentEntryPoint.import; let newImportValue: string | string[]; if (typeof currentImportValue === 'string') { @@ -217,7 +219,7 @@ function addFileToExistingEntryPoint( } newEntryPoint = { - ...newEntryPoint, + ...currentEntryPoint, import: newImportValue, }; } From d6f04f5d1e1dd9e19a146ddd61457352abfd44d9 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 14:02:06 -0700 Subject: [PATCH 11/16] split EntryPointValue type out --- packages/nextjs/src/config/types.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/nextjs/src/config/types.ts b/packages/nextjs/src/config/types.ts index c2164fc37526..2fabbd856b91 100644 --- a/packages/nextjs/src/config/types.ts +++ b/packages/nextjs/src/config/types.ts @@ -52,18 +52,14 @@ export type BuildContext = { dev: boolean; isServer: boolean; buildId: string }; // For our purposes, the value for `entry` is either an object, or an async function which returns such an object export type WebpackEntryProperty = EntryPropertyObject | EntryPropertyFunction; -// Each value in that object is either a string representing a single entry point, an array of such strings, or an -// object containing either of those, along with other configuration options. In that third case, the entry point(s) are -// listed under the key `import`. export type EntryPropertyObject = { - [key: string]: - | string - | Array - // only in webpack 5 - | EntryPointObject; + [key: string]: EntryPointValue; }; export type EntryPropertyFunction = () => Promise; -// An object with options for a single entry point, potentially one of many in the webpack `entry` property +// Each value in that object is either a string representing a single entry point, an array of such strings, or an +// object containing either of those, along with other configuration options. In that third case, the entry point(s) are +// listed under the key `import`. +export type EntryPointValue = string | Array | EntryPointObject; export type EntryPointObject = { import: string | Array }; From 32df84ffd144e097f6725f7302ed221b2dd4f97c Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 15:01:03 -0700 Subject: [PATCH 12/16] fix comments --- packages/nextjs/src/config/webpack.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 68d1e5deaa0c..a112c981b44e 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -59,7 +59,6 @@ export function constructWebpackConfigFunction( // we're building server or client, whether we're in dev, what version of webpack we're using, etc). Note that // `currentWebpackConfig` and `buildContext` are referred to as `config` and `options` in the nextjs docs. const newWebpackFunction = (incomingConfig: WebpackConfigObject, buildContext: BuildContext): WebpackConfigObject => { - // clone to avoid mutability issues 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 @@ -208,7 +207,9 @@ function addFileToExistingEntryPoint( newEntryPoint = [currentEntryPoint, filepath]; } else if (Array.isArray(currentEntryPoint)) { newEntryPoint = [...currentEntryPoint, filepath]; - } else { + } + // descriptor object (webpack 5+) + else { const currentImportValue = currentEntryPoint.import; let newImportValue: string | string[]; From bf575a8bfcb05171e95c731ea657e0a6a1df0964 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 15:05:01 -0700 Subject: [PATCH 13/16] clean up newEntryProperty --- packages/nextjs/src/config/webpack.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index a112c981b44e..0b88a79d7a4a 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -140,11 +140,8 @@ 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. - let newEntryProperty = currentEntryProperty; - if (typeof currentEntryProperty === 'function') { - newEntryProperty = await currentEntryProperty(); - } - newEntryProperty = newEntryProperty as EntryPropertyObject; + 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 From fc0ad69cd4ca7f1cb577234628bf511339c63929 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 15:09:24 -0700 Subject: [PATCH 14/16] clean up copy pasta in handlers.ts --- packages/nextjs/src/utils/handlers.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/packages/nextjs/src/utils/handlers.ts b/packages/nextjs/src/utils/handlers.ts index ecb5103c6db6..dda7c63e936a 100644 --- a/packages/nextjs/src/utils/handlers.ts +++ b/packages/nextjs/src/utils/handlers.ts @@ -20,7 +20,6 @@ export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { if (currentScope) { currentScope.addEventProcessor(event => addRequestDataToEvent(event, req as NextRequest)); - // We only want to record page and API requests if (hasTracingEnabled()) { // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) let traceparentData; @@ -34,20 +33,18 @@ export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { let reqPath = stripUrlQueryAndFragment(url); // Replace with placeholder if (req.query) { + // TODO get this from next if possible, to avoid accidentally replacing non-dynamic parts of the path if + // they match dynamic parts for (const [key, value] of Object.entries(req.query)) { reqPath = reqPath.replace(`${value}`, `[${key}]`); } } - - // requests for pages will only ever be GET requests, so don't bother to include the method in the transaction - // name; requests to API routes could be GET, POST, PUT, etc, so do include it there - const namePrefix = `${(req.method || 'GET').toUpperCase()} `; + const reqMethod = `${(req.method || 'GET').toUpperCase()} `; const transaction = startTransaction( { - name: `${namePrefix}${reqPath}`, + name: `${reqMethod}${reqPath}`, op: 'http.server', - metadata: { requestPath: reqPath }, ...traceparentData, }, // extra context passed to the `tracesSampler` @@ -57,7 +54,7 @@ export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { } } - return await handler(req, res); // Call Handler + return await handler(req, res); // Call original handler } catch (e) { withScope(scope => { scope.addEventProcessor(event => { @@ -74,10 +71,6 @@ export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { if (transaction) { transaction.setHttpStatus(res.statusCode); - // we'll collect this data in a more targeted way in the event processor we added above, - // `addRequestDataToEvent` - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - delete transaction.metadata.requestPath; transaction.finish(); } From 1f5727115abebfc2b66f8f73544b2c4f89662914 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Jul 2021 15:32:42 -0700 Subject: [PATCH 15/16] make prettier happy --- packages/nextjs/src/config/webpack.ts | 5 +---- packages/nextjs/src/utils/handlers.ts | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 0b88a79d7a4a..c03e8abf8d83 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -13,10 +13,7 @@ import { WebpackConfigObject, WebpackEntryProperty, } from './types'; -import { - SERVER_SDK_INIT_PATH, - storeServerConfigFileLocation, -} from './utils'; +import { SERVER_SDK_INIT_PATH, storeServerConfigFileLocation } from './utils'; export { SentryWebpackPlugin }; diff --git a/packages/nextjs/src/utils/handlers.ts b/packages/nextjs/src/utils/handlers.ts index dda7c63e936a..9b86e7bdd5e4 100644 --- a/packages/nextjs/src/utils/handlers.ts +++ b/packages/nextjs/src/utils/handlers.ts @@ -71,7 +71,6 @@ export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { if (transaction) { transaction.setHttpStatus(res.statusCode); - transaction.finish(); } try { From 736204261beb352746496cc1e744d7d33b3d1d15 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 8 Jul 2021 09:51:17 -0700 Subject: [PATCH 16/16] code review changes --- packages/nextjs/src/config/webpack.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index c03e8abf8d83..47d4b253bac7 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -54,7 +54,7 @@ export function constructWebpackConfigFunction( ): WebpackConfigFunction { // Will be called by nextjs and passed its default webpack configuration and context data about the build (whether // we're building server or client, whether we're in dev, what version of webpack we're using, etc). Note that - // `currentWebpackConfig` and `buildContext` are referred to as `config` and `options` in the nextjs docs. + // `incomingConfig` and `buildContext` are referred to as `config` and `options` in the nextjs docs. const newWebpackFunction = (incomingConfig: WebpackConfigObject, buildContext: BuildContext): WebpackConfigObject => { let newConfig = { ...incomingConfig }; @@ -203,7 +203,7 @@ function addFileToExistingEntryPoint( newEntryPoint = [...currentEntryPoint, filepath]; } // descriptor object (webpack 5+) - else { + else if (typeof currentEntryPoint === 'object' && 'import' in currentEntryPoint) { const currentImportValue = currentEntryPoint.import; let newImportValue: string | string[]; @@ -217,6 +217,16 @@ function addFileToExistingEntryPoint( ...currentEntryPoint, import: newImportValue, }; + } else { + // mimic the logger prefix in order to use `console.warn` (which will always be printed, regardless of SDK settings) + // eslint-disable-next-line no-console + console.error( + 'Sentry Logger [Error]:', + `Could not inject SDK initialization code into entry point ${entryPointName}, as it is not a recognized format.\n`, + `Expected: string | Array | { [key:string]: any, import: string | Array }\n`, + `Got: ${currentEntryPoint}`, + ); + return; } entryProperty[entryPointName] = newEntryPoint;