From 2208804301ac38373e46f5955451e5117eb976f6 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Jul 2026 16:39:38 -0700 Subject: [PATCH] fix(speech): keep segmented download resumes progressing --- .../model-manager-download-resume.test.ts | 121 ++++++++++++++++++ src/main/speech/model-manager.ts | 52 +++++--- 2 files changed, 153 insertions(+), 20 deletions(-) diff --git a/src/main/speech/model-manager-download-resume.test.ts b/src/main/speech/model-manager-download-resume.test.ts index 4472593e589..4f2d16470b3 100644 --- a/src/main/speech/model-manager-download-resume.test.ts +++ b/src/main/speech/model-manager-download-resume.test.ts @@ -235,6 +235,127 @@ describe('ModelManager download resume', () => { } }) + it('continues through more than eight advancing range segments', async () => { + const dir = mkdtempSync(join(tmpdir(), 'orca-model-resume-')) + try { + for (let offset = 1; offset <= 9; offset += 1) { + scriptRequest((sentHeaders) => { + expect(sentHeaders.range).toBe(`bytes=${offset}-`) + return { + statusCode: 206, + headers: { + 'content-length': '1', + 'content-range': `bytes ${offset}-${offset}/${PAYLOAD.length}` + }, + chunks: [PAYLOAD.subarray(offset, offset + 1)] + } + }) + } + scriptRequest((sentHeaders) => { + expect(sentHeaders.range).toBe('bytes=10-') + return { + statusCode: 206, + headers: { + 'content-length': '10', + 'content-range': `bytes 10-19/${PAYLOAD.length}` + }, + chunks: [PAYLOAD.subarray(10)] + } + }) + const manager = new ModelManager(dir) as unknown as ModelManagerInternals + const archivePath = join(dir, 'model.tar.bz2') + writeFileSync(archivePath, PAYLOAD.subarray(0, 1)) + + await manager.downloadArchiveWithRetry( + 'https://example.com/model.tar.bz2', + archivePath, + PAYLOAD.length, + 'm', + () => false, + new AbortController().signal + ) + + expect(netRequestMock).toHaveBeenCalledTimes(10) + expect(readFileSync(archivePath)).toEqual(PAYLOAD) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) + + it('bounds pathological advancing range segment requests', async () => { + const dir = mkdtempSync(join(tmpdir(), 'orca-model-resume-')) + try { + const manager = new ModelManager(dir) as unknown as ModelManagerInternals + const archivePath = join(dir, 'model.tar.bz2') + let bytesWritten = 0 + const downloadFileMock = vi.spyOn(manager, 'downloadFile').mockImplementation(() => { + bytesWritten += 1 + writeFileSync(archivePath, Buffer.alloc(bytesWritten)) + return Promise.resolve() + }) + + await expect( + manager.downloadArchiveWithRetry( + 'https://example.com/model.tar.bz2', + archivePath, + 300, + 'm', + () => false, + new AbortController().signal + ) + ).rejects.toThrow(/after 256 attempts/) + + expect(downloadFileMock).toHaveBeenCalledTimes(256) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) + + it('keeps the known archive total when Content-Range omits it', async () => { + const dir = mkdtempSync(join(tmpdir(), 'orca-model-resume-')) + try { + scriptRequest((sentHeaders) => { + expect(sentHeaders.range).toBe('bytes=10-') + return { + statusCode: 206, + headers: { + 'content-length': '5', + 'content-range': 'bytes 10-14/*' + }, + chunks: [PAYLOAD.subarray(10, 15)] + } + }) + scriptRequest((sentHeaders) => { + expect(sentHeaders.range).toBe('bytes=15-') + return { + statusCode: 206, + headers: { + 'content-length': '5', + 'content-range': 'bytes 15-19/*' + }, + chunks: [PAYLOAD.subarray(15)] + } + }) + const manager = new ModelManager(dir) as unknown as ModelManagerInternals + const archivePath = join(dir, 'model.tar.bz2') + writeFileSync(archivePath, PAYLOAD.subarray(0, 10)) + + await manager.downloadArchiveWithRetry( + 'https://example.com/model.tar.bz2', + archivePath, + PAYLOAD.length, + 'm', + () => false, + new AbortController().signal + ) + + expect(netRequestMock).toHaveBeenCalledTimes(2) + expect(readFileSync(archivePath)).toEqual(PAYLOAD) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) + it('rejects and discards a partial when Content-Range does not match the offset', async () => { const dir = mkdtempSync(join(tmpdir(), 'orca-model-resume-')) try { diff --git a/src/main/speech/model-manager.ts b/src/main/speech/model-manager.ts index 98be1d61603..fc83539bd34 100644 --- a/src/main/speech/model-manager.ts +++ b/src/main/speech/model-manager.ts @@ -51,7 +51,10 @@ const DOWNLOAD_IDLE_TIMEOUT_MS = 120_000 // request lets those connections finish instead of failing the same way on // every full restart. const DOWNLOAD_RETRY_DELAYS_MS = [1_000, 2_000, 4_000] -const MAX_DOWNLOAD_ATTEMPTS = 8 +const MAX_DOWNLOAD_FAILURES = 8 +// Why: successful bounded ranges may require more than the failure budget, but +// a server returning pathological tiny segments must not create an endless loop. +const MAX_ADVANCING_RANGE_SEGMENTS = 256 // Why: a longer server window should be surfaced for a later manual retry, // rather than leaving the settings download looking stuck for minutes. const MAX_RETRY_AFTER_MS = 120_000 @@ -450,11 +453,13 @@ export class ModelManager { isAborted: () => boolean, signal: AbortSignal ): Promise { - let attempt = 0 + let requestCount = 0 + let retryFailures = 0 + let advancingRangeSegments = 0 let noProgressStreak = 0 const totals: DownloadTotals = { totalBytes: expectedSize } for (;;) { - attempt += 1 + requestCount += 1 const offset = this.getPartialArchiveBytes(archivePath) // Why: the transport can fail after the final byte reached disk; the // caller's SHA-256 check is the authoritative completion test. @@ -487,18 +492,19 @@ export class ModelManager { const incompleteResponse = new Error( `Model download response ended at ${receivedBytes} of ${totals.totalBytes} bytes` ) - if (attempt >= MAX_DOWNLOAD_ATTEMPTS) { - throw describeInterruptedDownload( - incompleteResponse, - receivedBytes, - totals.totalBytes, - attempt - ) - } if (receivedBytes > offset) { // Why: some proxies return a valid but bounded range segment; request // the next segment immediately instead of failing checksum or waiting. noProgressStreak = 0 + advancingRangeSegments += 1 + if (advancingRangeSegments >= MAX_ADVANCING_RANGE_SEGMENTS) { + throw describeInterruptedDownload( + incompleteResponse, + receivedBytes, + totals.totalBytes, + requestCount + ) + } continue } const retryableIncompleteResponse = incompleteResponse as HttpStatusError @@ -516,11 +522,12 @@ export class ModelManager { if (!isRetryableDownloadError(err)) { throw err } + retryFailures += 1 if ( - attempt >= MAX_DOWNLOAD_ATTEMPTS || + retryFailures >= MAX_DOWNLOAD_FAILURES || noProgressStreak > DOWNLOAD_RETRY_DELAYS_MS.length ) { - throw describeInterruptedDownload(err, receivedBytes, totals.totalBytes, attempt) + throw describeInterruptedDownload(err, receivedBytes, totals.totalBytes, requestCount) } const retryAfterMs = (err as HttpStatusError).retryAfterMs if (retryAfterMs !== undefined && retryAfterMs > MAX_RETRY_AFTER_MS) { @@ -529,7 +536,11 @@ export class ModelManager { `HTTP ${statusCode}; server requested retry after ${Math.ceil(retryAfterMs / 1_000)} seconds` ) } - console.warn(`[speech] Model download attempt ${attempt} failed, retrying:`, modelId, err) + console.warn( + `[speech] Model download attempt ${requestCount} failed, retrying:`, + modelId, + err + ) await sleepUnlessAborted( retryAfterMs ?? DOWNLOAD_RETRY_DELAYS_MS[ @@ -721,12 +732,13 @@ export class ModelManager { // Why: a 200 despite our Range request means the server restarted the // transfer from byte zero, so the partial file must be overwritten. const progressBase = resumed ? resumeOffset : 0 - const totalSize = - resumed && contentRange?.totalBytes !== undefined - ? contentRange.totalBytes - : parsedLength > 0 - ? progressBase + parsedLength - : expectedSize + // Why: Content-Length on a 206 describes only this segment; when + // Content-Range uses '*', retain the known complete archive size. + const totalSize = resumed + ? (contentRange?.totalBytes ?? totals?.totalBytes ?? expectedSize) + : parsedLength > 0 + ? parsedLength + : expectedSize if (totals) { totals.totalBytes = totalSize }