From 24348a993891c6e2de2523e029505f7137450e79 Mon Sep 17 00:00:00 2001 From: Matt Moyles Date: Thu, 24 Sep 2026 17:49:29 -0400 Subject: [PATCH] fix(downloads): report a retrying model download as queued, not failed Model downloads were the only type whose status ignored the BullMQ state fetchJobsWithStates() already tags each job with. Status was read off `failedReason` instead, and BullMQ keeps `failedReason` on a job it has moved to `delayed` for another attempt, so a download retrying while Ollama starts up (Easy Setup) showed as a permanent failure with a Retry button. It now reports the tagged state, which the downloads list already renders as "queued". DownloadModelJob's readiness guard also could never fire: getModels() throws before returning while Ollama isn't up, and never returns null. The throw is now caught, so the retry carries the intended "Ollama service not ready yet" instead of the raw init error. Closes #1311 --- admin/app/jobs/download_model_job.ts | 11 ++-- admin/app/services/download_service.ts | 7 ++- .../tests/unit/download_model_status.spec.ts | 61 +++++++++++++++++++ 3 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 admin/tests/unit/download_model_status.spec.ts diff --git a/admin/app/jobs/download_model_job.ts b/admin/app/jobs/download_model_job.ts index 0921950b..82336d52 100644 --- a/admin/app/jobs/download_model_job.ts +++ b/admin/app/jobs/download_model_job.ts @@ -47,11 +47,14 @@ export class DownloadModelJob { const ollamaService = new OllamaService() - // Even if no models are installed, this should return an empty array if ready - const existingModels = await ollamaService.getModels() - if (!existingModels) { + // getModels() throws while Ollama is still starting — Easy Setup dispatches model + // downloads without waiting for the install — so treat that as "not ready" and let + // the retry backoff wait it out, rather than surfacing the raw init error (#1311) + try { + await ollamaService.getModels() + } catch (error) { logger.warn( - `[DownloadModelJob] Ollama service not ready yet for model ${modelName}. Will retry...` + `[DownloadModelJob] Ollama service not ready yet for model ${modelName} (${error instanceof Error ? error.message : error}). Will retry...` ) throw new Error('Ollama service not ready yet') } diff --git a/admin/app/services/download_service.ts b/admin/app/services/download_service.ts index f0dd6d43..0d94a910 100644 --- a/admin/app/services/download_service.ts +++ b/admin/app/services/download_service.ts @@ -97,13 +97,16 @@ export class DownloadService { } }) - const modelDownloads = modelTagged.map(({ job }) => ({ + // Status comes from the tagged BullMQ state like every other type. BullMQ keeps + // `failedReason` on a job it has moved to `delayed` for another attempt, so + // reading status off `failedReason` showed a retrying download as failed (#1311). + const modelDownloads = modelTagged.map(({ job, state }) => ({ jobId: job.id!.toString(), url: job.data.modelName || 'Unknown Model', progress: parseInt(job.progress.toString(), 10), filepath: job.data.modelName || 'Unknown Model', filetype: 'model', - status: (job.failedReason ? 'failed' : 'active') as 'active' | 'failed', + status: state, failedReason: job.failedReason || undefined, })) diff --git a/admin/tests/unit/download_model_status.spec.ts b/admin/tests/unit/download_model_status.spec.ts new file mode 100644 index 00000000..8a46bd3b --- /dev/null +++ b/admin/tests/unit/download_model_status.spec.ts @@ -0,0 +1,61 @@ +/** + * Tests for how model downloads report their status in the downloads list. + * + * BullMQ keeps `failedReason` on a job it has moved to `delayed` for another + * attempt, so a model download retrying while Ollama starts (Easy Setup) used + * to render as a permanent failure (#1311). Status must come from the queue + * state the job was fetched from, as it already does for file downloads. + * + * A Japa spec: DownloadService imports transmit and the Adonis logger at module + * scope. Driven through a fake queue, so no Redis is needed. Run with + * `node ace test unit`. + */ +import assert from 'node:assert/strict' +import { test } from '@japa/runner' + +import { DownloadService } from '../../app/services/download_service.js' +import { DownloadModelJob } from '../../app/jobs/download_model_job.js' + +type FakeJob = { id: string; data: Record; progress: number; failedReason?: string } + +/** A QueueService whose model queue holds the given jobs by state; every other queue is empty. */ +function fakeQueueService(modelJobs: Partial>) { + return { + getQueue: (name: string) => ({ + getJobs: async ([state]: string[]) => + name === DownloadModelJob.queue ? (modelJobs as Record)[state] ?? [] : [], + }), + } as any +} + +const job = (id: string, failedReason?: string): FakeJob => ({ + id, + data: { modelName: `model-${id}` }, + progress: 0, + failedReason, +}) + +test('model download: a retrying job with a failedReason reports delayed, not failed', async () => { + const service = new DownloadService( + fakeQueueService({ delayed: [job('1', 'Ollama service not ready yet')] }) + ) + const [download] = await service.listDownloadJobs('model') + assert.equal(download.status, 'delayed') + // The reason is still passed through for display + assert.equal(download.failedReason, 'Ollama service not ready yet') +}) + +test('model download: each queue state is reported as itself', async () => { + const service = new DownloadService( + fakeQueueService({ + waiting: [job('w')], + active: [job('a')], + delayed: [job('d', 'retrying')], + failed: [job('f', 'gave up')], + }) + ) + const byId = Object.fromEntries( + (await service.listDownloadJobs('model')).map((d) => [d.jobId, d.status]) + ) + assert.deepEqual(byId, { w: 'waiting', a: 'active', d: 'delayed', f: 'failed' }) +})