From 8397781f77872f3b0adb36c579d91eb488a471f0 Mon Sep 17 00:00:00 2001 From: CyberWolf Date: Fri, 6 Nov 2026 02:59:42 +0800 Subject: [PATCH] =?UTF-8?q?fix(converter):=20=E8=BE=93=E5=87=BA=E6=88=AA?= =?UTF-8?q?=E6=96=AD=E6=9C=80=E5=A4=9A=E9=87=8D=E8=AF=95=201=20=E6=AC=A1?= =?UTF-8?q?=EF=BC=8C=E9=81=BF=E5=85=8D=E6=8C=89=204=20=E5=80=8D=E8=AE=A1?= =?UTF-8?q?=E8=B4=B9=20(#96)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 截断的错误文案含 "LLM"，被归为普通 LLM 错误，按 maxRetries 最多请求 4 次， 而每次都要全额付费。总是截断的页面费用变成 4 倍，最后仍然失败。 - 新增 OutputTruncatedError 和 TRUNCATION_ERROR，按类型而非文案识别， 不会误伤图片损坏时的 "The image file is truncated" - 新增配置 converter.maxTruncationRetries = 1，按页统计截断次数，与 attempt 分开：其他错误既不占用也不增加这个额度，其重试次数不变 - 文案从 "Try again" 改为提示换用输出上限更高的模型 --- docs/CONVERTER_WORKER_DESIGN.md | 1 + .../application/workers/ConverterWorker.ts | 37 ++++++- .../workers/__tests__/ConverterWorker.test.ts | 96 ++++++++++++++++++- .../config/__tests__/worker.config.test.ts | 4 + .../infrastructure/config/worker.config.ts | 5 + 5 files changed, 136 insertions(+), 7 deletions(-) diff --git a/docs/CONVERTER_WORKER_DESIGN.md b/docs/CONVERTER_WORKER_DESIGN.md index 591c37e..578ce26 100644 --- a/docs/CONVERTER_WORKER_DESIGN.md +++ b/docs/CONVERTER_WORKER_DESIGN.md @@ -921,6 +921,7 @@ eventBus.onTaskEvent('task:*', (data) => { enum ErrorType { NETWORK_ERROR = 'network_error', // 网络错误（可重试） LLM_ERROR = 'llm_error', // LLM API 错误（可重试） + TRUNCATION_ERROR = 'truncation_error', // 输出被截断（只重试 1 次：每次都按全价计费） RATE_LIMIT_ERROR = 'rate_limit_error', // 速率限制（可重试，延迟更长） QUOTA_EXCEEDED_ERROR = 'quota_exceeded_error', // 配额超限（不可重试） CONFIG_ERROR = 'config_error', // 配置错误（不可重试） diff --git a/src/core/application/workers/ConverterWorker.ts b/src/core/application/workers/ConverterWorker.ts index b3a7a19..0660b2d 100644 --- a/src/core/application/workers/ConverterWorker.ts +++ b/src/core/application/workers/ConverterWorker.ts @@ -15,6 +15,7 @@ import { FINISH_REASON_TRUNCATED } from '../../domain/llm/ILLMClient.js'; enum ErrorType { NETWORK_ERROR = 'network_error', LLM_ERROR = 'llm_error', + TRUNCATION_ERROR = 'truncation_error', RATE_LIMIT_ERROR = 'rate_limit_error', QUOTA_EXCEEDED_ERROR = 'quota_exceeded_error', CONFIG_ERROR = 'config_error', @@ -23,6 +24,20 @@ enum ErrorType { UNKNOWN_ERROR = 'unknown_error', } +/** + * The model hit its output token limit. Identified by type rather than by message, as other + * errors mention truncation too (a truncated image file, for one). + */ +class OutputTruncatedError extends Error { + constructor() { + super( + 'LLM output was truncated at the output token limit, so the page content would be incomplete. ' + + 'Switch to a model with a higher output token limit.' + ); + this.name = 'OutputTruncatedError'; + } +} + /** * Result of a successful page conversion. */ @@ -70,6 +85,7 @@ interface PageWithTask { */ export class ConverterWorker extends WorkerBase { private readonly maxRetries: number; + private readonly maxTruncationRetries: number; private readonly maxContentLength: number; private readonly pollInterval: number; private readonly retryDelayBase: number; @@ -78,6 +94,7 @@ export class ConverterWorker extends WorkerBase { constructor() { super(); this.maxRetries = WORKER_CONFIG.converter.maxRetries; + this.maxTruncationRetries = WORKER_CONFIG.converter.maxTruncationRetries; this.maxContentLength = WORKER_CONFIG.converter.maxContentLength; this.pollInterval = WORKER_CONFIG.converter.pollInterval; this.retryDelayBase = WORKER_CONFIG.converter.retryDelayBase; @@ -247,6 +264,7 @@ export class ConverterWorker extends WorkerBase { */ private async processPageWithRetry(page: PageWithTask): Promise { let lastError: Error | null = null; + let truncations = 0; for (let attempt = 0; attempt <= this.maxRetries; attempt++) { try { @@ -272,6 +290,14 @@ export class ConverterWorker extends WorkerBase { break; } + // Truncated responses are billed in full, so they get a budget of their own. + // Counted apart from attempt so that other errors neither use it up nor gain from it. + if (errorType === ErrorType.TRUNCATION_ERROR) truncations++; + if (truncations > this.maxTruncationRetries) { + console.log(`[Converter-${this.workerId.slice(0, 8)}] Output still truncated, marking as failed`); + break; + } + // Check if we have retries left if (attempt < this.maxRetries) { // Check if task has been cancelled before retrying @@ -322,12 +348,8 @@ export class ConverterWorker extends WorkerBase { // Step 5: Validate content // Truncation comes first: the output limit can also leave the content empty. - // Retryable: a model stuck repeating itself can finish normally on another attempt. if (response.finishReason === FINISH_REASON_TRUNCATED) { - throw new Error( - 'LLM output was truncated at the output token limit, so the page content would be incomplete. ' + - 'Try again with a model or provider setting that allows longer output.' - ); + throw new OutputTruncatedError(); } if (!response.content || response.content.trim().length === 0) { @@ -744,6 +766,10 @@ export class ConverterWorker extends WorkerBase { * Analyze error to determine type. */ private analyzeError(error: Error): ErrorType { + if (error instanceof OutputTruncatedError) { + return ErrorType.TRUNCATION_ERROR; + } + const message = error.message.toLowerCase(); // Network errors @@ -825,6 +851,7 @@ export class ConverterWorker extends WorkerBase { switch (errorType) { case ErrorType.NETWORK_ERROR: case ErrorType.LLM_ERROR: + case ErrorType.TRUNCATION_ERROR: case ErrorType.RATE_LIMIT_ERROR: case ErrorType.TIMEOUT_ERROR: case ErrorType.UNKNOWN_ERROR: diff --git a/src/core/application/workers/__tests__/ConverterWorker.test.ts b/src/core/application/workers/__tests__/ConverterWorker.test.ts index ba4281b..60a0fa8 100644 --- a/src/core/application/workers/__tests__/ConverterWorker.test.ts +++ b/src/core/application/workers/__tests__/ConverterWorker.test.ts @@ -51,6 +51,7 @@ vi.mock('../../../infrastructure/config/worker.config.js', () => ({ WORKER_CONFIG: { converter: { maxRetries: 3, + maxTruncationRetries: 1, maxContentLength: 500000, pollInterval: 100, // Short for tests retryDelayBase: 10, // Short for tests @@ -522,6 +523,20 @@ describe('ConverterWorker', () => { }); }); + it('should identify truncation by the error type, not by the word in a message', async () => { + vi.mocked(modelLogic.transformImageMessage).mockResolvedValue([] as any); + vi.mocked(modelLogic.completion).mockResolvedValue({ + content: '# Partial', + finishReason: FINISH_REASON_TRUNCATED, + } as any); + const page = { task: 'task123', page: 1, provider: 1, model: 'test-model' }; + const truncated = await (worker as any).convertPage(page).catch((error: Error) => error); + + expect((worker as any).analyzeError(truncated)).toBe('truncation_error'); + // What ImageReencoder throws for a damaged page image: no output was billed + expect((worker as any).analyzeError(new Error('The image file is truncated'))).toBe('unknown_error'); + }); + it('should return unknown_error for unrecognized errors', () => { const error = new Error('Something unexpected happened'); const result = (worker as any).analyzeError(error); @@ -534,6 +549,7 @@ describe('ConverterWorker', () => { const retryableTypes = [ 'network_error', 'llm_error', + 'truncation_error', 'rate_limit_error', 'timeout_error', 'unknown_error', @@ -1076,6 +1092,11 @@ describe('ConverterWorker', () => { }); describe('when the output is truncated', () => { + // clearAllMocks keeps queued once-values, which a failing test would pass on to the next + beforeEach(() => { + vi.mocked(modelLogic.completion).mockReset(); + }); + const stubPageTransaction = (pages: number) => { const tx = { taskDetail: { @@ -1103,17 +1124,19 @@ describe('ConverterWorker', () => { const truncated = { content: '# Partial', finishReason: FINISH_REASON_TRUNCATED, rawResponse: {} } as any; - it('should retry, then fail the page with a visible reason and without the partial content', async () => { + it('should retry once, then fail the page with a visible reason and without the partial content', async () => { const tx = stubPageTransaction(10); vi.mocked(modelLogic.completion).mockResolvedValue(truncated); await (worker as any).processPageWithRetry(mockPage); - expect(modelLogic.completion).toHaveBeenCalledTimes(4); // first attempt + maxRetries + expect(modelLogic.completion).toHaveBeenCalledTimes(2); // first attempt + maxTruncationRetries + expect(prisma.taskDetail.update).toHaveBeenCalledTimes(1); // the retry count, for that one retry expect(tx.taskDetail.update).toHaveBeenCalledTimes(1); const { data } = tx.taskDetail.update.mock.calls[0][0]; expect(data.status).toBe(PageStatus.FAILED); expect(data.error).toContain('truncated'); + expect(data.error).toContain('Switch to a model with a higher output token limit'); expect(data.content).toBeUndefined(); expect(tx.task.update).toHaveBeenCalledWith( expect.objectContaining({ data: { failed_count: { increment: 1 } } }) @@ -1142,12 +1165,14 @@ describe('ConverterWorker', () => { it('should complete the page when a retry finishes normally', async () => { const tx = stubPageTransaction(10); + const analyzeError = vi.spyOn(worker as any, 'analyzeError'); vi.mocked(modelLogic.completion) .mockResolvedValueOnce(truncated) .mockResolvedValueOnce({ content: '# Complete', finishReason: 'stop', rawResponse: {} } as any); await (worker as any).processPageWithRetry(mockPage); + expect(analyzeError).toHaveReturnedWith('truncation_error'); expect(modelLogic.completion).toHaveBeenCalledTimes(2); expect(tx.taskDetail.update).toHaveBeenCalledTimes(1); expect(tx.taskDetail.update.mock.calls[0][0].data).toMatchObject({ @@ -1156,6 +1181,72 @@ describe('ConverterWorker', () => { }); }); + // Each case runs a truncated page first: on the same worker, one kind of failure gets the + // short budget and the other still gets what it always had. + it.each([ + ['a network error', 4, () => Promise.reject(new Error('Network error'))], + ['a rate limit', 4, () => Promise.reject(new Error('429 Too Many Requests'))], + ['a timeout', 4, () => Promise.reject(new Error('Request timed out'))], + ['an LLM error', 4, () => Promise.reject(new Error('OpenAI API returned 500'))], + ['an unknown error', 4, () => Promise.reject(new Error('Something unexpected happened'))], + ['an empty response', 4, () => Promise.resolve({ content: ' ', finishReason: 'stop', rawResponse: {} })], + ['a config error', 1, () => Promise.reject(new Error('Invalid API key'))], + ['a quota error', 1, () => Promise.reject(new Error('insufficient_quota'))], + ] as const)('should leave the requests made for %s at %i', async (_name, requests, respond) => { + stubPageTransaction(10); + vi.mocked(modelLogic.completion).mockResolvedValue(truncated); + await (worker as any).processPageWithRetry(mockPage); + expect(modelLogic.completion).toHaveBeenCalledTimes(2); + + vi.mocked(modelLogic.completion).mockReset(); + vi.mocked(modelLogic.completion).mockImplementation(respond as any); + await (worker as any).processPageWithRetry(mockPage); + + expect(modelLogic.completion).toHaveBeenCalledTimes(requests); + }); + + it('should give each page its own budget', async () => { + stubPageTransaction(10); + vi.mocked(modelLogic.completion) + .mockResolvedValueOnce(truncated) + .mockResolvedValueOnce({ content: '# Complete', finishReason: 'stop', rawResponse: {} } as any) + .mockResolvedValue(truncated); + + await (worker as any).processPageWithRetry(mockPage); + await (worker as any).processPageWithRetry(mockPage); + + expect(modelLogic.completion).toHaveBeenCalledTimes(4); // 2 for each page + }); + + it('should stop at the second truncation even with another error in between', async () => { + const tx = stubPageTransaction(10); + vi.mocked(modelLogic.completion) + .mockResolvedValueOnce(truncated) + .mockRejectedValueOnce(new Error('Network error')) + .mockResolvedValue(truncated); + + await (worker as any).processPageWithRetry(mockPage); + + expect(modelLogic.completion).toHaveBeenCalledTimes(3); + expect(tx.taskDetail.update.mock.calls[0][0].data.error).toContain('truncated'); + }); + + it('should still retry a first truncation after other errors used up most attempts', async () => { + const tx = stubPageTransaction(10); + const analyzeError = vi.spyOn(worker as any, 'analyzeError'); + vi.mocked(modelLogic.completion) + .mockRejectedValueOnce(new Error('Network error')) + .mockRejectedValueOnce(new Error('Network error')) + .mockResolvedValueOnce(truncated) + .mockResolvedValueOnce({ content: '# Complete', finishReason: 'stop', rawResponse: {} } as any); + + await (worker as any).processPageWithRetry(mockPage); + + expect(analyzeError).toHaveReturnedWith('truncation_error'); + expect(modelLogic.completion).toHaveBeenCalledTimes(4); + expect(tx.taskDetail.update.mock.calls[0][0].data.status).toBe(PageStatus.COMPLETED); + }); + it('should fail the page for a truncated response from a real client', async () => { const tx = stubPageTransaction(10); vi.mocked(modelLogic.completion).mockImplementation(async (_provider, options) => { @@ -1170,6 +1261,7 @@ describe('ConverterWorker', () => { await (worker as any).processPageWithRetry(mockPage); + expect(modelLogic.completion).toHaveBeenCalledTimes(2); expect(tx.taskDetail.update.mock.calls[0][0].data.status).toBe(PageStatus.FAILED); }); }); diff --git a/src/core/infrastructure/config/__tests__/worker.config.test.ts b/src/core/infrastructure/config/__tests__/worker.config.test.ts index 8293eb0..8a5fa8a 100644 --- a/src/core/infrastructure/config/__tests__/worker.config.test.ts +++ b/src/core/infrastructure/config/__tests__/worker.config.test.ts @@ -49,6 +49,10 @@ describe('WORKER_CONFIG', () => { expect(WORKER_CONFIG.converter.maxRetries).toBeGreaterThanOrEqual(1) }) + it('should allow one retry after a truncated response', () => { + expect(WORKER_CONFIG.converter.maxTruncationRetries).toBe(1) + }) + it('should have valid retryDelayBase', () => { expect(WORKER_CONFIG.converter.retryDelayBase).toBe(1000) expect(WORKER_CONFIG.converter.retryDelayBase).toBeGreaterThan(0) diff --git a/src/core/infrastructure/config/worker.config.ts b/src/core/infrastructure/config/worker.config.ts index 62317af..0b3e830 100644 --- a/src/core/infrastructure/config/worker.config.ts +++ b/src/core/infrastructure/config/worker.config.ts @@ -39,6 +39,11 @@ export const WORKER_CONFIG = { timeout: 120000, // 2 minutes /** Maximum retry attempts for transient errors */ maxRetries: 3, + /** + * Maximum retries after a truncated response. It costs as much as a full one and is likely to + * repeat, but a model stuck repeating itself can recover, hence not 0. + */ + maxTruncationRetries: 1, /** Base retry delay in milliseconds (exponential backoff) */ retryDelayBase: 1000, /** Maximum content length in characters */