From 603f2e0f2f617939b924c94e5dc39bb522b0378e Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Sun, 24 Nov 2024 09:19:00 -0800 Subject: [PATCH] Reapply "ensure webpack worker exits bubble to parent process (#72921)" (#73138) This relands #72921 to be a more minimal refactor. This re-applies the core change to use the existing worker that has proper error bubbling handling without changing the worker lifecycle. For the purposes of the internal OOM we were seeing, this ensures that any custom `max-old-space-size` flags are preserved during the webpack build step, even when using the shared worker. Instead, I moved it to `createStaticWorker`, as that was where it was intended to be respected when it landed in #46705. --- packages/next/src/build/index.ts | 12 ++++- .../next/src/build/webpack-build/index.ts | 44 +++++++------------ packages/next/src/lib/worker.ts | 11 ----- 3 files changed, 26 insertions(+), 41 deletions(-) diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index d2f9650b1b98b..2533f50a21efe 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -216,6 +216,10 @@ import { inlineStaticEnv } from './flying-shuttle/inline-static-env' import { FallbackMode, fallbackModeToFallbackField } from '../lib/fallback' import { RenderingMode } from './rendering-mode' import { getParamKeys } from '../server/request/fallback-params' +import { + formatNodeOptions, + getParsedNodeOptionsWithoutInspect, +} from '../server/lib/utils' type Fallback = null | boolean | string @@ -684,6 +688,12 @@ export function createStaticWorker( clear: () => void } ): StaticWorker { + // Get the node options without inspect and also remove the + // --max-old-space-size flag as it can cause memory issues. + const nodeOptions = getParsedNodeOptionsWithoutInspect() + delete nodeOptions['max-old-space-size'] + delete nodeOptions['max_old_space_size'] + return new Worker(staticWorkerPath, { logger: Log, numWorkers: getNumberOfWorkers(config), @@ -694,7 +704,7 @@ export function createStaticWorker( progress?.clear() }, forkOptions: { - env: process.env, + env: { ...process.env, NODE_OPTIONS: formatNodeOptions(nodeOptions) }, }, enableWorkerThreads: config.experimental.workerThreads, exposedMethods: staticWorkerExposedMethods, diff --git a/packages/next/src/build/webpack-build/index.ts b/packages/next/src/build/webpack-build/index.ts index c88c6781d7239..3537041cf7a19 100644 --- a/packages/next/src/build/webpack-build/index.ts +++ b/packages/next/src/build/webpack-build/index.ts @@ -2,11 +2,14 @@ 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' +import { + formatNodeOptions, + getParsedNodeOptionsWithoutInspect, +} from '../../server/lib/utils' const debug = origDebug('next:build:webpack-build') @@ -38,8 +41,15 @@ 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, + } + + const nodeOptions = getParsedNodeOptionsWithoutInspect() + + for (const compilerName of compilerNames) { + const worker = new Worker(path.join(__dirname, 'impl.js'), { exposedMethods: ['workerMain'], numWorkers: 1, maxRetries: 0, @@ -47,34 +57,10 @@ async function webpackBuildWithWorker( env: { ...process.env, NEXT_PRIVATE_BUILD_WORKER: '1', + NODE_OPTIONS: formatNodeOptions(nodeOptions), }, }, }) 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, diff --git a/packages/next/src/lib/worker.ts b/packages/next/src/lib/worker.ts index aeeeaa1d59338..1adddc8b418d4 100644 --- a/packages/next/src/lib/worker.ts +++ b/packages/next/src/lib/worker.ts @@ -1,9 +1,5 @@ import type { ChildProcess } from 'child_process' import { Worker as JestWorker } from 'next/dist/compiled/jest-worker' -import { - getParsedNodeOptionsWithoutInspect, - formatNodeOptions, -} from '../server/lib/utils' import { Transform } from 'stream' type FarmOptions = ConstructorParameters[1] @@ -47,12 +43,6 @@ export class Worker { }) const createWorker = () => { - // Get the node options without inspect and also remove the - // --max-old-space-size flag as it can cause memory issues. - const nodeOptions = getParsedNodeOptionsWithoutInspect() - delete nodeOptions['max-old-space-size'] - delete nodeOptions['max_old_space_size'] - this._worker = new JestWorker(workerPath, { ...farmOptions, forkOptions: { @@ -60,7 +50,6 @@ export class Worker { env: { ...((farmOptions.forkOptions?.env || {}) as any), ...process.env, - NODE_OPTIONS: formatNodeOptions(nodeOptions), } as any, }, maxRetries: 0,