From 8ff3cc14a5abdc66b64f53ad26d270dd1111965e Mon Sep 17 00:00:00 2001 From: TLSRUF Date: Fri, 28 Aug 2026 16:22:10 +0900 Subject: [PATCH 1/2] fix(start-plugin-core): prerender retryCount never retries, failOnError never fails the build Two bugs in prerender.ts's addCrawlPageTask/prerenderPages: 1. retryCount never retries. The retry path called addCrawlPageTask(page) again, but the page's path was already marked in the 'seen' set on the first attempt, so it returned early without re-queuing. Fixed by deleting the path from 'seen' before re-adding it. 2. failOnError couldn't fail the build. queue.add(...)'s returned promise was never awaited or attached to, so a rejection (thrown once retries were exhausted) became an unhandled promise rejection while queue.start() resolved regardless via onSettled (which fires on success or failure). The build exited 0 with the page missing from the output. Fixed by collecting each task's rejection and rethrowing an AggregateError after queue.start() settles. Added tests/prerender-retry.test.ts covering: a page that fails then succeeds within retryCount, failOnError rejecting the build once retries are exhausted, and failOnError: false still resolving without throwing. Verified all three fail against the pre-fix code with the exact symptoms described in the issue (retry count 1 instead of 3, promise resolving instead of rejecting) before restoring the fix. --- .changeset/prerender-retry-fail-on-error.md | 5 + packages/start-plugin-core/src/prerender.ts | 27 ++++- .../tests/prerender-retry.test.ts | 110 ++++++++++++++++++ 3 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 .changeset/prerender-retry-fail-on-error.md create mode 100644 packages/start-plugin-core/tests/prerender-retry.test.ts diff --git a/.changeset/prerender-retry-fail-on-error.md b/.changeset/prerender-retry-fail-on-error.md new file mode 100644 index 00000000000..e3ad9a9ea67 --- /dev/null +++ b/.changeset/prerender-retry-fail-on-error.md @@ -0,0 +1,5 @@ +--- +'@tanstack/start-plugin-core': patch +--- + +Fix `prerender.retryCount` never actually retrying a failed page (the path was already marked "seen" on the first attempt, so the retry was silently dropped), and fix `prerender.failOnError` not failing the build once retries were exhausted (the queued task's rejection was never awaited, so it became an unhandled promise rejection instead of stopping the build). diff --git a/packages/start-plugin-core/src/prerender.ts b/packages/start-plugin-core/src/prerender.ts index 2fbd9e74b9c..a091dcde5cf 100644 --- a/packages/start-plugin-core/src/prerender.ts +++ b/packages/start-plugin-core/src/prerender.ts @@ -86,6 +86,10 @@ export async function prerender({ const seen = new Set() const prerendered = new Set() const retriesByPath = new Map() + // Collects rejections from queue.add() tasks (e.g. failOnError) so they + // can be rethrown after the queue settles, instead of becoming unhandled + // promise rejections that let the build exit 0 with pages missing. + const taskErrors: Array = [] const concurrency = startConfig.prerender?.concurrency ?? os.cpus().length logger.info(`Concurrency: ${concurrency}`) const queue = new Queue({ concurrency }) @@ -106,6 +110,13 @@ export async function prerender({ await queue.start() + if (taskErrors.length) { + throw new AggregateError( + taskErrors, + `Failed to prerender ${taskErrors.length} page(s)`, + ) + } + return Array.from(prerendered) function addCrawlPageTask(page: Page) { @@ -131,7 +142,7 @@ export async function prerender({ ...page.prerender, } - queue.add(async () => { + const task = queue.add(async () => { logger.info(`Crawling: ${page.path}`) const retries = retriesByPath.get(page.path) || 0 @@ -219,12 +230,26 @@ export async function prerender({ ) await new Promise((resolve) => setTimeout(resolve, retryDelay)) retriesByPath.set(page.path, retries + 1) + // `seen` guards addCrawlPageTask against re-queuing a page that's + // already in flight/queued, but that also blocked the retry + // itself (the path was marked seen on the first attempt). Clear + // it so the retry actually gets queued. + seen.delete(page.path) addCrawlPageTask(page) } else if (prerenderOptions.failOnError ?? true) { throw error } } }) + + // `queue.add()` returns a promise that settles with the task, but + // nothing awaited it here, so a rejection (failOnError) became an + // unhandled promise rejection - the queue's own `isSettled()` doesn't + // care whether tasks succeeded, so `await queue.start()` below resolved + // and the build could exit 0 with pages missing. Collect it instead. + task.catch((error: unknown) => { + taskErrors.push(error) + }) } } diff --git a/packages/start-plugin-core/tests/prerender-retry.test.ts b/packages/start-plugin-core/tests/prerender-retry.test.ts new file mode 100644 index 00000000000..b5530175f4f --- /dev/null +++ b/packages/start-plugin-core/tests/prerender-retry.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it, vi } from 'vitest' +import { prerender } from '../src/prerender' + +vi.mock('../src/utils', async () => { + const actual = await vi.importActual('../src/utils') + return { + ...actual, + createLogger: () => ({ info: () => {}, warn: () => {}, error: () => {} }), + } +}) + +// Mock fs to prevent actual file system operations +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs') + return { + ...actual, + promises: { + ...actual.promises, + mkdir: vi.fn().mockResolvedValue(undefined), + writeFile: vi.fn().mockResolvedValue(undefined), + }, + } +}) + +function makeStartConfig( + pagePath: string, + prerenderOverrides: Record = {}, +) { + return { + prerender: { + enabled: true, + autoStaticPathsDiscovery: false, + concurrency: 1, + // keep retries fast in tests + retryDelay: 1, + ...prerenderOverrides, + }, + pages: [{ path: pagePath }], + router: { basepath: '' }, + spa: { + enabled: false, + prerender: { + outputPath: '/_shell', + crawlLinks: false, + retryCount: 0, + enabled: true, + }, + }, + } as any +} + +const handler = { + getClientOutputDirectory: () => '/client', +} + +describe('prerender retries', () => { + it('retries a page that fails and succeeds before exhausting retryCount', async () => { + let calls = 0 + const request = vi.fn(() => { + calls++ + if (calls <= 2) { + return new Response('nope', { status: 500 }) + } + return new Response('', { + status: 200, + headers: { 'content-type': 'text/html' }, + }) + }) + + const startConfig = makeStartConfig('/about', { retryCount: 2 }) + + await expect( + prerender({ startConfig, handler: { ...handler, request } }), + ).resolves.not.toThrow() + + // 1 initial attempt + 2 retries, third attempt succeeds + expect(request).toHaveBeenCalledTimes(3) + }) + + it('rethrows once retries are exhausted and failOnError is true', async () => { + const request = vi.fn(() => new Response('nope', { status: 500 })) + + const startConfig = makeStartConfig('/about', { + retryCount: 1, + failOnError: true, + }) + + await expect( + prerender({ startConfig, handler: { ...handler, request } }), + ).rejects.toThrow() + + // 1 initial attempt + 1 retry, both fail + expect(request).toHaveBeenCalledTimes(2) + }) + + it('does not throw when retries are exhausted and failOnError is false', async () => { + const request = vi.fn(() => new Response('nope', { status: 500 })) + + const startConfig = makeStartConfig('/about', { + retryCount: 1, + failOnError: false, + }) + + await expect( + prerender({ startConfig, handler: { ...handler, request } }), + ).resolves.not.toThrow() + + expect(request).toHaveBeenCalledTimes(2) + }) +}) From f48ed4bc020bd54f0b23b3ec3988eb74086e39b1 Mon Sep 17 00:00:00 2001 From: TLSRUF Date: Fri, 28 Aug 2026 21:23:56 +0900 Subject: [PATCH 2/2] test(start-plugin-core): remove any casts from prerender-retry.test.ts Type the vi.importActual() mocks and the test fixture's config object instead of using bare any, per CodeRabbit review feedback. --- .../start-plugin-core/tests/prerender-retry.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/start-plugin-core/tests/prerender-retry.test.ts b/packages/start-plugin-core/tests/prerender-retry.test.ts index b5530175f4f..1846923db77 100644 --- a/packages/start-plugin-core/tests/prerender-retry.test.ts +++ b/packages/start-plugin-core/tests/prerender-retry.test.ts @@ -1,8 +1,11 @@ import { describe, expect, it, vi } from 'vitest' import { prerender } from '../src/prerender' +import type { TanStackStartOutputConfig } from '../src/schema' +import type * as Utils from '../src/utils' +import type * as Fs from 'node:fs' vi.mock('../src/utils', async () => { - const actual = await vi.importActual('../src/utils') + const actual = await vi.importActual('../src/utils') return { ...actual, createLogger: () => ({ info: () => {}, warn: () => {}, error: () => {} }), @@ -11,7 +14,7 @@ vi.mock('../src/utils', async () => { // Mock fs to prevent actual file system operations vi.mock('node:fs', async () => { - const actual = await vi.importActual('node:fs') + const actual = await vi.importActual('node:fs') return { ...actual, promises: { @@ -46,7 +49,9 @@ function makeStartConfig( enabled: true, }, }, - } as any + // Only a subset of TanStackStartOutputConfig is needed to exercise + // prerender(); building the full parsed schema shape isn't worth it here. + } as unknown as TanStackStartOutputConfig } const handler = {