fix: NODE_OPTIONS='--inspect' in next dev for debugging (#48019)

## Bug This fix closes #47083 issue This fix closes #47561 issue This fix closes #48376 issue **Invalid repetition PRs:** #47671 (this PR changing expired code) (This issue still exist on [v13.4.3-canary.1](https://github.com/vercel/next.js/releases/tag/v13.4.3-canary.1) - [x] Related issues linked using `fixes #number` ### What? When running `NODE_OPTIONS='--inspect' next dev`, the render server didn't start with `--inspect`. In some cases, the `--inspect` flag will be passed when `__NEXT_DISABLE_MEMORY_WATCHER` was set. ### Why? Since #47208 revamped some startup processes, the `NODE_OPTIONS` environment parameter is not passed down to the render server worker. ### How? Just add back the original startup process. ![image](https://user-images.githubusercontent.com/14261588/230398898-791e6909-6f4c-493b-937d-058a7b788849.png) link NEXT-1176 --------- Co-authored-by: JJ Kasper <[email protected]>
vercel · May 23, 2023 · 90331f3 · 90331f3
1 parent ff070e5
commit 90331f3
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 25 deletions.
diff --git a/packages/next/src/server/lib/render-server-standalone.ts b/packages/next/src/server/lib/render-server-standalone.ts
@@ -66,6 +66,7 @@ export const createServerHandler = async ({
     hostname,
     minimalMode,
     workerType: 'router',
+    isNodeDebugging: false,
   })
   didInitialize = true
 

diff --git a/packages/next/src/server/lib/render-server.ts b/packages/next/src/server/lib/render-server.ts
@@ -14,6 +14,7 @@ import {
   deleteAppClientCache as _deleteAppClientCache,
 } from '../../build/webpack/plugins/nextjs-require-cache-hot-reloader'
 import { clearModuleContext as _clearModuleContext } from '../web/sandbox/context'
+import { getFreePort } from '../lib/worker-utils'
 export const WORKER_SELF_EXIT_CODE = 77
 
 const MAXIMUM_HEAP_SIZE_ALLOWED =
@@ -45,6 +46,7 @@ export async function initialize(opts: {
   minimalMode?: boolean
   hostname?: string
   workerType: 'router' | 'render'
+  isNodeDebugging: boolean
   keepAliveTimeout?: number
 }): Promise<NonNullable<typeof result>> {
   // if we already setup the server return as we only need to do
@@ -79,7 +81,7 @@ export async function initialize(opts: {
     server.keepAliveTimeout = opts.keepAliveTimeout
   }
 
-  return new Promise((resolve, reject) => {
+  return new Promise(async (resolve, reject) => {
     server.on('error', (err: NodeJS.ErrnoException) => {
       console.error(`Invariant: failed to start render worker`, err)
       process.exit(1)
@@ -123,6 +125,7 @@ export async function initialize(opts: {
           customServer: false,
           httpServer: server,
           port: opts.port,
+          isNodeDebugging: opts.isNodeDebugging,
         })
 
         requestHandler = app.getRequestHandler()
@@ -133,6 +136,6 @@ export async function initialize(opts: {
         return reject(err)
       }
     })
-    server.listen(0, opts.hostname)
+    server.listen((await getFreePort()) + 1, opts.hostname)
   })
 }
diff --git a/packages/next/src/server/lib/server-ipc/index.ts b/packages/next/src/server/lib/server-ipc/index.ts
@@ -1,9 +1,10 @@
 import type NextServer from '../../next-server'
 
-import { genExecArgv, getNodeOptionsWithoutInspect } from '../utils'
+import { getNodeOptionsWithoutInspect } from '../utils'
 import { deserializeErr, errorToJSON } from '../../render'
 import crypto from 'crypto'
 import isError from '../../../lib/is-error'
+import { genRenderExecArgv } from '../worker-utils'
 
 // we can't use process.send as jest-worker relies on
 // it already and can cause unexpected message errors
@@ -78,8 +79,7 @@ export async function createIpcServer(
   }
 }
 
-export const createWorker = (
-  serverPort: number,
+export const createWorker = async (
   ipcPort: number,
   ipcValidationKey: string,
   isNodeDebugging: boolean | 'brk' | undefined,
@@ -88,6 +88,7 @@ export const createWorker = (
 ) => {
   const { initialEnv } = require('@next/env') as typeof import('@next/env')
   const { Worker } = require('next/dist/compiled/jest-worker')
+
   const worker = new Worker(require.resolve('../render-server'), {
     numWorkers: 1,
     // TODO: do we want to allow more than 10 OOM restarts?
@@ -97,7 +98,7 @@ export const createWorker = (
         FORCE_COLOR: '1',
         ...initialEnv,
         // we don't pass down NODE_OPTIONS as it can
-        // extra memory usage
+        // allow more memory usage than expected
         NODE_OPTIONS: getNodeOptionsWithoutInspect()
           .replace(/--max-old-space-size=[\d]{1,}/, '')
           .trim(),
@@ -115,10 +116,7 @@ export const createWorker = (
             }
           : {}),
       },
-      execArgv: genExecArgv(
-        isNodeDebugging === undefined ? false : isNodeDebugging,
-        (serverPort || 0) + 1
-      ),
+      execArgv: isNodeDebugging ? genRenderExecArgv(type) : undefined,
     },
     exposedMethods: [
       'initialize',

diff --git a/packages/next/src/server/lib/start-server.ts b/packages/next/src/server/lib/start-server.ts
@@ -1,11 +1,12 @@
 import http from 'http'
 import { isIPv6 } from 'net'
 import * as Log from '../../build/output/log'
-import { getNodeOptionsWithoutInspect } from './utils'
 import type { IncomingMessage, ServerResponse } from 'http'
 import type { ChildProcess } from 'child_process'
 import { normalizeRepeatedSlashes } from '../../shared/lib/utils'
 import { initialEnv } from '@next/env'
+import { genExecArgv, getNodeOptionsWithoutInspect } from './utils'
+import { getFreePort } from './worker-utils'
 
 export interface StartServerOptions {
   dir: string
@@ -184,11 +185,17 @@ export async function startServer({
         // TODO: do we want to allow more than 10 OOM restarts?
         maxRetries: 10,
         forkOptions: {
+          execArgv: isNodeDebugging
+            ? genExecArgv(
+                isNodeDebugging === undefined ? false : isNodeDebugging,
+                await getFreePort()
+              )
+            : undefined,
           env: {
             FORCE_COLOR: '1',
             ...((initialEnv || process.env) as typeof process.env),
             PORT: port + '',
-            NODE_OPTIONS: getNodeOptionsWithoutInspect().trim(),
+            NODE_OPTIONS: getNodeOptionsWithoutInspect(),
           },
         },
         exposedMethods: ['initialize'],
@@ -235,6 +242,7 @@ export async function startServer({
         hostname,
         dev: !!isDev,
         workerType: 'router',
+        isNodeDebugging: !!isNodeDebugging,
         keepAliveTimeout,
       })
       didInitialize = true

diff --git a/packages/next/src/server/lib/worker-utils.ts b/packages/next/src/server/lib/worker-utils.ts
@@ -1,6 +1,23 @@
 import * as Log from '../../build/output/log'
+import http from 'http'
 
-export const genRenderExecArgv = () => {
+export const getFreePort = async (): Promise<number> => {
+  return new Promise((resolve, reject) => {
+    const server = http.createServer(() => {})
+    server.listen(0, () => {
+      const address = server.address()
+      server.close()
+
+      if (address && typeof address === 'object') {
+        resolve(address.port)
+      } else {
+        reject(new Error('invalid address from server: ' + address?.toString()))
+      }
+    })
+  })
+}
+
+export const genRenderExecArgv = (type: 'pages' | 'app') => {
   const isDebugging =
     process.execArgv.some((localArg) => localArg.startsWith('--inspect')) ||
     process.env.NODE_OPTIONS?.match?.(/--inspect(=\S+)?( |$)/)
@@ -9,7 +26,7 @@ export const genRenderExecArgv = () => {
     process.execArgv.some((localArg) => localArg.startsWith('--inspect-brk')) ||
     process.env.NODE_OPTIONS?.match?.(/--inspect-brk(=\S+)?( |$)/)
 
-  const debugPort = (() => {
+  let debugPort = (() => {
     const debugPortStr =
       process.execArgv
         .find(
@@ -22,13 +39,15 @@ export const genRenderExecArgv = () => {
     return debugPortStr ? parseInt(debugPortStr, 10) : 9229
   })()
 
+  debugPort += type === 'pages' ? 2 : 3
+
   if (isDebugging || isDebuggingWithBrk) {
-    Log.warn(
+    Log.info(
       `the --inspect${
         isDebuggingWithBrk ? '-brk' : ''
-      } option was detected, the Next.js server should be inspected at port ${
-        debugPort + 1
-      }.`
+      } option was detected, the Next.js server${
+        type === 'pages' ? ' for pages' : type === 'app' ? ' for app' : ''
+      } should be inspected at port ${debugPort}.`
     )
   }
   const execArgv = process.execArgv.filter((localArg) => {
@@ -38,9 +57,7 @@ export const genRenderExecArgv = () => {
   })
 
   if (isDebugging || isDebuggingWithBrk) {
-    execArgv.push(
-      `--inspect${isDebuggingWithBrk ? '-brk' : ''}=${debugPort + 1}`
-    )
+    execArgv.push(`--inspect${isDebuggingWithBrk ? '-brk' : ''}=${debugPort}`)
   }
 
   return execArgv

diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts
@@ -273,6 +273,7 @@ export default class NextNodeServer extends BaseServer {
         hostname: this.hostname,
         minimalMode: this.minimalMode,
         dev: !!options.dev,
+        isNodeDebugging: !!options.isNodeDebugging,
       }
       const { createWorker, createIpcServer } =
         require('./lib/server-ipc') as typeof import('./lib/server-ipc')
@@ -281,17 +282,15 @@ export default class NextNodeServer extends BaseServer {
           this.renderWorkers = {}
           const { ipcPort, ipcValidationKey } = await createIpcServer(this)
           if (this.hasAppDir) {
-            this.renderWorkers.app = createWorker(
-              this.port || 0,
+            this.renderWorkers.app = await createWorker(
               ipcPort,
               ipcValidationKey,
               options.isNodeDebugging,
               'app',
               this.nextConfig.experimental.serverActions
             )
           }
-          this.renderWorkers.pages = createWorker(
-            this.port || 0,
+          this.renderWorkers.pages = await createWorker(
             ipcPort,
             ipcValidationKey,
             options.isNodeDebugging,