Revert static worker refactor (#56767)

ztanner · web-flow · commit 10ba5f772b6e · 2023-10-12T23:03:48.000Z
The changes to this worker from #55841 seem to also cause strange behavior where the process doesn't exit despite sending an exit signal. Reverting until we can spend the time to investigate what the issue is. [slack x-ref](https://vercel.slack.com/archives/C03S8ED1DKM/p1697129702263269) [slack x-ref](https://vercel.slack.com/archives/C056QDZARTM/p1697149427099799?thread_ts=1696351087.736439&cid=C056QDZARTM)
diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts
@@ -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'
@@ -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') {
@@ -1265,7 +1252,14 @@ export default async function build(
             'getDefinedNamedExports',
             'exportPage',
           ],
-        })
+        }) as Worker &
+          Pick<
+            typeof import('./worker'),
+            | 'hasCustomGetInitialProps'
+            | 'isPageStatic'
+            | 'getDefinedNamedExports'
+            | 'exportPage'
+          >
       }
 
       let CacheHandler: any
diff --git a/packages/next/src/export/index.ts b/packages/next/src/export/index.ts
@@ -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'
@@ -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,
diff --git a/packages/next/src/lib/worker.ts b/packages/next/src/lib/worker.ts
@@ -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')
@@ -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)
@@ -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(
@@ -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)
       })
@@ -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)
     }
   }
 
