From 9b4e3433ed764b289fd376ee95b77e2f84c093ad Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 2 Aug 2026 20:48:31 +0200 Subject: [PATCH] fix(downloads): bound restored file probes --- CLAUDE.md | 10 ++-- .../download-file-availability.spec.ts | 23 ++++++++ .../database/download-file-availability.ts | 49 +++++++++++++++-- .../events/database/download-requests.spec.ts | 52 ++++++++++++++++--- .../app/events/database/download-requests.ts | 31 ++++++----- docs/architecture/download-manager.md | 11 ++-- 6 files changed, 145 insertions(+), 31 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 74e9b6dd0..5a998c2d5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -946,10 +946,12 @@ engine` (restart required) or results are counted as skipped, and no batch IPC is introduced. The latter comes from an asynchronous main-process filesystem recheck before a completed-missing row can be reset, so a file restored after the renderer - snapshot is not orphaned or downloaded again. Episode and season download - actions require an authoritative global list. A successful snapshot remains - authoritative while a later background refresh is in flight; a latest - refresh failure leaves + snapshot is not orphaned or downloaded again. The recheck has a one-second + deadline; timeout or probe failure leaves the row untouched and reports a + failed submission so the season loop can continue. Episode and season + download actions require an authoritative global list. A successful snapshot + remains authoritative while a later background refresh is in flight; a + latest refresh failure leaves loading/empty-state resolution intact but disables starts until another snapshot succeeds. - Episode ownership uses normalized `episode.id` as the canonical `xtreamId` diff --git a/apps/electron-backend/src/app/events/database/download-file-availability.spec.ts b/apps/electron-backend/src/app/events/database/download-file-availability.spec.ts index d23b32f22..22d3eb503 100644 --- a/apps/electron-backend/src/app/events/database/download-file-availability.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-file-availability.spec.ts @@ -1,6 +1,7 @@ import { decorateDownloadItem, getDownloadFileAvailability, + getDownloadFileAvailabilityWithTimeoutAsync, isAvailableDownloadFile, type DownloadLstat, } from './download-file-availability'; @@ -16,6 +17,28 @@ function lstatResult(options: { } describe('download file availability', () => { + it('bounds a restored-file probe and returns unknown on timeout', async () => { + jest.useFakeTimers(); + try { + const probe = jest.fn(() => new Promise(() => undefined)); + const result = getDownloadFileAvailabilityWithTimeoutAsync( + { + filePath: '/downloads/unresponsive/episode.mp4', + status: 'completed', + }, + 25, + probe + ); + + await jest.advanceTimersByTimeAsync(25); + + await expect(result).resolves.toBe('unknown'); + expect(probe).toHaveBeenCalledTimes(1); + } finally { + jest.useRealTimers(); + } + }); + it('marks only a completed regular non-symbolic-link file available', () => { const lstat = lstatResult({ isFile: true }); diff --git a/apps/electron-backend/src/app/events/database/download-file-availability.ts b/apps/electron-backend/src/app/events/database/download-file-availability.ts index a269c11b0..f7eaae19b 100644 --- a/apps/electron-backend/src/app/events/database/download-file-availability.ts +++ b/apps/electron-backend/src/app/events/database/download-file-availability.ts @@ -21,11 +21,13 @@ type DownloadAsyncLstat = ( filePath: string ) => Promise>; -type DownloadFileAvailabilityProbe = ( - filePath: string -) => Promise; +type DownloadFileAvailabilityProbe = (filePath: string) => Promise; const DEFAULT_MAX_CONCURRENT_FILE_PROBES = 4; +const DEFAULT_RESTORED_FILE_PROBE_TIMEOUT_MS = 1_000; + +export type BoundedDownloadFileAvailability = + ElectronDownloadFileAvailability | 'unknown'; function createDownloadFileAvailabilityProbe( asyncLstat: DownloadAsyncLstat = lstat, @@ -131,6 +133,47 @@ export async function getDownloadFileAvailabilityAsync( return (await probe(download.filePath)) ? 'available' : 'missing'; } +export async function getDownloadFileAvailabilityWithTimeoutAsync( + download: DownloadFileRow, + timeoutMs = DEFAULT_RESTORED_FILE_PROBE_TIMEOUT_MS, + probe: DownloadFileAvailabilityProbe = probeDownloadFileAvailability +): Promise { + if (download.status !== 'completed') { + return 'not-applicable'; + } + + if (!download.filePath) { + return 'missing'; + } + + const boundedTimeoutMs = + Number.isFinite(timeoutMs) && timeoutMs >= 0 + ? timeoutMs + : DEFAULT_RESTORED_FILE_PROBE_TIMEOUT_MS; + let timeout: ReturnType | undefined; + const timedOut = new Promise<'unknown'>((resolve) => { + timeout = setTimeout(() => resolve('unknown'), boundedTimeoutMs); + }); + + try { + const available = await Promise.race([ + probe(download.filePath), + timedOut, + ]); + return available === 'unknown' + ? available + : available + ? 'available' + : 'missing'; + } catch { + return 'unknown'; + } finally { + if (timeout !== undefined) { + clearTimeout(timeout); + } + } +} + export function decorateDownloadItem( download: T, lstat: DownloadLstat = lstatSync diff --git a/apps/electron-backend/src/app/events/database/download-requests.spec.ts b/apps/electron-backend/src/app/events/database/download-requests.spec.ts index b091568fd..65c8c6e71 100644 --- a/apps/electron-backend/src/app/events/database/download-requests.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-requests.spec.ts @@ -12,7 +12,7 @@ const metadataSnapshot: DownloadMetadataSnapshot = { async function setupStartMetadataRequest( existing: Record | undefined, coordinateRows: Record[] = [], - completedFileAvailable = false + completedFileAvailability: 'available' | 'missing' | 'unknown' = 'missing' ) { jest.resetModules(); const schema = await import('../../database/schema'); @@ -41,10 +41,15 @@ async function setupStartMetadataRequest( update: jest.fn(() => ({ set })), }; const enqueueDownload = jest.fn(); - const getDownloadFileAvailabilityAsync = jest.fn(async () => - completedFileAvailable ? 'available' : 'missing' + const getDownloadFileAvailabilityAsync = jest.fn( + async () => completedFileAvailability + ); + const getDownloadFileAvailabilityWithTimeoutAsync = jest.fn( + async () => completedFileAvailability + ); + const isAvailableDownloadFile = jest.fn( + () => completedFileAvailability === 'available' ); - const isAvailableDownloadFile = jest.fn(() => completedFileAvailable); const authorizer = { requireAuthorized: jest.fn(async (directory: string) => directory), } as unknown as DownloadDirectoryAuthorizer; @@ -60,6 +65,7 @@ async function setupStartMetadataRequest( })); jest.doMock('./download-file-availability', () => ({ getDownloadFileAvailabilityAsync, + getDownloadFileAvailabilityWithTimeoutAsync, isAvailableDownloadFile, })); @@ -70,6 +76,7 @@ async function setupStartMetadataRequest( downloadLimit, enqueueDownload, getDownloadFileAvailabilityAsync, + getDownloadFileAvailabilityWithTimeoutAsync, insertValues, isAvailableDownloadFile, set, @@ -450,7 +457,7 @@ describe('download request identity resolution', () => { const request = await setupStartMetadataRequest( completedRow, [completedRow], - true + 'available' ); await expect( @@ -468,12 +475,41 @@ describe('download request identity resolution', () => { expect(request.db.insert).not.toHaveBeenCalled(); expect(request.db.update).not.toHaveBeenCalled(); expect(request.enqueueDownload).not.toHaveBeenCalled(); - expect(request.getDownloadFileAvailabilityAsync).toHaveBeenCalledWith( - completedRow - ); + expect( + request.getDownloadFileAvailabilityWithTimeoutAsync + ).toHaveBeenCalledWith(completedRow); + expect(request.getDownloadFileAvailabilityAsync).not.toHaveBeenCalled(); expect(request.isAvailableDownloadFile).not.toHaveBeenCalled(); }); + it('fails closed when a completed-file recheck times out', async () => { + const completedRow = createStartDownloadRow({ + filePath: '/downloads/unresponsive/episode.mp4', + status: 'completed', + xtreamId: 700, + }); + const request = await setupStartMetadataRequest( + completedRow, + [completedRow], + 'unknown' + ); + + await expect( + request.startDownloadRequest( + episodeStartPayload(), + request.authorizer + ) + ).resolves.toEqual({ + error: 'Could not verify the completed download file', + id: completedRow.id, + success: false, + }); + + expect(request.db.insert).not.toHaveBeenCalled(); + expect(request.db.update).not.toHaveBeenCalled(); + expect(request.enqueueDownload).not.toHaveBeenCalled(); + }); + it.each(['queued', 'downloading', 'paused'] as const)( 'returns the stable duplicate result for an active legacy-coordinate %s row', async (status) => { diff --git a/apps/electron-backend/src/app/events/database/download-requests.ts b/apps/electron-backend/src/app/events/database/download-requests.ts index 45cb66faf..908c6548f 100644 --- a/apps/electron-backend/src/app/events/database/download-requests.ts +++ b/apps/electron-backend/src/app/events/database/download-requests.ts @@ -11,7 +11,7 @@ import * as schema from '../../database/schema'; import { assertRemoteUrlAllowed } from '../url-safety'; import { DownloadDirectoryAuthorizer } from './download-directory-authorization'; import { removePartialDownloadFile } from './download-file-path'; -import { getDownloadFileAvailabilityAsync } from './download-file-availability'; +import { getDownloadFileAvailabilityWithTimeoutAsync } from './download-file-availability'; import { resolveExistingDownloadIdentity } from './download-request-identity'; import { resolveStoredDownloadHeaders } from './download-request-headers'; import { @@ -147,17 +147,24 @@ export async function startDownloadRequest( data.url ); } - if ( - item.contentType === 'episode' && - item.status === 'completed' && - (await getDownloadFileAvailabilityAsync(item)) === 'available' - ) { - return { - error: 'Download already completed', - id: item.id, - reason: ELECTRON_BRIDGE_DOWNLOAD_START_REASONS.AlreadyDownloaded, - success: false, - }; + if (item.contentType === 'episode' && item.status === 'completed') { + const completedFileAvailability = + await getDownloadFileAvailabilityWithTimeoutAsync(item); + if (completedFileAvailability === 'unknown') { + return { + error: 'Could not verify the completed download file', + id: item.id, + success: false, + }; + } + if (completedFileAvailability === 'available') { + return { + error: 'Download already completed', + id: item.id, + reason: ELECTRON_BRIDGE_DOWNLOAD_START_REASONS.AlreadyDownloaded, + success: false, + }; + } } if (!['completed', 'failed', 'canceled'].includes(item.status)) { return { diff --git a/docs/architecture/download-manager.md b/docs/architecture/download-manager.md index a4624e88f..bca160823 100644 --- a/docs/architecture/download-manager.md +++ b/docs/architecture/download-manager.md @@ -96,10 +96,13 @@ variants, contextual buttons, and theme-aware styling. such a completed row, `DOWNLOADS_START` asynchronously rechecks its retained path in the main process. A restored file returns stable `reason: 'already-downloaded'` without mutation; active matches return - `reason: 'already-in-progress'`. The - coordinator counts both as skipped. There is no batch IPC, parallel transfer, - or queue reordering: destination authorization, persisted header handling, - and the backend's one-active-transfer FIFO semantics remain unchanged. + `reason: 'already-in-progress'`. The recheck has a one-second deadline; + timeout or probe failure leaves the row untouched and returns a failed + submission, allowing the sequential season loop to continue. The coordinator + counts both stable duplicate reasons as skipped. There is no batch IPC, + parallel transfer, or queue reordering: destination authorization, persisted + header handling, and the backend's one-active-transfer FIFO semantics remain + unchanged. - **Pure manager model** (`download-manager.viewmodel.ts` and `download-library.viewmodel.ts`) derives the current route scope, search/category filtering, queue partitions,