fix(start-plugin-core): retry failed prerender pages

.changeset/prerender-retry-count.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/start-plugin-core': patch
+---
+
+Fix `prerender.retryCount` never retrying a page. The retry re-entered the crawl step, which skipped the page because it had already been seen, so a page that failed once was dropped without being retried or reported, even with `failOnError` enabled. Failed pages are now requested again up to `retryCount` times. A page that still fails with `failOnError` enabled now stops the pages that have not started yet and makes prerendering reject with its error, instead of resolving and leaving the error as an unhandled rejection.

packages/start-plugin-core/src/prerender.ts

@@ -89,7 +89,7 @@ export async function prerender({
   async function prerenderPages({ outputDir }: { outputDir: string }) {
     const seen = new Set<string>()
     const prerendered = new Set<string>()
-    const retriesByPath = new Map<string, number>()
+    const errors: Array<unknown> = []
     const concurrency = startConfig.prerender?.concurrency ?? os.cpus().length
     logger.info(`Concurrency: ${concurrency}`)
     const queue = new Queue({ concurrency })
@@ -108,6 +108,10 @@ export async function prerender({
 
     await queue.start()
 
+    if (errors.length) {
+      throw errors[0]
+    }
+
     return Array.from(prerendered)
 
     function addCrawlPageTask(page: Page) {
@@ -133,9 +137,10 @@ export async function prerender({
         ...page.prerender,
       }
 
-      queue.add(async () => {
+      const prerenderPage = async (retries: number) => {
+        if (errors.length) return
+
         logger.info(`Crawling: ${page.path}`)
-        const retries = retriesByPath.get(page.path) || 0
 
         try {
           const res = await requestWithRedirects(
@@ -232,13 +237,14 @@ export async function prerender({
               `Encountered error, retrying: ${page.path} in ${retryDelay}ms`,
             )
             await new Promise((resolve) => setTimeout(resolve, retryDelay))
-            retriesByPath.set(page.path, retries + 1)
-            addCrawlPageTask(page)
+            queue.add(() => prerenderPage(retries + 1), { priority: true })
           } else if (prerenderOptions.failOnError ?? true) {
-            throw error
+            errors.push(error)
           }
         }
-      })
+      }
+
+      queue.add(() => prerenderPage(0))
     }
   }
 

packages/start-plugin-core/tests/prerender-fail-on-error.test.ts

@@ -0,0 +1,72 @@
+import { mkdtemp, rm } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import path from 'node:path'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { prerender } from '../src/prerender'
+import { parseStartConfig } from '../src/schema'
+
+vi.mock('../src/utils', async () => {
+  const actual = await vi.importActual<any>('../src/utils')
+  return {
+    ...actual,
+    createLogger: () => ({ info: () => {}, warn: () => {}, error: () => {} }),
+  }
+})
+
+const publicDirs: Array<string> = []
+
+afterEach(async () => {
+  await Promise.all(
+    publicDirs
+      .splice(0)
+      .map((dir) => rm(dir, { recursive: true, force: true })),
+  )
+})
+
+describe('prerender failOnError', () => {
+  it.each([0, 1])(
+    'stops requesting queued pages after a page fails with retryCount %i',
+    async (retryCount) => {
+      const publicDir = await mkdtemp(
+        path.join(tmpdir(), 'tss-prerender-fail-'),
+      )
+      publicDirs.push(publicDir)
+      const request = vi.fn(async (requestPath: string) => {
+        if (requestPath === '/missing/') {
+          return new Response('Not Found', {
+            status: 404,
+            statusText: 'Not Found',
+          })
+        }
+        return new Response(`<html><body>${requestPath}</body></html>`, {
+          headers: { 'content-type': 'text/html' },
+        })
+      })
+      const startConfig = parseStartConfig(
+        {
+          prerender: {
+            enabled: true,
+            autoStaticPathsDiscovery: false,
+            crawlLinks: false,
+            concurrency: 1,
+            retryCount,
+            retryDelay: 0,
+          },
+          pages: [{ path: '/missing' }, { path: '/about' }],
+        },
+        { framework: 'react' },
+        process.cwd(),
+      )
+
+      await expect(
+        prerender({
+          startConfig,
+          handler: { getClientOutputDirectory: () => publicDir, request },
+        }),
+      ).rejects.toThrow('Failed to fetch /missing: Not Found')
+      expect(request.mock.calls.map(([requestPath]) => requestPath)).toEqual(
+        Array(1 + retryCount).fill('/missing/'),
+      )
+    },
+  )
+})

packages/start-plugin-core/tests/prerender-retry.test.ts

@@ -0,0 +1,101 @@
+import { mkdtemp, readFile, rm } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import path from 'node:path'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { prerender } from '../src/prerender'
+import { parseStartConfig } from '../src/schema'
+import type { TanStackStartInputConfig } from '../src/schema'
+
+vi.mock('../src/utils', async () => {
+  const actual = await vi.importActual<any>('../src/utils')
+  return {
+    ...actual,
+    createLogger: () => ({ info: () => {}, warn: () => {}, error: () => {} }),
+  }
+})
+
+const aboutDocument = '<html><body>about</body></html>'
+
+const publicDirs: Array<string> = []
+
+afterEach(async () => {
+  await Promise.all(
+    publicDirs
+      .splice(0)
+      .map((dir) => rm(dir, { recursive: true, force: true })),
+  )
+})
+
+async function prerenderAbout(
+  prerenderOptions: NonNullable<TanStackStartInputConfig['prerender']>,
+  failures: number,
+) {
+  const publicDir = await mkdtemp(path.join(tmpdir(), 'tss-prerender-retry-'))
+  publicDirs.push(publicDir)
+  const request = vi.fn(async () => {
+    if (request.mock.calls.length <= failures) {
+      return new Response('Server Error', { status: 500 })
+    }
+    return new Response(aboutDocument, {
+      headers: { 'content-type': 'text/html' },
+    })
+  })
+  const startConfig = parseStartConfig(
+    {
+      prerender: {
+        enabled: true,
+        autoStaticPathsDiscovery: false,
+        retryDelay: 0,
+        ...prerenderOptions,
+      },
+      pages: [{ path: '/about' }],
+    },
+    { framework: 'react' },
+    process.cwd(),
+  )
+
+  const result = prerender({
+    startConfig,
+    handler: { getClientOutputDirectory: () => publicDir, request },
+  })
+
+  return {
+    result,
+    request,
+    readAbout: () => readFile(path.join(publicDir, 'about/index.html'), 'utf8'),
+  }
+}
+
+describe('prerender retries', () => {
+  it('writes a page that succeeds on a retry', async () => {
+    const { result, request, readAbout } = await prerenderAbout(
+      { retryCount: 2 },
+      1,
+    )
+
+    await expect(result).resolves.toBeUndefined()
+    expect(request).toHaveBeenCalledTimes(2)
+    await expect(readAbout()).resolves.toBe(aboutDocument)
+  })
+
+  it('requests a failing page once per allowed retry before skipping it', async () => {
+    const { result, request, readAbout } = await prerenderAbout(
+      { retryCount: 2, failOnError: false },
+      Infinity,
+    )
+
+    await expect(result).resolves.toBeUndefined()
+    expect(request).toHaveBeenCalledTimes(3)
+    await expect(readAbout()).rejects.toMatchObject({ code: 'ENOENT' })
+  })
+
+  it.each([0, 2])(
+    'rejects once a failing page exhausts retryCount %i',
+    async (retryCount) => {
+      const { result, request } = await prerenderAbout({ retryCount }, Infinity)
+
+      await expect(result).rejects.toThrow('Failed to fetch /about')
+      expect(request).toHaveBeenCalledTimes(1 + retryCount)
+    },
+  )
+})