Fix speech model downloads behind proxies (#5210)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo Hong 2026-06-11 15:23:35 -07:00 committed by GitHub
parent 7e73cd4719
commit e2b2bede91
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 279 additions and 158 deletions

View File

@ -5,24 +5,22 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
import { SPEECH_MODEL_CATALOG } from './model-catalog'
import { ModelManager } from './model-manager'
const { httpsGetMock } = vi.hoisted(() => ({
httpsGetMock: vi.fn()
const { netRequestMock } = vi.hoisted(() => ({
netRequestMock: vi.fn()
}))
vi.mock('electron', () => ({
app: {
getPath: () => '/tmp/orca-speech-models-test'
},
net: {
request: netRequestMock
}
}))
vi.mock('https', async () => {
const actual = await vi.importActual('https')
return { ...(actual as Record<string, unknown>), get: httpsGetMock }
})
describe('ModelManager download failures', () => {
beforeEach(() => {
httpsGetMock.mockReset()
netRequestMock.mockReset()
})
it('rejects failed model downloads so the caller can surface the error', async () => {
@ -31,8 +29,15 @@ describe('ModelManager download failures', () => {
const manifest = SPEECH_MODEL_CATALOG[0]
const errorHandlers: ((err: Error) => void)[] = []
const request = {
destroy: vi.fn(() => request),
setTimeout: vi.fn(() => request),
abort: vi.fn(() => request),
end: vi.fn(() => {
queueMicrotask(() => {
for (const handler of errorHandlers) {
handler(new Error('network down'))
}
})
return request
}),
on: vi.fn((event: string, cb: (err: Error) => void) => {
if (event === 'error') {
errorHandlers.push(cb)
@ -49,14 +54,7 @@ describe('ModelManager download failures', () => {
return request
})
}
httpsGetMock.mockImplementation(() => {
queueMicrotask(() => {
for (const handler of errorHandlers) {
handler(new Error('network down'))
}
})
return request
})
netRequestMock.mockReturnValue(request)
const manager = new ModelManager(dir)
await expect(manager.downloadModel(manifest.id)).rejects.toThrow('network down')

View File

@ -5,21 +5,19 @@ import { PassThrough } from 'stream'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { ModelManager } from './model-manager'
const { httpsGetMock } = vi.hoisted(() => ({
httpsGetMock: vi.fn()
const { netRequestMock } = vi.hoisted(() => ({
netRequestMock: vi.fn()
}))
vi.mock('electron', () => ({
app: {
getPath: () => '/tmp/orca-speech-models-test'
},
net: {
request: netRequestMock
}
}))
vi.mock('https', async () => {
const actual = await vi.importActual('https')
return { ...(actual as Record<string, unknown>), get: httpsGetMock }
})
type ModelManagerInternals = {
downloadFile: (
url: string,
@ -33,7 +31,7 @@ type ModelManagerInternals = {
describe('ModelManager stream cleanup', () => {
beforeEach(() => {
httpsGetMock.mockReset()
netRequestMock.mockReset()
})
it('removes response progress listeners after a model download finishes', async () => {
@ -45,16 +43,24 @@ describe('ModelManager stream cleanup', () => {
}
response.statusCode = 200
response.headers = { 'content-length': '4' }
const responseHandlers: ((response: unknown) => void)[] = []
const request = {
destroy: vi.fn(() => request),
setTimeout: vi.fn(() => request),
on: vi.fn(() => request),
abort: vi.fn(() => request),
end: vi.fn(() => {
for (const handler of responseHandlers) {
handler(response)
}
return request
}),
on: vi.fn((event: string, cb: (response: unknown) => void) => {
if (event === 'response') {
responseHandlers.push(cb)
}
return request
}),
off: vi.fn(() => request)
}
httpsGetMock.mockImplementation((_url: URL, cb: (response: unknown) => void) => {
cb(response)
return request
})
netRequestMock.mockReturnValue(request)
const manager = new ModelManager(dir) as unknown as ModelManagerInternals
const download = manager.downloadFile(

View File

@ -6,15 +6,18 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
import { SPEECH_MODEL_CATALOG } from './model-catalog'
import { ModelManager } from './model-manager'
const { hasOpenAiSpeechApiKeyMock, httpsGetMock, spawnMock } = vi.hoisted(() => ({
const { hasOpenAiSpeechApiKeyMock, netRequestMock, spawnMock } = vi.hoisted(() => ({
hasOpenAiSpeechApiKeyMock: vi.fn(),
httpsGetMock: vi.fn(),
netRequestMock: vi.fn(),
spawnMock: vi.fn()
}))
vi.mock('electron', () => ({
app: {
getPath: () => '/tmp/orca-speech-models-test'
},
net: {
request: netRequestMock
}
}))
@ -23,11 +26,6 @@ vi.mock('child_process', async () => {
return { ...(actual as Record<string, unknown>), spawn: spawnMock }
})
vi.mock('https', async () => {
const actual = await vi.importActual('https')
return { ...(actual as Record<string, unknown>), get: httpsGetMock }
})
vi.mock('./openai-api-key-store', () => ({
hasOpenAiSpeechApiKey: hasOpenAiSpeechApiKeyMock
}))
@ -52,7 +50,7 @@ type ModelManagerInternals = {
describe('ModelManager', () => {
beforeEach(() => {
httpsGetMock.mockReset()
netRequestMock.mockReset()
hasOpenAiSpeechApiKeyMock.mockReset()
hasOpenAiSpeechApiKeyMock.mockReturnValue(false)
spawnMock.mockReset()
@ -129,23 +127,30 @@ describe('ModelManager', () => {
try {
const manifest = SPEECH_MODEL_CATALOG[0]
const errorHandlers: ((err: Error) => void)[] = []
const timeoutHandlers: (() => void)[] = []
const responseHandlers: ((response: unknown) => void)[] = []
const redirectHandlers: ((
statusCode: number,
method: string,
redirectUrl: string
) => void)[] = []
const request = {
destroy: vi.fn((err?: Error) => {
abort: vi.fn(() => {
queueMicrotask(() => {
for (const handler of errorHandlers) {
handler(err ?? new Error('destroyed'))
handler(new Error('Aborted'))
}
})
return request
}),
setTimeout: vi.fn((_ms: number, cb: () => void) => {
timeoutHandlers.push(cb)
return request
}),
on: vi.fn((event: string, cb: (err: Error) => void) => {
if (event === 'error') {
errorHandlers.push(cb)
} else if (event === 'response') {
responseHandlers.push(cb as unknown as (response: unknown) => void)
} else if (event === 'redirect') {
redirectHandlers.push(
cb as unknown as (statusCode: number, method: string, redirectUrl: string) => void
)
}
return request
}),
@ -156,75 +161,154 @@ describe('ModelManager', () => {
errorHandlers.splice(index, 1)
}
}
if (event === 'timeout') {
const index = timeoutHandlers.indexOf(cb as () => void)
if (event === 'response') {
const index = responseHandlers.indexOf(cb as (response: unknown) => void)
if (index !== -1) {
timeoutHandlers.splice(index, 1)
responseHandlers.splice(index, 1)
}
}
if (event === 'redirect') {
const index = redirectHandlers.indexOf(
cb as (statusCode: number, method: string, redirectUrl: string) => void
)
if (index !== -1) {
redirectHandlers.splice(index, 1)
}
}
return request
})
}),
end: vi.fn(() => request)
}
httpsGetMock.mockImplementation(
(
_url: URL,
options: { signal?: AbortSignal } | ((response: unknown) => void),
_cb?: (response: unknown) => void
) => {
if (typeof options !== 'function') {
options.signal?.addEventListener('abort', () => request.destroy(new Error('Aborted')), {
once: true
})
}
return request
}
)
netRequestMock.mockReturnValue(request)
const manager = new ModelManager(dir)
const download = manager.downloadModel(manifest.id)
manager.cancelDownload(manifest.id)
await expect(download).resolves.toBeUndefined()
expect(request.destroy).toHaveBeenCalledWith(expect.any(Error))
expect(netRequestMock).toHaveBeenCalledWith({
method: 'GET',
url: expect.stringMatching(/^https:\/\//)
})
expect(request.end).toHaveBeenCalled()
expect(request.abort).toHaveBeenCalled()
expect(request.off).toHaveBeenCalledWith('error', expect.any(Function))
expect(request.off).toHaveBeenCalledWith('timeout', expect.any(Function))
expect(request.off).toHaveBeenCalledWith('response', expect.any(Function))
expect(request.off).toHaveBeenCalledWith('redirect', expect.any(Function))
expect(errorHandlers).toHaveLength(0)
expect(timeoutHandlers).toHaveLength(0)
expect(responseHandlers).toHaveLength(0)
expect(redirectHandlers).toHaveLength(0)
expect((await manager.getModelState(manifest.id)).status).toBe('not-downloaded')
} finally {
rmSync(dir, { recursive: true, force: true })
}
})
it('settles immediately when the abort signal fires before a response', async () => {
vi.useFakeTimers()
const dir = mkdtempSync(join(tmpdir(), 'orca-model-manager-'))
try {
const errorHandlers: ((err: Error) => void)[] = []
const responseHandlers: ((response: unknown) => void)[] = []
const redirectHandlers: ((
statusCode: number,
method: string,
redirectUrl: string
) => void)[] = []
const request = {
abort: vi.fn(() => request),
on: vi.fn((event: string, cb: (err: Error) => void) => {
if (event === 'error') {
errorHandlers.push(cb)
} else if (event === 'response') {
responseHandlers.push(cb as unknown as (response: unknown) => void)
} else if (event === 'redirect') {
redirectHandlers.push(
cb as unknown as (statusCode: number, method: string, redirectUrl: string) => void
)
}
return request
}),
off: vi.fn((event: string, cb: ((err: Error) => void) | (() => void)) => {
if (event === 'error') {
const index = errorHandlers.indexOf(cb as (err: Error) => void)
if (index !== -1) {
errorHandlers.splice(index, 1)
}
}
if (event === 'response') {
const index = responseHandlers.indexOf(cb as (response: unknown) => void)
if (index !== -1) {
responseHandlers.splice(index, 1)
}
}
if (event === 'redirect') {
const index = redirectHandlers.indexOf(
cb as (statusCode: number, method: string, redirectUrl: string) => void
)
if (index !== -1) {
redirectHandlers.splice(index, 1)
}
}
return request
}),
end: vi.fn(() => request)
}
netRequestMock.mockReturnValue(request)
const controller = new AbortController()
const manager = new ModelManager(dir) as unknown as ModelManagerInternals
const download = manager.downloadFile(
'https://example.com/model.tar.bz2',
join(dir, 'model.tar.bz2'),
1,
'm',
() => true,
controller.signal
)
const outcomePromise = download.then(
() => 'resolved',
(error) => (error instanceof Error ? error.message : String(error))
)
controller.abort()
await vi.advanceTimersByTimeAsync(0)
await expect(outcomePromise).resolves.toBe('Aborted')
expect(request.abort).toHaveBeenCalled()
expect(request.off).toHaveBeenCalledWith('error', expect.any(Function))
expect(request.off).toHaveBeenCalledWith('response', expect.any(Function))
expect(request.off).toHaveBeenCalledWith('redirect', expect.any(Function))
expect(errorHandlers).toHaveLength(0)
expect(responseHandlers).toHaveLength(0)
expect(redirectHandlers).toHaveLength(0)
} finally {
vi.useRealTimers()
rmSync(dir, { recursive: true, force: true })
}
})
it('times out a model download request that never responds', async () => {
vi.useFakeTimers()
const dir = mkdtempSync(join(tmpdir(), 'orca-model-manager-'))
try {
const errorHandlers: ((err: Error) => void)[] = []
const timeoutHandlers: (() => void)[] = []
const responseHandlers: ((response: unknown) => void)[] = []
const redirectHandlers: ((
statusCode: number,
method: string,
redirectUrl: string
) => void)[] = []
const request = {
destroy: vi.fn((err?: Error) => {
if (err) {
queueMicrotask(() => {
for (const handler of errorHandlers) {
handler(err)
}
})
}
return request
}),
setTimeout: vi.fn((ms: number, cb: () => void) => {
timeoutHandlers.push(cb)
setTimeout(() => {
for (const handler of timeoutHandlers) {
handler()
}
}, ms)
return request
}),
abort: vi.fn(() => request),
on: vi.fn((event: string, cb: (err: Error) => void) => {
if (event === 'error') {
errorHandlers.push(cb)
} else if (event === 'response') {
responseHandlers.push(cb as unknown as (response: unknown) => void)
} else if (event === 'redirect') {
redirectHandlers.push(
cb as unknown as (statusCode: number, method: string, redirectUrl: string) => void
)
}
return request
}),
@ -235,16 +319,25 @@ describe('ModelManager', () => {
errorHandlers.splice(index, 1)
}
}
if (event === 'timeout') {
const index = timeoutHandlers.indexOf(cb as () => void)
if (event === 'response') {
const index = responseHandlers.indexOf(cb as (response: unknown) => void)
if (index !== -1) {
timeoutHandlers.splice(index, 1)
responseHandlers.splice(index, 1)
}
}
if (event === 'redirect') {
const index = redirectHandlers.indexOf(
cb as (statusCode: number, method: string, redirectUrl: string) => void
)
if (index !== -1) {
redirectHandlers.splice(index, 1)
}
}
return request
})
}),
end: vi.fn(() => request)
}
httpsGetMock.mockReturnValue(request)
netRequestMock.mockReturnValue(request)
const manager = new ModelManager(dir) as unknown as ModelManagerInternals
const download = manager.downloadFile(
@ -263,11 +356,13 @@ describe('ModelManager', () => {
const outcome = await Promise.race([outcomePromise, Promise.resolve('pending')])
expect(outcome).toBe('Model download timed out after 120 seconds without network activity')
expect(request.destroy).toHaveBeenCalledWith()
expect(request.abort).toHaveBeenCalledWith()
expect(request.off).toHaveBeenCalledWith('error', expect.any(Function))
expect(request.off).toHaveBeenCalledWith('timeout', expect.any(Function))
expect(request.off).toHaveBeenCalledWith('response', expect.any(Function))
expect(request.off).toHaveBeenCalledWith('redirect', expect.any(Function))
expect(errorHandlers).toHaveLength(0)
expect(timeoutHandlers).toHaveLength(0)
expect(responseHandlers).toHaveLength(0)
expect(redirectHandlers).toHaveLength(0)
} finally {
vi.useRealTimers()
rmSync(dir, { recursive: true, force: true })

View File

@ -1,11 +1,9 @@
/* eslint-disable max-lines -- Why: model download, checksum, extraction, and cleanup share one state machine so progress/error transitions stay coupled. */
import { app } from 'electron'
import { app, net } from 'electron'
import { join, resolve, relative } from 'path'
import { existsSync, mkdirSync, createWriteStream, createReadStream, rmSync } from 'fs'
import { readdir, rm } from 'fs/promises'
import { createHash } from 'crypto'
import { get as httpsGet } from 'https'
import type { IncomingMessage } from 'http'
import { pipeline } from 'stream/promises'
import { spawn } from 'child_process'
import type {
@ -22,6 +20,12 @@ type DownloadHandle = {
}
type ProgressCallback = (modelId: string, progress: number) => void
type DownloadIncomingMessage = Electron.IncomingMessage &
NodeJS.ReadableStream & {
headers: Record<string, string | string[] | undefined>
resume: () => void
destroy?: () => void
}
const DOWNLOAD_IDLE_TIMEOUT_MS = 120_000
@ -287,16 +291,35 @@ export class ModelManager {
}
let settled = false
let request: ReturnType<typeof httpsGet> | null = null
let request: Electron.ClientRequest | null = null
let idleTimeout: ReturnType<typeof setTimeout> | null = null
const onSignalAbort = (): void => {
const activeRequest = request
rejectOnce(new Error('Aborted'))
activeRequest?.abort()
}
const clearIdleTimeout = (): void => {
if (idleTimeout) {
clearTimeout(idleTimeout)
idleTimeout = null
}
}
const cleanupRequestListeners = (): void => {
const activeRequest = request
clearIdleTimeout()
if (!activeRequest) {
return
}
activeRequest.off('error', onRequestError)
activeRequest.off('timeout', onRequestTimeout)
activeRequest.off('response', onResponse)
activeRequest.off('redirect', onRedirect)
signal?.removeEventListener('abort', onSignalAbort)
request = null
}
const resetIdleTimeout = (): void => {
clearIdleTimeout()
idleTimeout = setTimeout(onRequestTimeout, DOWNLOAD_IDLE_TIMEOUT_MS)
}
const resolveOnce = (): void => {
if (settled) {
return
@ -321,62 +344,57 @@ export class ModelManager {
`Model download timed out after ${DOWNLOAD_IDLE_TIMEOUT_MS / 1000} seconds without network activity`
)
)
activeRequest?.destroy()
activeRequest?.abort()
}
const onResponse = (response: IncomingMessage): void => {
if (
response.statusCode === 301 ||
response.statusCode === 302 ||
response.statusCode === 303 ||
response.statusCode === 307 ||
response.statusCode === 308
) {
const redirectUrl = response.headers.location
if (!redirectUrl) {
response.resume()
rejectOnce(new Error('Redirect without location'))
return
}
if (redirectCount >= 5) {
response.resume()
rejectOnce(new Error('Too many redirects'))
return
}
let resolvedRedirect: URL
try {
resolvedRedirect = new URL(redirectUrl, parsedUrl)
} catch {
response.resume()
rejectOnce(new Error('Invalid redirect URL'))
return
}
if (resolvedRedirect.protocol !== 'https:') {
response.resume()
rejectOnce(new Error('Model download redirect must use HTTPS'))
return
}
response.resume()
this.downloadFile(
resolvedRedirect.toString(),
dest,
expectedSize,
modelId,
isAborted,
signal,
redirectCount + 1
)
.then(resolveOnce)
.catch(rejectOnce)
const onRedirect = (_statusCode: number, _method: string, redirectUrl: string): void => {
if (redirectCount >= 5) {
const activeRequest = request
rejectOnce(new Error('Too many redirects'))
activeRequest?.abort()
return
}
let resolvedRedirect: URL
try {
resolvedRedirect = new URL(redirectUrl, parsedUrl)
} catch {
const activeRequest = request
rejectOnce(new Error('Invalid redirect URL'))
activeRequest?.abort()
return
}
if (resolvedRedirect.protocol !== 'https:') {
const activeRequest = request
rejectOnce(new Error('Model download redirect must use HTTPS'))
activeRequest?.abort()
return
}
const activeRequest = request
cleanupRequestListeners()
activeRequest?.abort()
this.downloadFile(
resolvedRedirect.toString(),
dest,
expectedSize,
modelId,
isAborted,
signal,
redirectCount + 1
)
.then(resolveOnce)
.catch(rejectOnce)
}
const onResponse = (incoming: Electron.IncomingMessage): void => {
const response = incoming as DownloadIncomingMessage
if (response.statusCode !== 200) {
response.resume()
rejectOnce(new Error(`HTTP ${response.statusCode}`))
return
}
const totalSize = parseInt(response.headers['content-length'] || '0', 10) || expectedSize
const contentLength = response.headers['content-length']
const totalSize =
parseInt(Array.isArray(contentLength) ? contentLength[0] : contentLength || '0', 10) ||
expectedSize
let downloaded = 0
const fileStream = createWriteStream(dest)
@ -385,9 +403,10 @@ export class ModelManager {
response.off('data', onResponseData)
}
const onResponseData = (chunk: Buffer): void => {
resetIdleTimeout()
if (isAborted()) {
request?.destroy(new Error('Aborted'))
response.destroy()
request?.abort()
response.destroy?.()
fileStream.destroy()
return
}
@ -412,15 +431,18 @@ export class ModelManager {
})
}
request = signal
? httpsGet(parsedUrl, { signal }, onResponse)
: httpsGet(parsedUrl, onResponse)
request = net.request({ method: 'GET', url: parsedUrl.toString() })
// Why: cancellation only helps after the user presses cancel; a peer
// that accepts the socket and goes silent must not leave the model stuck
// in "downloading" forever.
request.setTimeout(DOWNLOAD_IDLE_TIMEOUT_MS, onRequestTimeout)
// Why: Electron's net stack honors app proxy settings, unlike Node's
// https client, but it does not expose request.setTimeout().
resetIdleTimeout()
request.on('error', onRequestError)
request.on('response', onResponse)
request.on('redirect', onRedirect)
if (signal) {
signal.addEventListener('abort', onSignalAbort, { once: true })
}
request.end()
})
}