fix static worker restart behavior (#56728)

ztanner · web-flow · commit d735d317d1c1 · 2023-10-12T00:19:29.000Z
In [55841](#55841), this file was reworked to improve type safety and readability, but it changed the behavior of how we were invoking methods on the worker. Specifically, when a restart occurred, this timeout wrapping function was referencing an already ended worker, resulting in a "Farm is ended, no more calls can be done to it" build error. This PR ensures that we're fetching the method from the current `this._worker` at the time of invocation, not at the time of method creation. [Slack x-ref](https://vercel.slack.com/archives/C04KC8A53T7/p1697064752635179?thread_ts=1696952142.759769&cid=C04KC8A53T7)
diff --git a/packages/next/src/lib/worker.ts b/packages/next/src/lib/worker.ts
@@ -133,9 +133,7 @@ export class Worker<T extends object = object, Args extends any[] = any[]> {
     }
 
     const wrapMethodWithTimeout =
-      <M extends (...args: unknown[]) => Promise<unknown> | unknown>(
-        method: M
-      ) =>
+      (methodName: keyof T) =>
       async (...args: Args) => {
         activeTasks++
 
@@ -149,7 +147,8 @@ export class Worker<T extends object = object, Args extends any[] = any[]> {
             const result = await Promise.race([
               // Either we'll get the result from the worker, or we'll get the
               // restart promise to fire.
-              method(...args),
+              // @ts-expect-error - we're grabbing a dynamic method on the worker
+              this._worker[methodName](...args),
               restartPromise,
             ])
 
@@ -160,7 +159,7 @@ export class Worker<T extends object = object, Args extends any[] = any[]> {
             }
 
             // Otherwise, we'll need to restart the worker, and try again.
-            if (onRestart) onRestart(method.name, args, ++attempts)
+            if (onRestart) onRestart(methodName.toString(), args, ++attempts)
           }
         } finally {
           activeTasks--
@@ -174,7 +173,7 @@ export class Worker<T extends object = object, Args extends any[] = any[]> {
       // @ts-expect-error - we're grabbing a dynamic method on the worker
       let method = this._worker[name].bind(this._worker)
       if (timeout) {
-        method = wrapMethodWithTimeout(method)
+        method = wrapMethodWithTimeout(name)
       }
 
       // @ts-expect-error - we're dynamically creating methods
diff --git a/test/production/app-dir/worker-restart/app/bad-page/page.js b/test/production/app-dir/worker-restart/app/bad-page/page.js
@@ -0,0 +1,11 @@
+export async function generateMetadata() {
+  await new Promise((resolve) => setTimeout(resolve, 11000))
+
+  return {
+    title: 'Test',
+  }
+}
+
+export default function Page() {
+  return <div>Hello World</div>
+}
diff --git a/test/production/app-dir/worker-restart/app/layout.js b/test/production/app-dir/worker-restart/app/layout.js
@@ -0,0 +1,7 @@
+export default function Layout({ children }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/production/app-dir/worker-restart/app/page.js b/test/production/app-dir/worker-restart/app/page.js
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>Root Page</div>
+}
diff --git a/test/production/app-dir/worker-restart/next.config.js b/test/production/app-dir/worker-restart/next.config.js
@@ -0,0 +1,3 @@
+module.exports = {
+  staticPageGenerationTimeout: 10,
+}
diff --git a/test/production/app-dir/worker-restart/worker-restart.test.ts b/test/production/app-dir/worker-restart/worker-restart.test.ts
@@ -0,0 +1,21 @@
+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 output = stdout + stderr
+    expect(output).toContain(
+      'Restarted static page generation for /bad-page because it took more than 10 seconds'
+    )
+    expect(output).toContain(
+      'Static page generation for /bad-page is still timing out after 3 attempts'
+    )
+    expect(output).not.toContain(
+      'Error: Farm is ended, no more calls can be done to it'
+    )
+  })
+})

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export default function Page() {`
	`2`	`+ return <div>Root Page</div>`
	`3`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+module.exports = {`
	`2`	`+ staticPageGenerationTimeout: 10,`
	`3`	`+}`