ref(nextjs): Make build-phase check more robust (#5857)

This makes a few changes to the `isBuild` utility function in the nextjs SDK in order to make its operation less brittle. Currently, it relies on being called at the top level of `index.server.ts`, which ensures that it runs early enough in the build process to be executed in the main thread, before operation has been split out into child processes. (This is important because child processes don't conveniently have the word 'build' in their invocations the way the main thread (invoked by `next build`) does, removing one of `isBuild`'s clues about what phase it's in.) It then sets an environment variable as a clue to future calls to itself, so that, child processes or not, it knows the correct phase. This means, however, that if it stops being used at `index.server.ts`'s top level, and doesn't get called until after the process split, it won't have any way to know whether it's in the build phase or not.

This makes two changes to guard against those possibilities:

- To ensure that it's always called as early as possible in the build, it is now run independently of any use of its return value, instead storing that value in a constant, which is then used anywhere it's needed. 
- To provide a backup indicator of the current phase, it now also checks the next-provided `NEXT_PHASE` environment variable. (The reason it doesn't simply rely on this variable to begin with is that it's only set partway through the build process.)

As a result of these changes, two other changes are included here:

- The export of `isBuild` is deprecated in favor of the computed constant. (Since they're equivalent, no reason to keep exporting them both.)
- The local constant `isVercel` has been renamed `IS_VERCEL`, to match `IS_BUILD`.

Author: lobsterkatie

packages/nextjs/src/index.server.ts

@@ -22,7 +22,14 @@ export { ErrorBoundary, showReportDialog, withErrorBoundary } from '@sentry/reac
 type GlobalWithDistDir = typeof global & { __rewriteFramesDistDir__: string };
 const domain = domainModule as typeof domainModule & { active: (domainModule.Domain & Carrier) | null };
 
-const isVercel = !!process.env.VERCEL;
+// 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 = isBuild();
+const IS_VERCEL = !!process.env.VERCEL;
 
 /** Inits the Sentry NextJS SDK on node. */
 export function init(options: NextjsOptions): void {
@@ -63,7 +70,7 @@ export function init(options: NextjsOptions): void {
 
   configureScope(scope => {
     scope.setTag('runtime', 'node');
-    if (isVercel) {
+    if (IS_VERCEL) {
       scope.setTag('vercel', true);
     }
 
@@ -124,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,
@@ -142,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 (!isBuild() && !isVercel) {
+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 {

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 <path>/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 <path>/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;
   }