From e3beb6318207deea8e45baf6bbf7de6487acfdb8 Mon Sep 17 00:00:00 2001 From: ybai08 <271969568+ybai08@users.noreply.github.com> Date: Thu, 1 Oct 2026 01:16:07 -0500 Subject: [PATCH] fix(downloads): retry a failed curated ZIM with the current catalog entry A job's data is frozen when the job is created, so Retry re-sent the original URL and filepath. When a curated download failed because the upstream file was gone, a catalog fix corrected the manifest but Retry kept requesting the deleted file. On retry of a ZIM that came from a collection, look the resource up in the current catalog by resource_id and dispatch with its url, version and matching filename. A resource the catalog no longer lists, a download from no collection and a non-ZIM download keep their stored params. Fixes #1400 --- admin/app/services/download_service.ts | 22 ++++- admin/app/utils/download_retry.ts | 38 ++++++++ admin/tests/unit/download_retry.spec.ts | 115 ++++++++++++++++++++++++ 3 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 admin/app/utils/download_retry.ts create mode 100644 admin/tests/unit/download_retry.spec.ts diff --git a/admin/app/services/download_service.ts b/admin/app/services/download_service.ts index f0dd6d43..36c1343e 100644 --- a/admin/app/services/download_service.ts +++ b/admin/app/services/download_service.ts @@ -9,6 +9,9 @@ import { DownloadJobWithProgress, DownloadProgressData, RunDownloadJobParams } f import type { Job, Queue } from 'bullmq' import { normalize } from 'path' import { deleteFileIfExists } from '../utils/fs.js' +import { refreshRetryParams } from '../utils/download_retry.js' +import { CollectionManifestService } from './collection_manifest_service.js' +import type { ZimCategoriesSpec } from '../../types/collections.js' import transmit from '@adonisjs/transmit/services/main' import { BROADCAST_CHANNELS } from '../../constants/broadcast.js' @@ -200,11 +203,13 @@ export class DownloadService { return { success: true, message: `Retrying download for model ${modelName}` } } - // For file downloads (zim, map, etc.), re-dispatch with original params - const params = job.data as RunDownloadJobParams - if (!params.url || !params.filepath) { + // For file downloads (zim, map, etc.), re-dispatch with the stored params, + // refreshed from the current catalog for a curated ZIM + const stored = job.data as RunDownloadJobParams + if (!stored.url || !stored.filepath) { return { success: false, message: 'Cannot retry: missing URL or filepath in job data' } } + const params = refreshRetryParams(stored, await this.currentZimCatalog()) // Remove the old failed job, then dispatch a fresh one await job.remove().catch(() => {}) @@ -216,6 +221,17 @@ export class DownloadService { return { success: false, message: 'Failed job not found. It may have already been dismissed.' } } + /** Null when the catalog cannot be read, so a retry falls back to its stored params. */ + private async currentZimCatalog(): Promise { + try { + return await new CollectionManifestService().getSpecWithFallback( + 'zim_categories' + ) + } catch { + return null + } + } + async cancelJob(jobId: string): Promise<{ success: boolean; message: string }> { const queue = this.queueService.getQueue(RunDownloadJob.queue) const job = await queue.getJob(jobId) diff --git a/admin/app/utils/download_retry.ts b/admin/app/utils/download_retry.ts new file mode 100644 index 00000000..775db091 --- /dev/null +++ b/admin/app/utils/download_retry.ts @@ -0,0 +1,38 @@ +import { dirname, join } from 'node:path' + +import type { RunDownloadJobParams } from '../../types/downloads.js' +import type { ZimCategoriesSpec } from '../../types/collections.js' + +/** + * Params for retrying a failed download. A job's data is frozen when the job + * is created, so a curated ZIM whose catalog entry has since been corrected + * (a dead URL replaced, a new build date) is retried with what the catalog + * says now. The filename changes with the URL because ZIM filenames carry the + * build date. + * + * Anything else keeps its stored params: a download that came from no + * collection, a non-ZIM download, and a resource the catalog no longer lists + * (removed or renamed), so a retry is never worse than re-sending the original. + */ +export function refreshRetryParams( + params: RunDownloadJobParams, + spec: ZimCategoriesSpec | null +): RunDownloadJobParams { + const metadata = params.resourceMetadata + if (!spec || params.filetype !== 'zim' || !metadata?.collection_ref) return params + + const current = spec.categories + .flatMap((category) => category.tiers) + .flatMap((tier) => tier.resources) + .find((resource) => resource.id === metadata.resource_id && resource.type !== 'dataset') + const filename = current?.url.split('/').pop() + if (!current || !filename) return params + + return { + ...params, + url: current.url, + filepath: join(dirname(params.filepath), filename), + totalBytes: current.size_mb > 0 ? current.size_mb * 1024 * 1024 : params.totalBytes, + resourceMetadata: { ...metadata, version: current.version }, + } +} diff --git a/admin/tests/unit/download_retry.spec.ts b/admin/tests/unit/download_retry.spec.ts new file mode 100644 index 00000000..ce69b11a --- /dev/null +++ b/admin/tests/unit/download_retry.spec.ts @@ -0,0 +1,115 @@ +import * as assert from 'node:assert/strict' +import { test } from 'node:test' + +import { refreshRetryParams } from '../../app/utils/download_retry.js' +import type { RunDownloadJobParams } from '../../types/downloads.js' +import type { ZimCategoriesSpec } from '../../types/collections.js' + +const failed: RunDownloadJobParams = { + url: 'https://download.kiwix.org/zim/other/prepper_en_2025-01.zim', + filepath: '/app/storage/zim/prepper_en_2025-01.zim', + timeout: 30000, + allowedMimeTypes: ['application/x-zim'], + filetype: 'zim', + title: 'Prepper', + totalBytes: 100 * 1024 * 1024, + resourceMetadata: { + resource_id: 'prepper_en', + version: '2025-01', + collection_ref: 'survival', + }, +} + +function specWith(resource: { id: string; version: string; url: string; size_mb: number }) { + return { + spec_version: '1', + categories: [ + { + name: 'Survival', + slug: 'survival', + icon: 'tent', + description: '', + language: 'en', + tiers: [ + { + name: 'Essential', + slug: 'essential', + description: '', + resources: [{ title: 'Prepper', description: '', ...resource }], + }, + ], + }, + ], + } satisfies ZimCategoriesSpec +} + +test('retry of a curated download uses the url, version and filename the catalog has now', () => { + const retried = refreshRetryParams( + failed, + specWith({ + id: 'prepper_en', + version: '2026-09', + url: 'https://download.kiwix.org/zim/other/prepper_en_2026-09.zim', + size_mb: 120, + }) + ) + + assert.equal(retried.url, 'https://download.kiwix.org/zim/other/prepper_en_2026-09.zim') + assert.equal(retried.filepath, '/app/storage/zim/prepper_en_2026-09.zim') + assert.equal(retried.totalBytes, 120 * 1024 * 1024) + assert.deepEqual(retried.resourceMetadata, { + resource_id: 'prepper_en', + version: '2026-09', + collection_ref: 'survival', + }) + assert.equal(retried.title, 'Prepper') +}) + +test('retry keeps the stored params when the catalog no longer has the resource', () => { + const retried = refreshRetryParams( + failed, + specWith({ + id: 'prepper_en_renamed', + version: '2026-09', + url: 'https://download.kiwix.org/zim/other/prepper_en_renamed_2026-09.zim', + size_mb: 120, + }) + ) + + assert.deepEqual(retried, failed) +}) + +test('retry keeps the stored params when no catalog is available', () => { + assert.deepEqual(refreshRetryParams(failed, null), failed) +}) + +test('retry of a download that came from no collection keeps the stored params', () => { + const manual: RunDownloadJobParams = { + ...failed, + resourceMetadata: { resource_id: 'prepper_en', version: '2025-01', collection_ref: null }, + } + const spec = specWith({ + id: 'prepper_en', + version: '2026-09', + url: 'https://download.kiwix.org/zim/other/prepper_en_2026-09.zim', + size_mb: 120, + }) + + assert.deepEqual(refreshRetryParams(manual, spec), manual) + assert.deepEqual(refreshRetryParams({ ...failed, resourceMetadata: undefined }, spec), { + ...failed, + resourceMetadata: undefined, + }) +}) + +test('retry of a map download keeps the stored params', () => { + const map: RunDownloadJobParams = { ...failed, filetype: 'map' } + const spec = specWith({ + id: 'prepper_en', + version: '2026-09', + url: 'https://download.kiwix.org/zim/other/prepper_en_2026-09.zim', + size_mb: 120, + }) + + assert.deepEqual(refreshRetryParams(map, spec), map) +})