From 8e96a2138c13d90cc42cdd4d8befce8eaa84b805 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 29 Sep 2022 14:27:39 -0700 Subject: [PATCH 1/4] add `NEXT_PHASE` check to `isBuild()` --- packages/nextjs/src/utils/isBuild.ts | 30 +++++++++++++++++----------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/nextjs/src/utils/isBuild.ts b/packages/nextjs/src/utils/isBuild.ts index 2e849807d8dd..78d0a4edb858 100644 --- a/packages/nextjs/src/utils/isBuild.ts +++ b/packages/nextjs/src/utils/isBuild.ts @@ -2,18 +2,24 @@ * Decide if the currently running process is part of the build phase or happening at runtime. */ export function isBuild(): boolean { - // During build, the main process is invoked by - // `node next build` - // and child processes are invoked as - // `node /node_modules/.../jest-worker/processChild.js`. - // The former is (obviously) easy to recognize, but the latter could happen at runtime as well. Fortunately, the main - // process hits this file before any of the child processes do, so we're able to set an env variable which the child - // processes can then check. During runtime, the main process is invoked as - // `node next start` - // or - // `node /var/runtime/index.js`, - // so we never drop into the `if` in the first place. - if (process.argv.includes('build') || process.env.SENTRY_BUILD_PHASE) { + if ( + // During build, the main process is invoked by + // `node next build` + // and child processes are invoked as + // `node /node_modules/.../jest-worker/processChild.js`. + // The former is (obviously) easy to recognize, but the latter could happen at runtime as well. Fortunately, the main + // process hits this file before any of the child processes do, so we're able to set an env variable which the child + // processes can then check. During runtime, the main process is invoked as + // `node next start` + // or + // `node /var/runtime/index.js`, + // so we never drop into the `if` in the first place. + process.argv.includes('build') || + process.env.SENTRY_BUILD_PHASE || + // This is set by next, but not until partway through the build process, which is why we need the above checks. That + // said, in case this function isn't called until we're in a child process, it can serve as a good backup. + process.env.NEXT_PHASE === 'phase-production-build' + ) { process.env.SENTRY_BUILD_PHASE = 'true'; return true; } From 1b19c6b771d3410c894409e32010269ec325b728 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 29 Sep 2022 14:29:44 -0700 Subject: [PATCH 2/4] store `isBuild()` result as exported constant --- packages/nextjs/src/index.server.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index 56677e44c42a..f1cec1af27b2 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -22,6 +22,13 @@ export { ErrorBoundary, showReportDialog, withErrorBoundary } from '@sentry/reac type GlobalWithDistDir = typeof global & { __rewriteFramesDistDir__: string }; const domain = domainModule as typeof domainModule & { active: (domainModule.Domain & Carrier) | null }; +// Exporting this constant means we can compute it without the linter complaining, even if we stop directly using it in +// this file. It's important that it be computed as early as possible, because one of its indicators is seeing 'build' +// (as in the CLI command `next build`) in `process.argv`. Later on in the build process, everything's been spun out +// into child threads and `argv` turns into ['node', 'path/to/childProcess.js'], so the original indicator is lost. We +// thus want to compute it as soon as the SDK is loaded for the first time, which is normally when the user imports +// `withSentryConfig` into `next.config.js`. +export const IS_BUILD_PHASE = isBuild(); const isVercel = !!process.env.VERCEL; /** Inits the Sentry NextJS SDK on node. */ @@ -142,7 +149,7 @@ export { // deployments, because the current method of doing the wrapping a) crashes Next 12 apps deployed to Vercel and // b) doesn't work on those apps anyway. We also don't do it during build, because there's no server running in that // phase.) -if (!isBuild() && !isVercel) { +if (!IS_BUILD_PHASE && !isVercel) { // Dynamically require the file because even importing from it causes Next 12 to crash on Vercel. // In environments where the JS file doesn't exist, such as testing, import the TS file. try { From af56f87b4747b97f619dc9b84e4c308fe6d17723 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 29 Sep 2022 14:30:47 -0700 Subject: [PATCH 3/4] s/isVercel/IS_VERCEL --- packages/nextjs/src/index.server.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index f1cec1af27b2..92fd4cd55b37 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -29,7 +29,7 @@ const domain = domainModule as typeof domainModule & { active: (domainModule.Dom // thus want to compute it as soon as the SDK is loaded for the first time, which is normally when the user imports // `withSentryConfig` into `next.config.js`. export const IS_BUILD_PHASE = isBuild(); -const isVercel = !!process.env.VERCEL; +const IS_VERCEL = !!process.env.VERCEL; /** Inits the Sentry NextJS SDK on node. */ export function init(options: NextjsOptions): void { @@ -70,7 +70,7 @@ export function init(options: NextjsOptions): void { configureScope(scope => { scope.setTag('runtime', 'node'); - if (isVercel) { + if (IS_VERCEL) { scope.setTag('vercel', true); } @@ -149,7 +149,7 @@ export { // deployments, because the current method of doing the wrapping a) crashes Next 12 apps deployed to Vercel and // b) doesn't work on those apps anyway. We also don't do it during build, because there's no server running in that // phase.) -if (!IS_BUILD_PHASE && !isVercel) { +if (!IS_BUILD_PHASE && !IS_VERCEL) { // Dynamically require the file because even importing from it causes Next 12 to crash on Vercel. // In environments where the JS file doesn't exist, such as testing, import the TS file. try { From 2d365025df93d91c54c513e2765fa0ca327ae450 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 29 Sep 2022 14:37:50 -0700 Subject: [PATCH 4/4] deprecate exported `isBuild()` --- packages/nextjs/src/index.server.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index 92fd4cd55b37..ffd989510b11 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -28,7 +28,7 @@ const domain = domainModule as typeof domainModule & { active: (domainModule.Dom // into child threads and `argv` turns into ['node', 'path/to/childProcess.js'], so the original indicator is lost. We // thus want to compute it as soon as the SDK is loaded for the first time, which is normally when the user imports // `withSentryConfig` into `next.config.js`. -export const IS_BUILD_PHASE = isBuild(); +export const IS_BUILD = isBuild(); const IS_VERCEL = !!process.env.VERCEL; /** Inits the Sentry NextJS SDK on node. */ @@ -131,9 +131,16 @@ function addServerIntegrations(options: NextjsOptions): void { options.integrations = integrations; } +// TODO (v8): Remove this +/** + * @deprecated Use the constant `IS_BUILD` instead. + */ +const deprecatedIsBuild = (): boolean => isBuild(); +// eslint-disable-next-line deprecation/deprecation +export { deprecatedIsBuild as isBuild }; + export type { SentryWebpackPluginOptions } from './config/types'; export { withSentryConfig } from './config'; -export { isBuild } from './utils/isBuild'; export { withSentryGetServerSideProps, withSentryGetStaticProps, @@ -149,7 +156,7 @@ export { // deployments, because the current method of doing the wrapping a) crashes Next 12 apps deployed to Vercel and // b) doesn't work on those apps anyway. We also don't do it during build, because there's no server running in that // phase.) -if (!IS_BUILD_PHASE && !IS_VERCEL) { +if (!IS_BUILD && !IS_VERCEL) { // Dynamically require the file because even importing from it causes Next 12 to crash on Vercel. // In environments where the JS file doesn't exist, such as testing, import the TS file. try {