From 462aa6aeeb915a219d4a58fea92592ae219ecadf Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 10 Aug 2023 10:26:08 +0200 Subject: [PATCH 1/5] fix(nextjs): Execute sentry config independently of `autoInstrumentServerFunctions` and `autoInstrumentAppDirectory` --- .../src/config/loaders/wrappingLoader.ts | 31 ++++--- .../templates/sentryInitWrapperTemplate.ts | 7 ++ packages/nextjs/src/config/webpack.ts | 89 ++++++++++++------- 3 files changed, 86 insertions(+), 41 deletions(-) create mode 100644 packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index e4d58c579420..8d5a31a67d69 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -30,6 +30,9 @@ const requestAsyncStorageShimPath = path.resolve(__dirname, '..', 'templates', ' const requestAsyncStorageModuleExists = moduleExists(NEXTJS_REQUEST_ASYNC_STORAGE_MODULE_PATH); let showedMissingAsyncStorageModuleWarning = false; +const sentryInitWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'sentryInitWrapperTemplate.js'); +const sentryInitWrapperTemplateCode = fs.readFileSync(sentryInitWrapperTemplatePath, { encoding: 'utf8' }); + const serverComponentWrapperTemplatePath = path.resolve( __dirname, '..', @@ -43,7 +46,7 @@ type LoaderOptions = { appDir: string; pageExtensionRegex: string; excludeServerRoutes: Array; - wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component'; + wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component' | 'sentry-init'; sentryConfigFilePath?: string; vercelCronsConfig?: VercelCronsConfig; }; @@ -83,7 +86,22 @@ export default function wrappingLoader( let templateCode: string; - if (wrappingTargetKind === 'page' || wrappingTargetKind === 'api-route') { + if (wrappingTargetKind === 'sentry-init') { + templateCode = sentryInitWrapperTemplateCode; + + // Absolute paths to the sentry config do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 + // Se we need check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute. + // Examples where `this.resourcePath` could possibly be non-absolute are virtual modules. + if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { + const sentryConfigImportPath = path + .relative(path.dirname(this.resourcePath), sentryConfigFilePath) + .replace(/\\/g, '/'); + templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, sentryConfigImportPath); + } else { + // Bail without doing any wrapping + this.callback(null, userCode, userModuleSourceMap); + } + } else if (wrappingTargetKind === 'page' || wrappingTargetKind === 'api-route') { // Get the parameterized route name from this page's filepath const parameterizedPagesRoute = path.posix .normalize( @@ -207,15 +225,6 @@ export default function wrappingLoader( throw new Error(`Invariant: Could not get template code of unknown kind "${wrappingTargetKind}"`); } - // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, - // however we can only create relative paths to the sentry config from absolute paths.Examples where this could possibly be non - absolute are virtual modules. - if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { - const sentryConfigImportPath = path - .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 - .replace(/\\/g, '/'); - templateCode = `import "${sentryConfigImportPath}";\n`.concat(templateCode); - } - // Replace the import path of the wrapping target in the template with a path that the `wrapUserCode` function will understand. templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, WRAPPING_TARGET_MODULE_NAME); diff --git a/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts b/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts new file mode 100644 index 000000000000..00bd74e02aaa --- /dev/null +++ b/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts @@ -0,0 +1,7 @@ +// @ts-ignore This will be replaced with the user's sentry config gile +// eslint-disable-next-line import/no-unresolved +import '__SENTRY_CONFIG_IMPORT_PATH__'; + +// @ts-ignore This is the file we're wrapping +// eslint-disable-next-line import/no-unresolved +export * from '__SENTRY_WRAPPING_TARGET_FILE__'; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 49f450bb27a4..b158e72405ab 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -140,19 +140,68 @@ export function constructWebpackConfigFunction( return path.normalize(absoluteResourcePath); }; - if (isServer && userSentryOptions.autoInstrumentServerFunctions !== false) { - // It is very important that we insert our loaders at the beginning of the array because we expect any sort of transformations/transpilations (e.g. TS -> JS) to already have happened. + const isPageResource = (resourcePath: string): boolean => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + return ( + normalizedAbsoluteResourcePath.startsWith(pagesDirPath + path.sep) && + !normalizedAbsoluteResourcePath.startsWith(apiRoutesPath + path.sep) && + dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)) + ); + }; - // Wrap pages + const isApiRouteResource = (resourcePath: string): boolean => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + return ( + normalizedAbsoluteResourcePath.startsWith(apiRoutesPath + path.sep) && + dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)) + ); + }; + + const isMiddlewareResource = (resourcePath: string): boolean => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + return normalizedAbsoluteResourcePath === middlewareJsPath || normalizedAbsoluteResourcePath === middlewareTsPath; + }; + + const isServerComponentResource = (resourcePath: string): boolean => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + + // ".js, .jsx, or .tsx file extensions can be used for Pages" + // https://beta.nextjs.org/docs/routing/pages-and-layouts#pages:~:text=.js%2C%20.jsx%2C%20or%20.tsx%20file%20extensions%20can%20be%20used%20for%20Pages. + return ( + normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && + !!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)\.(js|jsx|tsx)$/) + ); + }; + + if (isServer) { + // Import the Sentry config in every user file newConfig.module.rules.unshift({ test: resourcePath => { - const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); return ( - normalizedAbsoluteResourcePath.startsWith(pagesDirPath + path.sep) && - !normalizedAbsoluteResourcePath.startsWith(apiRoutesPath + path.sep) && - dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)) + isPageResource(resourcePath) || + isApiRouteResource(resourcePath) || + isMiddlewareResource(resourcePath) || + isServerComponentResource(resourcePath) ); }, + use: [ + { + loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), + options: { + ...staticWrappingLoaderOptions, + wrappingTargetKind: 'sentry-init', + }, + }, + ], + }); + } + + if (isServer && userSentryOptions.autoInstrumentServerFunctions !== false) { + // It is very important that we insert our loaders at the beginning of the array because we expect any sort of transformations/transpilations (e.g. TS -> JS) to already have happened. + + // Wrap pages + newConfig.module.rules.unshift({ + test: isPageResource, use: [ { loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), @@ -190,13 +239,7 @@ export function constructWebpackConfigFunction( // Wrap api routes newConfig.module.rules.unshift({ - test: resourcePath => { - const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); - return ( - normalizedAbsoluteResourcePath.startsWith(apiRoutesPath + path.sep) && - dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)) - ); - }, + test: isApiRouteResource, use: [ { loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), @@ -211,12 +254,7 @@ export function constructWebpackConfigFunction( // Wrap middleware newConfig.module.rules.unshift({ - test: resourcePath => { - const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); - return ( - normalizedAbsoluteResourcePath === middlewareJsPath || normalizedAbsoluteResourcePath === middlewareTsPath - ); - }, + test: isMiddlewareResource, use: [ { loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), @@ -232,16 +270,7 @@ export function constructWebpackConfigFunction( if (isServer && userSentryOptions.autoInstrumentAppDirectory !== false) { // Wrap page server components newConfig.module.rules.unshift({ - test: resourcePath => { - const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); - - // ".js, .jsx, or .tsx file extensions can be used for Pages" - // https://beta.nextjs.org/docs/routing/pages-and-layouts#pages:~:text=.js%2C%20.jsx%2C%20or%20.tsx%20file%20extensions%20can%20be%20used%20for%20Pages. - return ( - normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && - !!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)\.(js|jsx|tsx)$/) - ); - }, + test: isServerComponentResource, use: [ { loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), From 4416c32b4d272bbb73dd870957678313cf2b62a5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 10 Aug 2023 10:36:30 +0200 Subject: [PATCH 2/5] . --- packages/nextjs/rollup.npm.config.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index f9c498c8f39e..40f70ab53955 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -24,11 +24,12 @@ export default [ ...makeNPMConfigVariants( makeBaseNPMConfig({ entrypoints: [ - 'src/config/templates/pageWrapperTemplate.ts', 'src/config/templates/apiWrapperTemplate.ts', 'src/config/templates/middlewareWrapperTemplate.ts', - 'src/config/templates/serverComponentWrapperTemplate.ts', + 'src/config/templates/pageWrapperTemplate.ts', 'src/config/templates/requestAsyncStorageShim.ts', + 'src/config/templates/sentryInitWrapperTemplate.ts', + 'src/config/templates/serverComponentWrapperTemplate.ts', ], packageSpecificConfig: { From 7a612f937faca2c6e5a9ea1faf83406a5c83a9fe Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 10 Aug 2023 10:52:05 +0200 Subject: [PATCH 3/5] . --- packages/nextjs/src/config/loaders/wrappingLoader.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 8d5a31a67d69..f2e07ab6537d 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -96,10 +96,11 @@ export default function wrappingLoader( const sentryConfigImportPath = path .relative(path.dirname(this.resourcePath), sentryConfigFilePath) .replace(/\\/g, '/'); - templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, sentryConfigImportPath); + templateCode = templateCode.replace(/__SENTRY_CONFIG_IMPORT_PATH__/g, sentryConfigImportPath); } else { // Bail without doing any wrapping this.callback(null, userCode, userModuleSourceMap); + return; } } else if (wrappingTargetKind === 'page' || wrappingTargetKind === 'api-route') { // Get the parameterized route name from this page's filepath From b8f9114f3400c870d3683fad5c3a1c0edf889693 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 10 Aug 2023 09:34:00 +0000 Subject: [PATCH 4/5] . --- packages/nextjs/rollup.npm.config.js | 1 + .../src/config/loaders/wrappingLoader.ts | 3 ++ .../templates/sentryInitWrapperTemplate.ts | 4 ++ packages/nextjs/src/config/webpack.ts | 46 +++++++++---------- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index 40f70ab53955..ce15b951235e 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -48,6 +48,7 @@ export default [ external: [ '@sentry/nextjs', 'next/dist/client/components/request-async-storage', + '__SENTRY_CONFIG_IMPORT_PATH__', '__SENTRY_WRAPPING_TARGET_FILE__', '__SENTRY_NEXTJS_REQUEST_ASYNC_STORAGE_SHIM__', ], diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index f2e07ab6537d..f210435fc653 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -233,6 +233,9 @@ export default function wrappingLoader( // individual exports (which nextjs seems to require). wrapUserCode(templateCode, userCode, userModuleSourceMap) .then(({ code: wrappedCode, map: wrappedCodeSourceMap }) => { + if (wrappingTargetKind === 'sentry-init') { + console.log({ wrappedCode }); + } this.callback(null, wrappedCode, wrappedCodeSourceMap); }) .catch(err => { diff --git a/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts b/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts index 00bd74e02aaa..1720c3b62672 100644 --- a/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/sentryInitWrapperTemplate.ts @@ -5,3 +5,7 @@ import '__SENTRY_CONFIG_IMPORT_PATH__'; // @ts-ignore This is the file we're wrapping // eslint-disable-next-line import/no-unresolved export * from '__SENTRY_WRAPPING_TARGET_FILE__'; + +// @ts-ignore This is the file we're wrapping +// eslint-disable-next-line import/no-unresolved +export { default } from '__SENTRY_WRAPPING_TARGET_FILE__'; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index b158e72405ab..d711fef5c14f 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -173,29 +173,6 @@ export function constructWebpackConfigFunction( ); }; - if (isServer) { - // Import the Sentry config in every user file - newConfig.module.rules.unshift({ - test: resourcePath => { - return ( - isPageResource(resourcePath) || - isApiRouteResource(resourcePath) || - isMiddlewareResource(resourcePath) || - isServerComponentResource(resourcePath) - ); - }, - use: [ - { - loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), - options: { - ...staticWrappingLoaderOptions, - wrappingTargetKind: 'sentry-init', - }, - }, - ], - }); - } - if (isServer && userSentryOptions.autoInstrumentServerFunctions !== false) { // It is very important that we insert our loaders at the beginning of the array because we expect any sort of transformations/transpilations (e.g. TS -> JS) to already have happened. @@ -283,6 +260,29 @@ export function constructWebpackConfigFunction( }); } + if (isServer) { + // Import the Sentry config in every user file + newConfig.module.rules.unshift({ + test: resourcePath => { + return ( + isPageResource(resourcePath) || + isApiRouteResource(resourcePath) || + isMiddlewareResource(resourcePath) || + isServerComponentResource(resourcePath) + ); + }, + use: [ + { + loader: path.resolve(__dirname, 'loaders', 'wrappingLoader.js'), + options: { + ...staticWrappingLoaderOptions, + wrappingTargetKind: 'sentry-init', + }, + }, + ], + }); + } + // The SDK uses syntax (ES6 and ES6+ features like object spread) which isn't supported by older browsers. For users // who want to support such browsers, `transpileClientSDK` allows them to force the SDK code to go through the same // transpilation that their code goes through. We don't turn this on by default because it increases bundle size From 450fc30b821eee7215d817f41bb59b2f789af0e5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 10 Aug 2023 09:53:29 +0000 Subject: [PATCH 5/5] . --- packages/nextjs/src/config/loaders/wrappingLoader.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index f210435fc653..f2e07ab6537d 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -233,9 +233,6 @@ export default function wrappingLoader( // individual exports (which nextjs seems to require). wrapUserCode(templateCode, userCode, userModuleSourceMap) .then(({ code: wrappedCode, map: wrappedCodeSourceMap }) => { - if (wrappingTargetKind === 'sentry-init') { - console.log({ wrappedCode }); - } this.callback(null, wrappedCode, wrappedCodeSourceMap); }) .catch(err => {