fix(downloads): preserve retained partial ownership

This commit is contained in:
4gray committed 2026-08-02 22:25:32 +02:00
1 parent 645a37afa1
commit 9f2e827e55
6 files changed
+298 -90

No files matched your search

+7 -3
View File
@@ -958,9 +958,13 @@ engine` (restart required) or
settles, so later callers have independent bounded waits without duplicating
stalled native work. Only `ENOENT` and `ENOTDIR` prove absence; permission,
I/O, and other filesystem errors remain unknown and cannot clear a completed
row. 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
row. Before a completed-missing, failed, or canceled row clears its retained
path, the start IPC asynchronously removes any `.part` through a separate
one-second, same-path-coalesced, four-operation cap. Timeout or non-absence
errors keep the row's ownership intact; `ENOENT` and `ENOTDIR` safely proceed.
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`
@@ -0,0 +1,79 @@
import {
createPartialDownloadCleanup,
removePartialDownloadFileWithTimeoutAsync,
} from './download-partial-cleanup';
describe('bounded partial download cleanup', () => {
it.each([
['EACCES', 'unknown'],
['EIO', 'unknown'],
['ENOENT', 'missing'],
['ENOTDIR', 'missing'],
] as const)(
'classifies an %s unlink result as %s',
async (code, expected) => {
const error = Object.assign(new Error(code), { code });
const unlink = jest.fn(async () => {
throw error;
});
const cleanup = createPartialDownloadCleanup(unlink);
await expect(
removePartialDownloadFileWithTimeoutAsync(
'/downloads/episode.mp4',
25,
cleanup
)
).resolves.toBe(expected);
expect(unlink).toHaveBeenCalledWith('/downloads/episode.mp4.part');
}
);
it('removes the partial asynchronously', async () => {
const unlink = jest.fn(async () => undefined);
const cleanup = createPartialDownloadCleanup(unlink);
await expect(
removePartialDownloadFileWithTimeoutAsync(
'/downloads/episode.mp4',
25,
cleanup
)
).resolves.toBe('removed');
expect(unlink).toHaveBeenCalledWith('/downloads/episode.mp4.part');
});
it('bounds each caller while retaining and coalescing the raw cleanup', async () => {
jest.useFakeTimers();
try {
let finish: (() => void) | undefined;
const unlink = jest.fn(
() =>
new Promise<void>((resolve) => {
finish = resolve;
})
);
const cleanup = createPartialDownloadCleanup(unlink, 1);
const first = removePartialDownloadFileWithTimeoutAsync(
'/downloads/episode.mp4',
25,
cleanup
);
const second = removePartialDownloadFileWithTimeoutAsync(
'/downloads/episode.mp4',
50,
cleanup
);
await jest.advanceTimersByTimeAsync(25);
await expect(first).resolves.toBe('unknown');
expect(unlink).toHaveBeenCalledTimes(1);
finish?.();
await expect(second).resolves.toBe('removed');
expect(unlink).toHaveBeenCalledTimes(1);
} finally {
jest.useRealTimers();
}
});
});
@@ -0,0 +1,110 @@
import { unlink } from 'node:fs/promises';
import { getPartialDownloadPath } from './download-file-path';
export type DownloadPartialCleanupResult = 'removed' | 'missing' | 'unknown';
export type DownloadAsyncUnlink = (filePath: string) => Promise<void>;
export type DownloadPartialCleanup = (
filePath: string
) => Promise<DownloadPartialCleanupResult>;
const DEFAULT_MAX_CONCURRENT_PARTIAL_CLEANUPS = 4;
const DEFAULT_PARTIAL_CLEANUP_TIMEOUT_MS = 1_000;
function isMissingFileSystemError(error: unknown): boolean {
if (!error || typeof error !== 'object' || !('code' in error)) {
return false;
}
const code = (error as { code?: unknown }).code;
return code === 'ENOENT' || code === 'ENOTDIR';
}
export function createPartialDownloadCleanup(
asyncUnlink: DownloadAsyncUnlink = unlink,
maxConcurrent = DEFAULT_MAX_CONCURRENT_PARTIAL_CLEANUPS
): DownloadPartialCleanup {
const concurrency = Math.max(1, Math.floor(maxConcurrent));
const pending: Array<() => void> = [];
const inFlight = new Map<string, Promise<DownloadPartialCleanupResult>>();
let active = 0;
const acquire = (): Promise<void> => {
if (active < concurrency) {
active += 1;
return Promise.resolve();
}
return new Promise((resolve) => pending.push(resolve));
};
const release = (): void => {
const next = pending.shift();
if (next) {
next();
} else {
active -= 1;
}
};
const remove = async (
filePath: string
): Promise<DownloadPartialCleanupResult> => {
await acquire();
try {
await asyncUnlink(getPartialDownloadPath(filePath));
return 'removed';
} catch (error) {
return isMissingFileSystemError(error) ? 'missing' : 'unknown';
} finally {
release();
}
};
return (filePath: string) => {
const existing = inFlight.get(filePath);
if (existing) {
return existing;
}
const cleanup = remove(filePath);
inFlight.set(filePath, cleanup);
const clear = () => {
if (inFlight.get(filePath) === cleanup) {
inFlight.delete(filePath);
}
};
void cleanup.then(clear, clear);
return cleanup;
};
}
const cleanupPartialDownloadFile = createPartialDownloadCleanup();
export async function removePartialDownloadFileWithTimeoutAsync(
filePath: string | null | undefined,
timeoutMs = DEFAULT_PARTIAL_CLEANUP_TIMEOUT_MS,
cleanup: DownloadPartialCleanup = cleanupPartialDownloadFile
): Promise<DownloadPartialCleanupResult> {
if (!filePath) {
return 'missing';
}
const boundedTimeoutMs =
Number.isFinite(timeoutMs) && timeoutMs >= 0
? timeoutMs
: DEFAULT_PARTIAL_CLEANUP_TIMEOUT_MS;
let timeout: ReturnType<typeof setTimeout> | undefined;
const timedOut = new Promise<'unknown'>((resolve) => {
timeout = setTimeout(() => resolve('unknown'), boundedTimeoutMs);
});
try {
return await Promise.race([cleanup(filePath), timedOut]);
} catch {
return 'unknown';
} finally {
if (timeout !== undefined) {
clearTimeout(timeout);
}
}
}
@@ -887,82 +887,91 @@ describe('download requests resume', () => {
});
});
it('deletes the retained partial before re-downloading a failed row from scratch', async () => {
jest.resetModules();
it.each(['failed', 'canceled'] as const)(
'deletes the retained partial asynchronously before re-downloading a %s row from scratch',
async (status) => {
jest.resetModules();
const failedRow = {
contentType: 'vod',
filePath: '/downloads/movie.mp4',
id: 42,
playlistId: 'playlist-1',
status: 'failed',
title: 'Movie',
url: 'https://example.test/movie.mp4',
xtreamId: 7,
};
const limit = jest
.fn()
.mockResolvedValueOnce([{ id: 'playlist-1' }])
.mockResolvedValueOnce([failedRow]);
const set = jest.fn<{ where: jest.Mock }, [Record<string, unknown>]>(
() => ({
const terminalRow = {
contentType: 'vod',
filePath: '/downloads/movie.mp4',
id: 42,
playlistId: 'playlist-1',
status,
title: 'Movie',
url: 'https://example.test/movie.mp4',
xtreamId: 7,
};
const limit = jest
.fn()
.mockResolvedValueOnce([{ id: 'playlist-1' }])
.mockResolvedValueOnce([terminalRow]);
const set = jest.fn<
{ where: jest.Mock },
[Record<string, unknown>]
>(() => ({
where: jest.fn().mockResolvedValue(undefined),
})
);
const db = {
select: jest.fn(() => ({
from: jest.fn(() => ({
where: jest.fn(() => ({ limit })),
}));
const db = {
select: jest.fn(() => ({
from: jest.fn(() => ({
where: jest.fn(() => ({ limit })),
})),
})),
})),
update: jest.fn(() => ({ set })),
};
const enqueueDownload = jest.fn();
const removePartialDownloadFile = jest.fn();
const authorizer = {
requireAuthorized: jest.fn(async (directory: string) => directory),
} as unknown as DownloadDirectoryAuthorizer;
update: jest.fn(() => ({ set })),
};
const enqueueDownload = jest.fn();
const removePartialDownloadFileWithTimeoutAsync = jest.fn(
async () => 'removed' as const
);
const authorizer = {
requireAuthorized: jest.fn(
async (directory: string) => directory
),
} as unknown as DownloadDirectoryAuthorizer;
jest.doMock('../../database/connection', () => ({
getDatabase: jest.fn().mockResolvedValue(db),
}));
jest.doMock('../url-safety', () => ({
assertRemoteUrlAllowed: jest.fn().mockResolvedValue(undefined),
}));
jest.doMock('./download-file-path', () => ({
removePartialDownloadFile,
}));
jest.doMock('./download-runtime', () => ({
enqueueDownload,
}));
jest.doMock('../../database/connection', () => ({
getDatabase: jest.fn().mockResolvedValue(db),
}));
jest.doMock('../url-safety', () => ({
assertRemoteUrlAllowed: jest.fn().mockResolvedValue(undefined),
}));
jest.doMock('./download-partial-cleanup', () => ({
removePartialDownloadFileWithTimeoutAsync,
}));
jest.doMock('./download-runtime', () => ({
enqueueDownload,
}));
const { startDownloadRequest } = await import('./download-requests');
const { startDownloadRequest } =
await import('./download-requests');
await expect(
startDownloadRequest(
{
contentType: 'vod',
downloadFolder: '/downloads',
playlistId: 'playlist-1',
title: 'Movie',
url: 'https://example.test/movie.mp4',
xtreamId: 7,
},
authorizer
)
).resolves.toEqual({ id: 42, success: true });
await expect(
startDownloadRequest(
{
contentType: 'vod',
downloadFolder: '/downloads',
playlistId: 'playlist-1',
title: 'Movie',
url: 'https://example.test/movie.mp4',
xtreamId: 7,
},
authorizer
)
).resolves.toEqual({ id: 42, success: true });
expect(removePartialDownloadFile).toHaveBeenCalledWith(
'/downloads/movie.mp4'
);
expect(set).toHaveBeenCalledWith(
expect.objectContaining({
filePath: null,
resumeValidator: null,
status: 'queued',
})
);
});
expect(
removePartialDownloadFileWithTimeoutAsync
).toHaveBeenCalledWith('/downloads/movie.mp4');
expect(set).toHaveBeenCalledWith(
expect.objectContaining({
filePath: null,
resumeValidator: null,
status: 'queued',
})
);
}
);
it('fails the re-download when the retained partial cannot be deleted', async () => {
jest.resetModules();
@@ -993,9 +1002,9 @@ describe('download requests resume', () => {
update: jest.fn(() => ({ set })),
};
const enqueueDownload = jest.fn();
const removePartialDownloadFile = jest.fn(() => {
throw new Error('EPERM: locked');
});
const removePartialDownloadFileWithTimeoutAsync = jest.fn(
async () => 'unknown' as const
);
const authorizer = {
requireAuthorized: jest.fn(async (directory: string) => directory),
} as unknown as DownloadDirectoryAuthorizer;
@@ -1006,8 +1015,8 @@ describe('download requests resume', () => {
jest.doMock('../url-safety', () => ({
assertRemoteUrlAllowed: jest.fn().mockResolvedValue(undefined),
}));
jest.doMock('./download-file-path', () => ({
removePartialDownloadFile,
jest.doMock('./download-partial-cleanup', () => ({
removePartialDownloadFileWithTimeoutAsync,
}));
jest.doMock('./download-runtime', () => ({
enqueueDownload,
@@ -10,8 +10,8 @@ import { getDatabase } from '../../database/connection';
import * as schema from '../../database/schema';
import { assertRemoteUrlAllowed } from '../url-safety';
import { DownloadDirectoryAuthorizer } from './download-directory-authorization';
import { removePartialDownloadFile } from './download-file-path';
import { getDownloadFileAvailabilityWithTimeoutAsync } from './download-file-availability';
import { removePartialDownloadFileWithTimeoutAsync } from './download-partial-cleanup';
import { resolveExistingDownloadIdentity } from './download-request-identity';
import { resolveStoredDownloadHeaders } from './download-request-headers';
import {
@@ -175,17 +175,19 @@ export async function startDownloadRequest(
};
}
if (item.status === 'failed' && item.filePath) {
// A failed row can still reference a retained .part; delete it
if (
['completed', 'failed', 'canceled'].includes(item.status) &&
item.filePath
) {
// A terminal row can still reference a retained .part; delete it
// before the restart clears filePath, or the file is orphaned.
// A locked .part must keep its database owner, so fail the
// restart instead of proceeding without the cleanup.
try {
removePartialDownloadFile(item.filePath);
} catch (error) {
// An unavailable or slow .part must keep its database owner.
const cleanup = await removePartialDownloadFileWithTimeoutAsync(
item.filePath
);
if (cleanup === 'unknown') {
console.error(
'[Downloads] Failed to delete retained partial before re-download:',
error
'[Downloads] Could not verify retained partial cleanup'
);
return {
error: 'Could not delete the previous partial file',
+7 -3
View File
@@ -109,9 +109,13 @@ variants, contextual buttons, and theme-aware styling.
actually settles, so later callers get independent bounded waits without
duplicating stalled native work. Only `ENOENT` and `ENOTDIR` are authoritative
absence; permission, I/O, and other probe errors remain unknown, so
`DOWNLOADS_START` leaves the completed row and file path untouched. The
coordinator counts both stable duplicate reasons as skipped. There is no
batch IPC,
`DOWNLOADS_START` leaves the completed row and file path untouched. Before a
completed-missing, failed, or canceled row clears its path, the same start IPC
asynchronously removes any retained `.part`. This cleanup also has a
one-second per-caller deadline, coalesces same-path work, and allows at most
four underlying unlinks; timeout, permission, and I/O failures keep the row's
ownership intact, while `ENOENT` and `ENOTDIR` safely proceed. 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.