fix(downloads): retain captures until replacement restoration succeeds

This commit is contained in:
4gray committed 2026-09-08 05:47:07 +02:00
1 parent e257b77fca
commit b05db7ca21
4 files changed
+87 -15

No files matched your search

@@ -1,4 +1,4 @@
import { lstatSync, rmdirSync, unlinkSync } from 'node:fs';
import { linkSync, lstatSync, rmdirSync, unlinkSync } from 'node:fs';
import { dirname } from 'node:path';
import {
sameArchiveFileIdentity,
@@ -11,29 +11,47 @@ export function cleanupArchiveCapture(
proof: ArchiveDownloadProof | undefined
): void {
if (!proof) return;
cleanupCapture(proof.partialCleanupPath, proof.partialIdentity);
cleanupCapture(
proof.partialCleanupPath,
proof.partialIdentity,
proof.filePath + '.part'
);
if (proof.phase !== 'transfer')
cleanupCapture(proof.finalCleanupPath, proof.finalIdentity);
cleanupCapture(
proof.finalCleanupPath,
proof.finalIdentity,
proof.filePath
);
}
function cleanupCapture(
path: string | undefined,
identity: ArchiveFileIdentity
identity: ArchiveFileIdentity,
publicPath: string
): void {
if (!path) return;
let file;
try {
const file = lstatSync(path);
if (file.isFile() && sameArchiveFileIdentity(file, identity)) {
unlinkSync(path);
} else {
console.warn(
'[Downloads] Replaced file retained for recovery:',
path
);
}
file = lstatSync(path);
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error;
}
if (file) {
if (!(file.isFile() && sameArchiveFileIdentity(file, identity))) {
// Restore without clobbering. If a prior restore linked successfully
// but unlink failed, the matching public entry permits cleanup retry.
try {
linkSync(path, publicPath);
} catch (error) {
if (
(error as NodeJS.ErrnoException).code !== 'EEXIST' ||
!sameArchiveFileIdentity(lstatSync(publicPath), file)
)
throw error;
}
}
unlinkSync(path);
}
try {
rmdirSync(dirname(path));
} catch {
@@ -76,6 +76,56 @@ it('restores a replacement captured at the cleanup boundary', () => {
removeJournaledCatchupPartial(filePath, proof, recordCapture);
expect(readFileSync(filePath + '.part', 'utf8')).toBe('unrelated bytes');
});
it('retains a replacement capture until no-clobber restoration succeeds', () => {
jest.mocked(renameSync).mockImplementationOnce((from, to) => {
actual.renameSync(from, join(directory, 'original'));
writeFileSync(from, 'foreign bytes');
actual.renameSync(from, to);
});
jest.mocked(linkSync).mockImplementation(() => {
throw Object.assign(new Error('hardlinks unavailable'), {
code: 'ENOTSUP',
});
});
expect(() =>
removeJournaledCatchupPartial(filePath, proof, recordCapture)
).toThrow('hardlinks unavailable');
const capture = proof.partialCleanupPath!;
expect(readFileSync(capture, 'utf8')).toBe('foreign bytes');
expect(() =>
removeJournaledCatchupPartial(filePath, proof, recordCapture)
).toThrow('hardlinks unavailable');
expect(proof.partialCleanupPath).toBe(capture);
jest.mocked(linkSync).mockImplementationOnce(() => {
throw Object.assign(new Error('destination parent missing'), {
code: 'ENOENT',
});
});
expect(() =>
removeJournaledCatchupPartial(filePath, proof, recordCapture)
).toThrow('destination parent missing');
expect(readFileSync(capture, 'utf8')).toBe('foreign bytes');
jest.mocked(linkSync).mockImplementation(actual.linkSync);
writeFileSync(filePath + '.part', 'newer public bytes');
expect(() =>
removeJournaledCatchupPartial(filePath, proof, recordCapture)
).toThrow();
expect(readFileSync(filePath + '.part', 'utf8')).toBe('newer public bytes');
expect(readFileSync(capture, 'utf8')).toBe('foreign bytes');
actual.unlinkSync(filePath + '.part');
jest.mocked(unlinkSync).mockImplementationOnce(() => {
throw new Error('restored capture still locked');
});
expect(() =>
removeJournaledCatchupPartial(filePath, proof, recordCapture)
).toThrow('restored capture still locked');
expect(readFileSync(filePath + '.part', 'utf8')).toBe('foreign bytes');
expect(readFileSync(capture, 'utf8')).toBe('foreign bytes');
removeJournaledCatchupPartial(filePath, proof, recordCapture);
expect(readFileSync(filePath + '.part', 'utf8')).toBe('foreign bytes');
expect(() => lstatSync(capture)).toThrow();
});
it('retries a durable capture after an I/O error without needing hardlinks', () => {
jest.mocked(unlinkSync).mockImplementationOnce(() => {
throw Object.assign(new Error('locked'), { code: 'EACCES' });
@@ -65,11 +65,12 @@ function removeOwnedEntry(
try {
linkSync(captured, path);
unlinkSync(captured);
} catch {
} catch (error) {
console.warn(
'[Downloads] Replaced file retained for recovery:',
captured
);
throw error;
}
}
} finally {
+4 -1
View File
@@ -114,7 +114,10 @@ startup recovery use the same journal-backed cleanup as manual actions. Cancel,
failure and removal of an unfinished attempt also clean its journaled final target;
removing a completed row preserves its media. Retry cleans an incomplete owned
final before reserving a destination. A failed
unlink keeps that durable pointer; Remove/Clear, Retry/Resume and fresh
unlink or replacement restoration keeps that durable pointer and blocks journal
deletion/replacement. Later cleanup retries no-clobber restoration of captured
foreign entries; an occupied public path or unsupported hardlinks preserves both
the capture and its journal for recovery. Remove/Clear, Retry/Resume and fresh
reservations retry identity-verified cleanup, including after restart and on
filesystems without hardlinks. Cleanup remains synchronous after the
runtime guard, so a completion transition cannot interleave with unlink.