From 8b7ecb02e377b407592d6ef20a624a50facdfde2 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sat, 1 Aug 2026 20:56:19 +0200 Subject: [PATCH] fix(downloads): validate partials before resuming --- .changes/downloads-vod-reliability.md | 2 +- CLAUDE.md | 11 ++++-- .../database/download-request-headers.spec.ts | 21 +++++++++-- .../database/download-request-headers.ts | 6 +++- .../events/database/download-reserve.spec.ts | 1 + .../events/database/download-resume.spec.ts | 36 +++++++++++++++++++ .../events/database/download-runtime.spec.ts | 33 +++++++++++------ .../app/events/database/download-transfer.ts | 17 ++++++--- docs/architecture/download-manager.md | 10 +++--- .../xtream-portal-compatibility.md | 6 +++- 10 files changed, 116 insertions(+), 27 deletions(-) diff --git a/.changes/downloads-vod-reliability.md b/.changes/downloads-vod-reliability.md index dcb1fb7ea..a69b636e5 100644 --- a/.changes/downloads-vod-reliability.md +++ b/.changes/downloads-vod-reliability.md @@ -4,4 +4,4 @@ area: downloads issues: [897, 1289] --- -Xtream movie downloads now use the same provider-compatible client identity as portal requests. If a connection drops after data has arrived, IPTVnator keeps the partial file, shows a credential-safe interruption code, and Retry resumes it with Range validation instead of starting over. +Xtream movie downloads now keep their provider-compatible identity for legacy retries after source removal. Recoverable connection drops retain validated partials and show a credential-safe code; Retry resumes only with ETag or Last-Modified, otherwise it safely restarts. diff --git a/CLAUDE.md b/CLAUDE.md index 5eecde25c..8db46ba12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -915,9 +915,14 @@ engine` (restart required) or `XTREAM_CLIENT_USER_AGENT` used by API requests and stream probes, while an explicit playlist User-Agent still wins. Retry, resume, and missing-file recovery also add the fallback to legacy Xtream rows that have no stored - User-Agent. Allowlisted connection resets after bytes reach disk retain the partial and show a credential-safe - `DOWNLOAD_NETWORK_INTERRUPTED` code; Retry continues with Range/If-Range - instead of starting from zero. + User-Agent. Because download rows survive source deletion, a headerless + legacy row whose playlist is already absent receives the same IPTV-player + fallback; a known Stalker row remains unchanged. Allowlisted connection + resets after bytes reach disk retain the partial and show a credential-safe + `DOWNLOAD_NETWORK_INTERRUPTED` code only when the response supplied a strong + ETag or Last-Modified validator. Retry then continues with Range/If-Range; + without a validator it starts from byte zero and overwrites the unverified + partial instead of risking mixed-representation corruption. - The desktop-only manager shares one global download store across the global, Xtream-scoped, and Stalker-scoped routes. Completed movie and grouped-series cards use the global Small/Medium/Large cover-grid tokens; missing completed diff --git a/apps/electron-backend/src/app/events/database/download-request-headers.spec.ts b/apps/electron-backend/src/app/events/database/download-request-headers.spec.ts index 9ef125d90..050c6df61 100644 --- a/apps/electron-backend/src/app/events/database/download-request-headers.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-request-headers.spec.ts @@ -1,7 +1,9 @@ import type { DownloadsDatabase } from './download-task'; -function createDatabase(playlistType: 'xtream' | 'stalker') { - const limit = jest.fn().mockResolvedValue([{ type: playlistType }]); +function createDatabase(playlistType?: 'xtream' | 'stalker') { + const limit = jest + .fn() + .mockResolvedValue(playlistType ? [{ type: playlistType }] : []); const db = { select: jest.fn(() => ({ from: jest.fn(() => ({ @@ -45,6 +47,21 @@ describe('stored download request headers', () => { ).resolves.toBeUndefined(); }); + it('adds the player fallback when a legacy row outlives its deleted source', async () => { + const { db } = createDatabase(); + const { resolveStoredDownloadHeaders } = + await import('./download-request-headers'); + + await expect( + resolveStoredDownloadHeaders(db, { + playlistId: 'deleted-playlist', + requestHeaders: null, + }) + ).resolves.toEqual({ + 'User-Agent': 'VLC/3.0.18 LibVLC/3.0.18', + }); + }); + it('preserves an explicit stored User-Agent without querying the playlist', async () => { const { db, select } = createDatabase('xtream'); const { resolveStoredDownloadHeaders } = diff --git a/apps/electron-backend/src/app/events/database/download-request-headers.ts b/apps/electron-backend/src/app/events/database/download-request-headers.ts index 709b9b4d8..0f19754f6 100644 --- a/apps/electron-backend/src/app/events/database/download-request-headers.ts +++ b/apps/electron-backend/src/app/events/database/download-request-headers.ts @@ -51,10 +51,14 @@ export async function resolveStoredDownloadHeaders( .from(schema.playlists) .where(eq(schema.playlists.id, item.playlistId)) .limit(1); - if (playlists[0]?.type !== 'xtream') { + const playlistType = playlists[0]?.type; + if (playlistType !== undefined && playlistType !== 'xtream') { return headers; } + // Download rows intentionally survive individual playlist deletion. Older + // rows have no stored provider marker, so a missing source must use the + // IPTV-player fallback as the only recoverable identity-compatible default. return { ...headers, 'User-Agent': XTREAM_CLIENT_USER_AGENT, diff --git a/apps/electron-backend/src/app/events/database/download-reserve.spec.ts b/apps/electron-backend/src/app/events/database/download-reserve.spec.ts index 46941058f..bf8cea9f5 100644 --- a/apps/electron-backend/src/app/events/database/download-reserve.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-reserve.spec.ts @@ -62,6 +62,7 @@ describe('destination collision handling', () => { runtime.enqueueDownload({ ...createTask(), filePath: '/downloads/movie.mp4', + resumeValidator: '"etag-1"', totalBytes: 100, }); await waitForStatus(set, 'completed'); diff --git a/apps/electron-backend/src/app/events/database/download-resume.spec.ts b/apps/electron-backend/src/app/events/database/download-resume.spec.ts index 9a3699550..1e68b016f 100644 --- a/apps/electron-backend/src/app/events/database/download-resume.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-resume.spec.ts @@ -151,6 +151,34 @@ describe('download resume validation', () => { ); }); + it('restarts without Range when a retained partial has no validator', async () => { + const harness = await setupResumeHarness({ + finalSize: 4, + partialSize: 50, + response: { + data: Readable.from([Buffer.from('full')]), + headers: { 'content-length': '4', etag: '"etag-new"' }, + status: 200, + }, + }); + + harness.runtime.enqueueDownload( + createTask({ + filePath: '/downloads/movie.mp4', + totalBytes: 54, + }) + ); + await waitForStatus(harness.set, 'completed'); + + const requestOptions = + harness.requestWithValidatedRedirects.mock.calls[0][1]; + expect(requestOptions.headers).toEqual({}); + expect(harness.createWriteStream).toHaveBeenCalledWith( + '/downloads/movie.mp4.part', + { flags: 'w' } + ); + }); + it('restarts from byte zero when a resume request is answered with 200', async () => { const harness = await setupResumeHarness({ finalSize: 4, @@ -215,6 +243,7 @@ describe('download resume validation', () => { harness.runtime.enqueueDownload( createTask({ filePath: '/downloads/movie.mp4', + resumeValidator: '"etag-1"', totalBytes: 54, }) ); @@ -251,6 +280,7 @@ describe('download resume validation', () => { harness.runtime.enqueueDownload( createTask({ filePath: '/downloads/movie.mp4', + resumeValidator: '"etag-1"', totalBytes: 100, }) ); @@ -392,6 +422,12 @@ describe('download resume validation', () => { label: 'response without an advertised total', partialSizeAfterTransferError: 20, }, + { + code: 'ECONNRESET', + headers: { 'content-length': '100' }, + label: 'response without a representation validator', + partialSizeAfterTransferError: 20, + }, { code: 'ECONNRESET', headers: { 'content-length': '100' }, diff --git a/apps/electron-backend/src/app/events/database/download-runtime.spec.ts b/apps/electron-backend/src/app/events/database/download-runtime.spec.ts index 9856b8de9..ce818eb86 100644 --- a/apps/electron-backend/src/app/events/database/download-runtime.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-runtime.spec.ts @@ -42,17 +42,16 @@ describe('download runtime pause and resume', () => { it('persists active pause without deleting the partial file', async () => { jest.resetModules(); - const set = jest.fn(() => ({ where: jest.fn().mockResolvedValue(undefined) })); + const set = jest.fn(() => ({ + where: jest.fn().mockResolvedValue(undefined), + })); const db = { update: jest.fn(() => ({ set })), }; const removePartialDownloadFile = jest.fn(); const stream = new PassThrough(); const requestWithValidatedRedirects = jest.fn( - async ( - _url: string, - options: { signal?: AbortSignal } - ) => { + async (_url: string, options: { signal?: AbortSignal }) => { options.signal?.addEventListener('abort', () => { stream.destroy(new Error('aborted')); }); @@ -119,10 +118,12 @@ describe('download runtime pause and resume', () => { } }); - it('uses an HTTP Range header when a partial file already exists', async () => { + it('uses an HTTP Range header when a validated partial file already exists', async () => { jest.resetModules(); - const set = jest.fn(() => ({ where: jest.fn().mockResolvedValue(undefined) })); + const set = jest.fn(() => ({ + where: jest.fn().mockResolvedValue(undefined), + })); const db = { update: jest.fn(() => ({ set })), }; @@ -168,13 +169,17 @@ describe('download runtime pause and resume', () => { runtime.enqueueDownload({ ...createTask(), filePath: '/downloads/movie.mp4', + resumeValidator: '"etag-1"', }); await waitForCallCount(requestWithValidatedRedirects, 1); expect(requestWithValidatedRedirects).toHaveBeenCalledWith( 'https://example.test/movie.mp4', expect.objectContaining({ - headers: expect.objectContaining({ Range: 'bytes=50-' }), + headers: expect.objectContaining({ + 'If-Range': '"etag-1"', + Range: 'bytes=50-', + }), }), { allowPrivateNetworks: true } ); @@ -184,7 +189,9 @@ describe('download runtime pause and resume', () => { it('deletes a queued resumed partial file when the queued task is canceled', async () => { jest.resetModules(); - const set = jest.fn(() => ({ where: jest.fn().mockResolvedValue(undefined) })); + const set = jest.fn(() => ({ + where: jest.fn().mockResolvedValue(undefined), + })); const db = { update: jest.fn(() => ({ set })), }; @@ -271,7 +278,9 @@ describe('download runtime pause and resume', () => { it('retains the partial path when canceling a queued task whose partial cannot be deleted', async () => { jest.resetModules(); - const set = jest.fn(() => ({ where: jest.fn().mockResolvedValue(undefined) })); + const set = jest.fn(() => ({ + where: jest.fn().mockResolvedValue(undefined), + })); const db = { update: jest.fn(() => ({ set })) }; const removePartialDownloadFile = jest.fn(() => { throw new Error('EPERM: locked'); @@ -338,7 +347,9 @@ describe('download runtime pause and resume', () => { it('retains the partial path when canceling a paused row whose partial cannot be deleted', async () => { jest.resetModules(); - const set = jest.fn(() => ({ where: jest.fn().mockResolvedValue(undefined) })); + const set = jest.fn(() => ({ + where: jest.fn().mockResolvedValue(undefined), + })); const db = { select: jest.fn(() => ({ from: jest.fn(() => ({ diff --git a/apps/electron-backend/src/app/events/database/download-transfer.ts b/apps/electron-backend/src/app/events/database/download-transfer.ts index abb19f7b2..638d89a53 100644 --- a/apps/electron-backend/src/app/events/database/download-transfer.ts +++ b/apps/electron-backend/src/app/events/database/download-transfer.ts @@ -60,7 +60,13 @@ export async function transferToPartialFile( task: DownloadTask, reservation: ReservedPartialDownloadFile ): Promise { - const resumeOffset = getResumeOffset(task, reservation); + const retainedOffset = getResumeOffset(task, reservation); + const resumeOffset = task.resumeValidator ? retainedOffset : 0; + if (retainedOffset > 0 && resumeOffset === 0) { + console.warn( + `[Downloads] Restarting ${reservation.filename} from the beginning (saved partial has no ETag or Last-Modified validator)` + ); + } const headers = { ...(task.headers ?? {}), @@ -156,7 +162,8 @@ export async function transferToPartialFile( error, reservation, effectiveOffset, - totalBytes + totalBytes, + task.resumeValidator ); if (interruptedProgress) { await persistProgress(db, task, interruptedProgress.progress); @@ -181,7 +188,8 @@ function getInterruptedTransferProgress( error: unknown, reservation: ReservedPartialDownloadFile, initialBytes: number, - totalBytes: number | null + totalBytes: number | null, + resumeValidator: string | null | undefined ): { networkCode: string; progress: TransferProgress } | null { const networkCode = error && typeof error === 'object' && 'code' in error @@ -189,7 +197,8 @@ function getInterruptedTransferProgress( : ''; if ( !RETAINABLE_NETWORK_ERROR_CODES.has(networkCode) || - totalBytes === null + totalBytes === null || + !resumeValidator ) { return null; } diff --git a/docs/architecture/download-manager.md b/docs/architecture/download-manager.md index b7e0527ff..c2abee799 100644 --- a/docs/architecture/download-manager.md +++ b/docs/architecture/download-manager.md @@ -12,7 +12,7 @@ variants, contextual buttons, and theme-aware styling. - **Queue control (`apps/electron-backend/src/app/events/database/download-runtime.ts`)** `DownloadTask` mirrors a row of the shared `downloads` table (type `Download` in `libs/shared/database/src/lib/schema.ts`) plus transient cancel/pause/progress helpers (shared task types live in `download-task.ts`). Request validation and row creation live in `download-requests.ts`, while `downloads.events.ts` stays focused on IPC registration. `enqueueDownload()` pushes the task onto `downloadQueue` and triggers `processQueue()`. `processQueue()` keeps one active download, updates the row to `downloading`, and calls `startDownload()`. The byte transfer itself lives in `download-transfer.ts`, finalization and retained-partial persistence in `download-finalize.ts`, and the renderer update broadcast in `download-broadcast.ts`. - **Range-aware transfer (`download-transfer.ts`)** - The transfer streams the response through the backend's validated Axios redirect helper instead of `electron-dl`. Headers (user agent, referer, origin) are persisted in `request_headers` and re-applied through the same allowlist when read back on retry/resume. Xtream VOD downloads use the playlist's configured User-Agent when present and otherwise share the provider-compatible `XTREAM_CLIENT_USER_AGENT` used by Xtream API requests and stream probes. Retry, resume, and missing-file recovery resolve the owning playlist type and add that fallback to legacy Xtream rows without a stored User-Agent; Stalker rows are left unchanged. Active pause/cancel operations abort the current request with `AbortController`; pause keeps the partial file and cancel removes it. Resume checks the existing `.part` size (rejecting anything that is not a regular file, so a symlink planted while paused is never followed) and sends `Range: bytes=-` plus `If-Range` with the stored entity validator. The first response's strong `ETag` (or `Last-Modified`) is persisted in `resume_validator` for exactly this purpose. A `206 Partial Content` answer must start at the requested offset (`Content-Range` is verified) before bytes are appended; any other 2xx answer — the server ignoring `Range`, or `If-Range` detecting that the remote file changed — restarts the transfer from byte zero over the same `.part` instead of failing the download. + The transfer streams the response through the backend's validated Axios redirect helper instead of `electron-dl`. Headers (user agent, referer, origin) are persisted in `request_headers` and re-applied through the same allowlist when read back on retry/resume. Xtream VOD downloads use the playlist's configured User-Agent when present and otherwise share the provider-compatible `XTREAM_CLIENT_USER_AGENT` used by Xtream API requests and stream probes. Retry, resume, and missing-file recovery resolve the owning playlist type and add that fallback to legacy Xtream rows without a stored User-Agent; known Stalker rows are left unchanged. Download rows deliberately outlive individually deleted playlists, so a headerless legacy row whose source no longer exists receives the same IPTV-player fallback because its original provider type cannot be recovered. Active pause/cancel operations abort the current request with `AbortController`; pause keeps the partial file and cancel removes it. Resume checks the existing `.part` size (rejecting anything that is not a regular file, so a symlink planted while paused is never followed). The first response's strong `ETag` (or `Last-Modified`) is persisted in `resume_validator`; only a partial carrying that validator may send `Range: bytes=-` plus `If-Range` and append bytes. A retained partial without a validator restarts from byte zero and overwrites its `.part`, so a changed remote representation can never be joined to an unverified prefix. A `206 Partial Content` answer must start at the requested offset (`Content-Range` is verified) before bytes are appended; any other 2xx answer — the server ignoring `Range`, or `If-Range` detecting that the remote file changed — restarts the transfer from byte zero over the same `.part` instead of failing the download. - **Destination collision policy** Existing destination files are never overwritten, inspected, or deleted. Before starting a new transfer, the backend atomically reserves a free @@ -25,9 +25,11 @@ variants, contextual buttons, and theme-aware styling. `unlink()`. Completion creates the final `filePath` from the `.part` without overwriting an existing file; cancel and non-recoverable transfer failures remove the `.part`, while finalization failures, completed-partial failures, - and allowlisted network interruptions after bytes reached disk deliberately - retain it (the row keeps `filePath` so a later retry can finish without - re-downloading); pause and restart recovery keep it for a later resume. + and allowlisted network interruptions after bytes reached disk with a stored + representation validator deliberately retain it (the row keeps `filePath` + so a later retry can finish without re-downloading); pause and restart + recovery keep partials, but a later retry starts over when no validator was + available. Re-downloading such a failed row from a detail page (`DOWNLOADS_START`) deletes the retained `.part` before the row is reset. - **Derived file readiness and recovery** diff --git a/docs/architecture/xtream-portal-compatibility.md b/docs/architecture/xtream-portal-compatibility.md index a983b9921..268e662b6 100644 --- a/docs/architecture/xtream-portal-compatibility.md +++ b/docs/architecture/xtream-portal-compatibility.md @@ -102,7 +102,11 @@ Electron's `XTREAM_REQUEST` and stream-probe handlers plus Xtream VOD download requests share the exported `XTREAM_CLIENT_USER_AGENT` fallback. A playlist's explicit User-Agent still wins for its stream probe and download. Legacy download rows without a stored User-Agent receive the fallback when retrying, -resuming, or recovering a missing completed file. Some Xtream +resuming, or recovering a missing completed file. Download rows intentionally +survive individual source deletion; when the playlist row is already gone and +its type can no longer be recovered, a headerless legacy download receives the +same IPTV-player fallback, while a still-identifiable Stalker row remains +unchanged. Some Xtream panels sit behind a WAF (e.g. Cloudflare) configured to challenge generic/incomplete browser-looking User-Agents while allowlisting known IPTV player clients; a player-style User-Agent (currently a VLC signature) avoids