Backport: ensure worker exits bubble to parent process (#73433)

Backports:
- #70997
- #73138

To avoid introducing the larger worker refactor in #68546, this PR
introduces additional handling needed to special case the "restarting"
flow to not exit the parent process.

packages/next/src/build/index.ts

@@ -186,6 +186,7 @@ import { createProgress } from './progress'
 import { traceMemoryUsage } from '../lib/memory/trace'
 import { generateEncryptionKeyBase64 } from '../server/app-render/encryption-utils'
 import type { DeepReadonly } from '../shared/lib/deep-readonly'
+import { getNodeOptionsWithoutInspect } from '../server/lib/utils'
 
 interface ExperimentalBypassForInfo {
   experimentalBypassFor?: RouteHas[]
@@ -564,7 +565,6 @@ function createStaticWorker(
 ): StaticWorker {
   let infoPrinted = false
   const timeout = config.staticPageGenerationTimeout || 0
-
   return new Worker(staticWorkerPath, {
     timeout: timeout * 1000,
     logger: Log,
@@ -607,6 +607,11 @@ function createStaticWorker(
           ? incrementalCacheIpcPort + ''
           : undefined,
         __NEXT_INCREMENTAL_CACHE_IPC_KEY: incrementalCacheIpcValidationKey,
+        // we don't pass down NODE_OPTIONS as it can
+        // extra memory usage
+        NODE_OPTIONS: getNodeOptionsWithoutInspect()
+          .replace(/--max-old-space-size=[\d]{1,}/, '')
+          .trim(),
       },
     },
     enableWorkerThreads: config.experimental.workerThreads,

packages/next/src/build/webpack-build/index.ts

@@ -2,9 +2,8 @@ import type { COMPILER_INDEXES } from '../../shared/lib/constants'
 import * as Log from '../output/log'
 import { NextBuildContext } from '../build-context'
 import type { BuildTraceContext } from '../webpack/plugins/next-trace-entrypoints-plugin'
-import { Worker } from 'next/dist/compiled/jest-worker'
+import { Worker } from '../../lib/worker'
 import origDebug from 'next/dist/compiled/debug'
-import type { ChildProcess } from 'child_process'
 import path from 'path'
 import { exportTraceState, recordTraceEvents } from '../../trace'
 
@@ -38,8 +37,13 @@ async function webpackBuildWithWorker(
 
   prunedBuildContext.pluginState = pluginState
 
-  const getWorker = (compilerName: string) => {
-    const _worker = new Worker(path.join(__dirname, 'impl.js'), {
+  const combinedResult = {
+    duration: 0,
+    buildTraceContext: {} as BuildTraceContext,
+  }
+
+  for (const compilerName of compilerNames) {
+    const worker = new Worker(path.join(__dirname, 'impl.js'), {
       exposedMethods: ['workerMain'],
       numWorkers: 1,
       maxRetries: 0,
@@ -50,31 +54,6 @@ async function webpackBuildWithWorker(
         },
       },
     }) as Worker & typeof import('./impl')
-    _worker.getStderr().pipe(process.stderr)
-    _worker.getStdout().pipe(process.stdout)
-
-    for (const worker of ((_worker as any)._workerPool?._workers || []) as {
-      _child: ChildProcess
-    }[]) {
-      worker._child.on('exit', (code, signal) => {
-        if (code || (signal && signal !== 'SIGINT')) {
-          debug(
-            `Compiler ${compilerName} unexpectedly exited with code: ${code} and signal: ${signal}`
-          )
-        }
-      })
-    }
-
-    return _worker
-  }
-
-  const combinedResult = {
-    duration: 0,
-    buildTraceContext: {} as BuildTraceContext,
-  }
-
-  for (const compilerName of compilerNames) {
-    const worker = getWorker(compilerName)
 
     const curResult = await worker.workerMain({
       buildContext: prunedBuildContext,

packages/next/src/lib/worker.ts

@@ -1,6 +1,6 @@
 import type { ChildProcess } from 'child_process'
 import { Worker as JestWorker } from 'next/dist/compiled/jest-worker'
-import { getNodeOptionsWithoutInspect } from '../server/lib/utils'
+
 type FarmOptions = ConstructorParameters<typeof JestWorker>[1]
 
 const RESTARTED = Symbol('restarted')
@@ -15,6 +15,7 @@ const cleanupWorkers = (worker: JestWorker) => {
 
 export class Worker {
   private _worker: JestWorker | undefined
+  private _restarting: boolean = false
 
   constructor(
     workerPath: string,
@@ -42,14 +43,10 @@ export class Worker {
           env: {
             ...((farmOptions.forkOptions?.env || {}) as any),
             ...process.env,
-            // we don't pass down NODE_OPTIONS as it can
-            // extra memory usage
-            NODE_OPTIONS: getNodeOptionsWithoutInspect()
-              .replace(/--max-old-space-size=[\d]{1,}/, '')
-              .trim(),
           } as any,
         },
       }) as JestWorker
+      this._restarting = false
       restartPromise = new Promise(
         (resolve) => (resolveRestartPromise = resolve)
       )
@@ -73,6 +70,12 @@ export class Worker {
               logger.error(
                 `Static worker exited with code: ${code} and signal: ${signal}`
               )
+
+              // We're restarting the worker, so we don't want to exit the parent process
+              if (!this._restarting) {
+                // if a child process doesn't exit gracefully, we want to bubble up the exit code to the parent process
+                process.exit(code ?? 1)
+              }
             }
           })
         }
@@ -87,14 +90,17 @@ export class Worker {
       const worker = this._worker
       if (!worker) return
       const resolve = resolveRestartPromise
-      createWorker()
       logger.warn(
         `Sending SIGTERM signal to static worker due to timeout${
           timeout ? ` of ${timeout / 1000} seconds` : ''
         }. Subsequent errors may be a result of the worker exiting.`
       )
+
+      this._restarting = true
+
       worker.end().then(() => {
         resolve(RESTARTED)
+        createWorker()
       })
     }
 
@@ -119,6 +125,7 @@ export class Worker {
                   (this._worker as any)[method](...args),
                   restartPromise,
                 ])
+
                 if (result !== RESTARTED) return result
                 if (onRestart) onRestart(method, args, ++attempts)
               }

test/production/app-dir/worker-restart/fixtures/worker-kill/app/bad-page/page.js

@@ -0,0 +1,5 @@
+export default function Page() {
+  process.kill(process.pid, 'SIGKILL')
+
+  return <div>Kaboom</div>
+}

test/production/app-dir/worker-restart/fixtures/worker-kill/app/layout.js

@@ -0,0 +1,7 @@
+export default function Layout({ children }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}

test/production/app-dir/worker-restart/fixtures/worker-kill/app/page.js

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>Hello World</div>
+}

test/production/app-dir/worker-restart/fixtures/worker-kill/next.config.js

@@ -0,0 +1 @@
+module.exports = {}

test/production/app-dir/worker-restart/worker-restart.test.ts

@@ -2,10 +2,14 @@ import { nextBuild } from 'next-test-utils'
 
 describe('worker-restart', () => {
   it('should properly exhaust all restart attempts and not fail with any worker errors', async () => {
-    const { stdout, stderr } = await nextBuild(__dirname, [], {
-      stdout: true,
-      stderr: true,
-    })
+    const { stdout, stderr } = await nextBuild(
+      __dirname + '/fixtures/timeout',
+      [],
+      {
+        stdout: true,
+        stderr: true,
+      }
+    )
 
     const output = stdout + stderr
     expect(output).toContain(
@@ -24,4 +28,20 @@ describe('worker-restart', () => {
       'Error: Farm is ended, no more calls can be done to it'
     )
   })
+
+  it('should fail the build if a worker process is killed', async () => {
+    const { stdout, stderr } = await nextBuild(
+      __dirname + '/fixtures/worker-kill',
+      [],
+      {
+        stdout: true,
+        stderr: true,
+      }
+    )
+
+    const output = stdout + stderr
+    expect(output).toContain(
+      'Static worker exited with code: null and signal: SIGKILL'
+    )
+  })
 })