fix(downloads): clean reservations when ownership writes fail

This commit is contained in:
4gray committed 2026-09-08 05:22:04 +02:00
1 parent 1dfafb404d
commit 4f55bc5e66
3 files changed
+59 -3

No files matched your search

@@ -180,3 +180,48 @@ it('binds a fresh reservation before a replacement can arrive during the HTTP wa
await rm(directory, { recursive: true, force: true });
}
});
it.each([false, true])(
'cleans a failed reservation journal without deleting a replacement (replaced=%s)',
async (replaced) => {
const directory = await mkdtemp(
join(tmpdir(), 'archive-reservation-failure-')
);
const filePath = join(directory, 'show.ts');
const task: DownloadTask = {
id: 20,
directory,
fileName: 'show.ts',
url: 'https://provider.test/archive.ts',
catchup: {
channelName: 'News',
startTimestamp: 100,
stopTimestamp: 200,
},
};
const failure = new Error('SQLite write failed');
jest.mocked(recordArchivePartial).mockImplementationOnce(async () => {
if (replaced) {
await rename(filePath + '.part', join(directory, 'original'));
await writeFile(filePath + '.part', 'foreign bytes');
}
throw failure;
});
try {
await expect(
reserveTarget({} as DownloadsDatabase, task)
).rejects.toBe(failure);
if (replaced) {
expect(await readFile(filePath + '.part', 'utf8')).toBe(
'foreign bytes'
);
} else {
await expect(lstat(filePath + '.part')).rejects.toMatchObject({
code: 'ENOENT',
});
}
} finally {
await rm(directory, { recursive: true, force: true });
}
}
);
@@ -1,5 +1,6 @@
import { closeSync, fstatSync, openSync } from 'node:fs';
import { lstat } from 'node:fs/promises';
import { cleanupCatchupPartial } from './download-catchup-cleanup';
import { cleanupStoredCatchupPartial } from './download-catchup-removal';
import {
clearArchiveFinalization,
@@ -67,6 +68,13 @@ export async function reserveOwnedCatchupTarget(
const identity = task.catchupExpectedPartialIdentity;
if (!identity)
throw new Error('Archive reservation identity is unavailable');
await recordArchivePartial(db, task.id, reservation.path, identity);
try {
await recordArchivePartial(db, task.id, reservation.path, identity);
} catch (error) {
// No media bytes have been written. Without a usable journal, remove
// only the empty reservation whose identity came from our descriptor.
await cleanupCatchupPartial(reservation.path, identity);
throw error;
}
return reservation;
}
+5 -2
View File
@@ -118,8 +118,11 @@ filesystems without hardlinks. Cleanup remains synchronous after the
runtime guard, so a completion transition cannot interleave with unlink.
Missing archive re-downloads claim a fresh reservation instead of inheriting the
completed file's identity.
A kill between exclusive copy-file creation and its identity journal commit can
leave an unowned **empty** destination: no bytes are written before the commit.
If the initial reservation journal write fails, descriptor-owned cleanup removes
the empty partial while preserving a replacement. A kill between exclusive
reservation/copy-file creation and its identity journal commit (or a failure of
that unjournaled cleanup) can leave an unowned **empty** destination: no bytes
are written before the commit.
Recovery preserves that file rather than guessing ownership; Retry uses a
numbered free destination. SQLite and filesystem creation cannot commit
atomically, and portable rename cannot guarantee no-clobber publication on the