Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert static worker refactor #56767

Merged
merged 3 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 10 additions & 16 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import type { PagesManifest } from './webpack/plugins/pages-manifest-plugin'
import type { ExportPathMap, NextConfigComplete } from '../server/config-shared'
import type { MiddlewareManifest } from './webpack/plugins/middleware-plugin'
import type { ActionManifest } from './webpack/plugins/flight-client-entry-plugin'
import type {
ExportAppOptions,
ExportAppWorker,
ExportPageInput,
} from '../export/types'
import type { ExportAppOptions, ExportAppWorker } from '../export/types'
import type { Revalidate } from '../server/lib/revalidate'

import '../lib/setup-exception-listeners'
Expand Down Expand Up @@ -1209,16 +1205,7 @@ export default async function build(
) {
let infoPrinted = false

return Worker.create<
Pick<
typeof import('./worker'),
| 'hasCustomGetInitialProps'
| 'isPageStatic'
| 'getDefinedNamedExports'
| 'exportPage'
>,
[ExportPageInput]
>(staticWorkerPath, {
return new Worker(staticWorkerPath, {
timeout: timeout * 1000,
onRestart: (method, [arg], attempts) => {
if (method === 'exportPage') {
Expand Down Expand Up @@ -1265,7 +1252,14 @@ export default async function build(
'getDefinedNamedExports',
'exportPage',
],
})
}) as Worker &
Pick<
typeof import('./worker'),
| 'hasCustomGetInitialProps'
| 'isPageStatic'
| 'getDefinedNamedExports'
| 'exportPage'
>
}

let CacheHandler: any
Expand Down
46 changes: 21 additions & 25 deletions packages/next/src/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type {
ExportAppOptions,
ExportWorker,
WorkerRenderOptsPartial,
ExportPageInput,
} from './types'
import type { PrerenderManifest } from '../build'
import type { PagesManifest } from '../build/webpack/plugins/pages-manifest-plugin'
Expand Down Expand Up @@ -174,32 +173,29 @@ function setupWorkers(

let infoPrinted = false

const worker = Worker.create<typeof import('./worker'), [ExportPageInput]>(
require.resolve('./worker'),
{
timeout: timeout * 1000,
onRestart: (_method, [{ path }], attempts) => {
if (attempts >= 3) {
throw new ExportError(
`Static page generation for ${path} is still timing out after 3 attempts. See more info here https://nextjs.org/docs/messages/static-page-generation-timeout`
)
}
const worker = new Worker(require.resolve('./worker'), {
timeout: timeout * 1000,
onRestart: (_method, [{ path }], attempts) => {
if (attempts >= 3) {
throw new ExportError(
`Static page generation for ${path} is still timing out after 3 attempts. See more info here https://nextjs.org/docs/messages/static-page-generation-timeout`
)
}
Log.warn(
`Restarted static page generation for ${path} because it took more than ${timeout} seconds`
)
if (!infoPrinted) {
Log.warn(
`Restarted static page generation for ${path} because it took more than ${timeout} seconds`
'See more info here https://nextjs.org/docs/messages/static-page-generation-timeout'
)
if (!infoPrinted) {
Log.warn(
'See more info here https://nextjs.org/docs/messages/static-page-generation-timeout'
)
infoPrinted = true
}
},
maxRetries: 0,
numWorkers: threads,
enableWorkerThreads: nextConfig.experimental.workerThreads,
exposedMethods: ['default'],
}
)
infoPrinted = true
}
},
maxRetries: 0,
numWorkers: threads,
enableWorkerThreads: nextConfig.experimental.workerThreads,
exposedMethods: ['default'],
}) as Worker & typeof import('./worker')

return {
pages: worker.default,
Expand Down
159 changes: 52 additions & 107 deletions packages/next/src/lib/worker.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import type { ChildProcess } from 'child_process'
import { Worker as JestWorker } from 'next/dist/compiled/jest-worker'
import { getNodeOptionsWithoutInspect } from '../server/lib/utils'

// We need this as we're using `Promise.withResolvers` which is not available in the node typings
import '../lib/polyfill-promise-with-resolvers'

type FarmOptions = ConstructorParameters<typeof JestWorker>[1]

const RESTARTED = Symbol('restarted')
Expand All @@ -17,58 +13,45 @@ const cleanupWorkers = (worker: JestWorker) => {
}
}

type Options<
T extends object = object,
Args extends any[] = any[]
> = FarmOptions & {
timeout?: number
onRestart?: (method: string, args: Args, attempts: number) => void
exposedMethods: ReadonlyArray<keyof T>
enableWorkerThreads?: boolean
}

export class Worker<T extends object = object, Args extends any[] = any[]> {
private _worker?: JestWorker
export class Worker {
private _worker: JestWorker | undefined

/**
* Creates a new worker with the correct typings associated with the selected
* methods.
*/
public static create<T extends object, Args extends any[] = any[]>(
constructor(
workerPath: string,
options: Options<T, Args>
): Worker<T, Args> & T {
return new Worker(workerPath, options) as Worker<T, Args> & T
}

constructor(workerPath: string, options: Options<T, Args>) {
options: FarmOptions & {
timeout?: number
onRestart?: (method: string, args: any[], attempts: number) => void
exposedMethods: ReadonlyArray<string>
enableWorkerThreads?: boolean
}
) {
let { timeout, onRestart, ...farmOptions } = options

let restartPromise: Promise<typeof RESTARTED>
let resolveRestartPromise: (arg: typeof RESTARTED) => void
let activeTasks = 0

this._worker = undefined

const createWorker = () => {
const worker = new JestWorker(workerPath, {
this._worker = new JestWorker(workerPath, {
...farmOptions,
forkOptions: {
...farmOptions.forkOptions,
env: {
...farmOptions.forkOptions?.env,
...((farmOptions.forkOptions?.env || {}) as any),
...process.env,
// We don't pass down NODE_OPTIONS as it can lead to extra memory
// usage,
// we don't pass down NODE_OPTIONS as it can
// extra memory usage
NODE_OPTIONS: getNodeOptionsWithoutInspect()
.replace(/--max-old-space-size=[\d]{1,}/, '')
.trim(),
},
stdio: 'inherit',
} as any,
},
})

const { promise, resolve } = Promise.withResolvers<typeof RESTARTED>()
restartPromise = promise
resolveRestartPromise = resolve
}) as JestWorker
restartPromise = new Promise(
(resolve) => (resolveRestartPromise = resolve)
)

/**
* Jest Worker has two worker types, ChildProcessWorker (uses child_process) and NodeThreadWorker (uses worker_threads)
Expand All @@ -80,14 +63,11 @@ export class Worker<T extends object = object, Args extends any[] = any[]> {
* But this property is not available in NodeThreadWorker, so we need to check if we are using ChildProcessWorker
*/
if (!farmOptions.enableWorkerThreads) {
const poolWorkers: { _child?: ChildProcess }[] =
// @ts-expect-error - we're accessing a private property
worker._workerPool?._workers ?? []

for (const poolWorker of poolWorkers) {
if (!poolWorker._child) continue

poolWorker._child.once('exit', (code, signal) => {
for (const worker of ((this._worker as any)._workerPool?._workers ||
[]) as {
_child?: ChildProcess
}[]) {
worker._child?.on('exit', (code, signal) => {
// log unexpected exit if .end() wasn't called
if ((code || (signal && signal !== 'SIGINT')) && this._worker) {
console.error(
Expand All @@ -98,22 +78,16 @@ export class Worker<T extends object = object, Args extends any[] = any[]> {
}
}

return worker
this._worker.getStdout().pipe(process.stdout)
this._worker.getStderr().pipe(process.stderr)
}

// Create the first worker.
this._worker = createWorker()
createWorker()

const onHanging = () => {
const worker = this._worker
if (!worker) return

// Grab the current restart promise, and create a new worker.
const resolve = resolveRestartPromise
this._worker = createWorker()

// Once the old worker is ended, resolve the restart promise to signal to
// any active tasks that the worker had to be restarted.
createWorker()
worker.end().then(() => {
resolve(RESTARTED)
})
Expand All @@ -122,62 +96,33 @@ export class Worker<T extends object = object, Args extends any[] = any[]> {
let hangingTimer: NodeJS.Timeout | false = false

const onActivity = () => {
// If there was an active hanging timer, clear it.
if (hangingTimer) clearTimeout(hangingTimer)

// If there are no active tasks, we don't need to start a new hanging
// timer.
if (activeTasks === 0) return

hangingTimer = setTimeout(onHanging, timeout)
hangingTimer = activeTasks > 0 && setTimeout(onHanging, timeout)
}

const wrapMethodWithTimeout =
(methodName: keyof T) =>
async (...args: Args) => {
activeTasks++

try {
let attempts = 0
for (;;) {
// Mark that we're doing work, we want to ensure that if the worker
// halts for any reason, we restart it.
onActivity()

const result = await Promise.race([
// Either we'll get the result from the worker, or we'll get the
// restart promise to fire.
// @ts-expect-error - we're grabbing a dynamic method on the worker
this._worker[methodName](...args),
restartPromise,
])

// If the result anything besides `RESTARTED`, we can return it, as
// it's the actual result from the worker.
if (result !== RESTARTED) {
return result
for (const method of farmOptions.exposedMethods) {
if (method.startsWith('_')) continue
;(this as any)[method] = timeout
? // eslint-disable-next-line no-loop-func
async (...args: any[]) => {
activeTasks++
try {
let attempts = 0
for (;;) {
onActivity()
const result = await Promise.race([
(this._worker as any)[method](...args),
restartPromise,
])
if (result !== RESTARTED) return result
if (onRestart) onRestart(method, args, ++attempts)
}
} finally {
activeTasks--
onActivity()
}

// Otherwise, we'll need to restart the worker, and try again.
if (onRestart) onRestart(methodName.toString(), args, ++attempts)
}
} finally {
activeTasks--
onActivity()
}
}

for (const name of farmOptions.exposedMethods) {
if (name.startsWith('_')) continue

// @ts-expect-error - we're grabbing a dynamic method on the worker
let method = this._worker[name].bind(this._worker)
if (timeout) {
method = wrapMethodWithTimeout(name)
}

// @ts-expect-error - we're dynamically creating methods
this[name] = method
: (this._worker as any)[method].bind(this._worker)
}
}

Expand Down