starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-18 08:03:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(speech): don't abandon a download that keeps making progress

Browse Source 
The retry loop gave up after 8 total failures regardless of progress, so
a large model (e.g. the ~1GB Paraformer) over a network that resets every
few hundred MB was abandoned mid-download even though every attempt was
resuming and advancing — the exact 'stuck then fails' symptom, just later.

Replace the all-time failure budget and the separate advancing-segment
cap with a single rule: give up only on a genuine stall (repeated
attempts with no forward progress) or an absolute request ceiling that
bounds a pathological tiny-segment server. Any download that keeps
advancing now runs to completion.

Verified live under Electron 43: a transfer reset mid-stream on every
attempt but advancing each time previously gave up at 67% after 8
attempts; it now completes. All prior resume/cancel/timeout scenarios
still pass.
This commit is contained in:
Brennan Benson
2026-07-14 17:46:18 -07:00
parent 2208804301
commit 4201dd99bf
2 changed files with 77 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/speech/model-manager-download-resume.test.ts
+51 -4
View File
@@ -282,12 +282,14 @@ describe('ModelManager download resume', () => {
}
})
it('bounds pathological advancing range segment requests', async () => {
it('bounds a server that advances by pathologically tiny segments forever', 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
// Advances one byte per request against a total larger than the request
// ceiling, so it makes forward progress forever without ever completing.
const downloadFileMock = vi.spyOn(manager, 'downloadFile').mockImplementation(() => {
bytesWritten += 1
writeFileSync(archivePath, Buffer.alloc(bytesWritten))
@@ -298,19 +300,64 @@ describe('ModelManager download resume', () => {
manager.downloadArchiveWithRetry(
'https://example.com/model.tar.bz2',
archivePath,
300,
1_000_000,
'm',
() => false,
new AbortController().signal
)
).rejects.toThrow(/after 256 attempts/)
).rejects.toThrow(/too many download requests/)
expect(downloadFileMock).toHaveBeenCalledTimes(256)
// The absolute request ceiling (MAX_TOTAL_DOWNLOAD_REQUESTS) bounds it.
expect(downloadFileMock).toHaveBeenCalledTimes(4_096)
} finally {
rmSync(dir, { recursive: true, force: true })
}
})
it('keeps resuming a download that advances across many mid-stream drops', async () => {
vi.useFakeTimers()
const dir = mkdtempSync(join(tmpdir(), 'orca-model-resume-'))
try {
// Every attempt delivers a small slice then drops mid-stream — the
// classic "dies partway, resumes" pattern. Because each attempt makes
// forward progress, the download must complete no matter how many drops
// it takes (regression guard: a fixed failure budget used to abandon a
// still-advancing large download around attempt 8).
const manager = new ModelManager(dir) as unknown as ModelManagerInternals
const archivePath = join(dir, 'model.tar.bz2')
const SLICE = 2
let delivered = 0
const downloadFileMock = vi.spyOn(manager, 'downloadFile').mockImplementation(() => {
delivered = Math.min(delivered + SLICE, PAYLOAD.length)
writeFileSync(archivePath, PAYLOAD.subarray(0, delivered))
return Promise.reject(new Error('net::ERR_CONNECTION_RESET'))
})
const download = manager.downloadArchiveWithRetry(
'https://example.com/model.tar.bz2',
archivePath,
PAYLOAD.length,
'm',
() => false,
new AbortController().signal
)
const outcome = download.then(
() => 'resolved',
(error: unknown) => (error instanceof Error ? error.message : String(error))
)
// Drive the per-attempt backoff (1s each, since progress resets the
// stall counter) well past the ten attempts this needs.
await vi.advanceTimersByTimeAsync(30_000)
await expect(outcome).resolves.toBe('resolved')
expect(downloadFileMock).toHaveBeenCalledTimes(PAYLOAD.length / SLICE)
expect(readFileSync(archivePath)).toEqual(PAYLOAD)
} finally {
vi.useRealTimers()
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 {
src/main/speech/model-manager.ts
+26 -20
View File
@@ -51,10 +51,17 @@ 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_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: abandon a download only after this many CONSECUTIVE attempts make no
// forward progress. A resume that keeps advancing (even across many mid-stream
// drops) is never given up on — that is the whole point of resumable downloads
// for large models over flaky networks, and a fixed all-time failure budget
// would wrongly kill a steadily-progressing large download.
const MAX_NO_PROGRESS_ATTEMPTS = DOWNLOAD_RETRY_DELAYS_MS.length + 1
// Why: an absolute backstop so a server handing back pathologically tiny (or
// zero-net) segments cannot loop forever; sized to cover the largest (~1GB)
// model even if a proxy caps each range response near ~256KB, far below any
// real CDN chunk.
const MAX_TOTAL_DOWNLOAD_REQUESTS = 4_096
// 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
@@ -454,8 +461,6 @@ export class ModelManager {
signal: AbortSignal
): Promise<void> {
let requestCount = 0
let retryFailures = 0
let advancingRangeSegments = 0
let noProgressStreak = 0
const totals: DownloadTotals = { totalBytes: expectedSize }
for (;;) {
@@ -466,6 +471,16 @@ export class ModelManager {
if (offset === totals.totalBytes) {
return
}
// Why: absolute backstop against a server that never lets the download
// finish; the stall and abort paths handle the common cases first.
if (requestCount > MAX_TOTAL_DOWNLOAD_REQUESTS) {
throw describeInterruptedDownload(
new Error('too many download requests'),
offset,
totals.totalBytes,
requestCount - 1
)
}
try {
// Why: each attempt restarts from the canonical URL rather than the
// last redirect target, because CDN redirect URLs are signed and expire.
@@ -495,16 +510,9 @@ export class ModelManager {
if (receivedBytes > offset) {
// Why: some proxies return a valid but bounded range segment; request
// the next segment immediately instead of failing checksum or waiting.
// Forward progress resets the stall counter; the total-request ceiling
// at the loop top bounds a pathological tiny-segment server.
noProgressStreak = 0
advancingRangeSegments += 1
if (advancingRangeSegments >= MAX_ADVANCING_RANGE_SEGMENTS) {
throw describeInterruptedDownload(
incompleteResponse,
receivedBytes,
totals.totalBytes,
requestCount
)
}
continue
}
const retryableIncompleteResponse = incompleteResponse as HttpStatusError
@@ -522,11 +530,9 @@ export class ModelManager {
if (!isRetryableDownloadError(err)) {
throw err
}
retryFailures += 1
if (
retryFailures >= MAX_DOWNLOAD_FAILURES ||
noProgressStreak > DOWNLOAD_RETRY_DELAYS_MS.length
) {
// Why: give up only on a genuine stall (repeated no forward progress);
// a download that keeps advancing across drops is allowed to continue.
if (noProgressStreak >= MAX_NO_PROGRESS_ATTEMPTS) {
throw describeInterruptedDownload(err, receivedBytes, totals.totalBytes, requestCount)
}
const retryAfterMs = (err as HttpStatusError).retryAfterMs