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 }; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 5b4e0dd58012..47d4b253bac7 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, @@ -12,12 +13,7 @@ import { WebpackConfigObject, WebpackEntryProperty, } from './types'; -import { - SENTRY_CLIENT_CONFIG_FILE, - SENTRY_SERVER_CONFIG_FILE, - SERVER_SDK_INIT_PATH, - storeServerConfigFileLocation, -} from './utils'; +import { SERVER_SDK_INIT_PATH, storeServerConfigFileLocation } from './utils'; export { SentryWebpackPlugin }; @@ -25,6 +21,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, @@ -53,9 +52,11 @@ export function constructWebpackConfigFunction( userNextConfig: NextConfigObject = {}, userSentryWebpackPluginOptions: Partial = {}, ): WebpackConfigFunction { - const newWebpackFunction = (config: WebpackConfigObject, options: BuildContext): WebpackConfigObject => { - // clone to avoid mutability issues - let newConfig = { ...config }; + // 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 + // `incomingConfig` and `buildContext` are referred to as `config` and `options` in the nextjs docs. + 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 @@ -66,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 @@ -78,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); // 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; @@ -92,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'; } @@ -103,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, }), @@ -121,15 +122,14 @@ 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 isServer A boolean provided by nextjs indicating whether we're handling the server bundles or the browser - * bundles + * @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, - isServer: boolean, + 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 // sets it to an async function which returns the promise of an object of string arrays. Because we don't know whether @@ -137,11 +137,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 = origEntryProperty; - if (typeof origEntryProperty === 'function') { - newEntryProperty = await origEntryProperty(); - } - 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 @@ -152,14 +149,14 @@ 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)] = 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 @@ -195,37 +192,44 @@ 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]; - - // 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; - } + 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 injectedInto === 'string') { - injectedInto = [injectedInto, filepath]; - } else if (Array.isArray(injectedInto)) { - injectedInto = [...injectedInto, filepath]; - } else { - let importVal: string | string[]; + if (typeof currentEntryPoint === 'string') { + newEntryPoint = [currentEntryPoint, filepath]; + } else if (Array.isArray(currentEntryPoint)) { + newEntryPoint = [...currentEntryPoint, filepath]; + } + // descriptor object (webpack 5+) + else if (typeof currentEntryPoint === 'object' && 'import' in currentEntryPoint) { + const currentImportValue = currentEntryPoint.import; + let newImportValue: string | string[]; - if (typeof injectedInto.import === 'string') { - importVal = [injectedInto.import, filepath]; + if (typeof currentImportValue === 'string') { + newImportValue = [currentImportValue, filepath]; } else { - importVal = [...injectedInto.import, filepath]; + newImportValue = [...currentImportValue, filepath]; } - injectedInto = { - ...injectedInto, - import: importVal, + newEntryPoint = { + ...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] = injectedInto; + entryProperty[entryPointName] = newEntryPoint; } /** diff --git a/packages/nextjs/src/utils/handlers.ts b/packages/nextjs/src/utils/handlers.ts index ecb5103c6db6..9b86e7bdd5e4 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,11 +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(); } try {