Skip to content

Commit f42ea1c

Browse files
authored
fix: handle zero-length reads to prevent EIO errors during file downloads (#400)
1 parent 1a818b0 commit f42ea1c

7 files changed

Lines changed: 122 additions & 4 deletions

File tree

src/backend/features/fuse/on-read/download-cache/download-and-save-block.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,31 @@ describe('downloadAndCacheBlock', () => {
176176
expect(props.onDownloadProgress).not.toHaveBeenCalled();
177177
});
178178

179+
it('retries range download and writes once a later attempt succeeds', async () => {
180+
const props = createProps();
181+
downloadFileRangeMock
182+
.mockResolvedValueOnce({ error: new Error('network failed') })
183+
.mockResolvedValueOnce({ data: Buffer.from('downloaded') });
184+
185+
await expect(downloadAndCacheBlock(props)).resolves.toStrictEqual({ data: undefined });
186+
187+
expect(downloadFileRangeMock).toHaveBeenCalledTimes(2);
188+
expect(writeChunkToDiskMock).toHaveBeenCalledWith('/tmp/cache-file', Buffer.from('downloaded'), 100);
189+
expect(isRangeHydrated(props.state, { position: props.blockStart, length: props.blockLength })).toBe(true);
190+
});
191+
192+
it('stops retrying after max attempts when download keeps failing', async () => {
193+
const props = createProps();
194+
downloadFileRangeMock.mockResolvedValue({ error: new Error('network failed') });
195+
196+
await expect(downloadAndCacheBlock(props)).resolves.toStrictEqual({ error: new Error('network failed') });
197+
198+
expect(downloadFileRangeMock).toHaveBeenCalledTimes(3);
199+
expect(writeChunkToDiskMock).not.toHaveBeenCalled();
200+
expect(isRangeHydrated(props.state, { position: props.blockStart, length: props.blockLength })).toBe(false);
201+
expect(props.onDownloadProgress).not.toHaveBeenCalled();
202+
});
203+
179204
it('does not mark hydrated or emit progress when the disk write fails', async () => {
180205
const props = createProps();
181206
writeChunkToDiskMock.mockRejectedValue(new Error('write failed'));

src/backend/features/fuse/on-read/download-cache/download-and-save-block.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ import { getHydratedBytes, type FileHydrationState, markBlocksInRangeDownloaded
44
import { type File } from '../../../../../context/virtual-drive/files/domain/File';
55
import { downloadFileRange } from '../../../../../infra/environment/download-file/download-file';
66
import { type Result } from '../../../../../context/shared/domain/Result';
7+
import { delay } from '../../../../../shared/async/delay';
8+
9+
const MAX_DOWNLOAD_ATTEMPTS = 3;
10+
const RETRY_BASE_DELAY_MS = 150;
11+
712
type Props = {
813
bucketId: HandleReadDeps['bucketId'];
914
mnemonic: HandleReadDeps['mnemonic'];
@@ -33,14 +38,15 @@ export async function downloadAndCacheBlock({
3338
if (isAborted(state)) return { data: undefined };
3439

3540
try {
36-
const download = await downloadFileRange({
41+
const download = await downloadBlockWithRetry({
3742
fileId: virtualFile.contentsId,
3843
bucketId,
3944
mnemonic,
4045
network,
4146
range: { position: blockStart, length: blockLength },
4247
signal: state.abortController.signal,
4348
});
49+
4450
if (isAborted(state)) return { data: undefined };
4551
if (download.error) return { error: download.error };
4652

@@ -60,3 +66,48 @@ export async function downloadAndCacheBlock({
6066
function isAborted(state: FileHydrationState): boolean {
6167
return state.abortController.signal.aborted;
6268
}
69+
70+
type DownloadBlockWithRetryProps = {
71+
fileId: string;
72+
bucketId: HandleReadDeps['bucketId'];
73+
mnemonic: HandleReadDeps['mnemonic'];
74+
network: HandleReadDeps['network'];
75+
range: { position: number; length: number };
76+
signal: AbortSignal;
77+
};
78+
79+
async function downloadBlockWithRetry({
80+
fileId,
81+
bucketId,
82+
mnemonic,
83+
network,
84+
range,
85+
signal,
86+
}: DownloadBlockWithRetryProps): Promise<Result<Buffer, Error>> {
87+
let lastError: Error | undefined;
88+
89+
for (let attempt = 1; attempt <= MAX_DOWNLOAD_ATTEMPTS; attempt++) {
90+
if (signal.aborted) return { data: Buffer.alloc(0) };
91+
92+
const download = await downloadFileRange({
93+
fileId,
94+
bucketId,
95+
mnemonic,
96+
network,
97+
range,
98+
signal,
99+
});
100+
101+
if (download.data) {
102+
return { data: download.data };
103+
}
104+
105+
lastError = download.error ?? new Error('Unknown error while downloading file range');
106+
107+
if (attempt < MAX_DOWNLOAD_ATTEMPTS) {
108+
await delay(RETRY_BASE_DELAY_MS * attempt);
109+
}
110+
}
111+
112+
return { error: lastError ?? new Error('Unable to download file range') };
113+
}

src/backend/features/fuse/on-read/download-cache/hydration-state.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ export function getHydratedBytes(state: FileHydrationState): number {
134134
}
135135

136136
function blocksWithinRange({ position, length }: ReadRange): Array<number> {
137+
if (length <= 0) return [];
137138
const first = blockIndexForByte(position);
138139
const last = blockIndexForByte(position + length - 1);
139140
const blocks: number[] = [];

src/backend/features/virtual-drive/controllers/operations/read.controller.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,26 @@ describe('readController', () => {
5252
expect(readMock).not.toHaveBeenCalled();
5353
});
5454

55+
it('should send EINVAL and empty buffer when length is not a number', async () => {
56+
req.body = { path: '/file.mp4', length: '10', offset: 0, processName: 'vlc' };
57+
58+
await readController(req, res, container);
59+
60+
expect(res.set).toHaveBeenCalledWith('X-Errno', String(FuseCodes.EINVAL));
61+
expect(res.send).toHaveBeenCalledWith(Buffer.alloc(0));
62+
expect(readMock).not.toHaveBeenCalled();
63+
});
64+
65+
it('should send EINVAL and empty buffer when offset is negative', async () => {
66+
req.body = { path: '/file.mp4', length: 10, offset: -1, processName: 'vlc' };
67+
68+
await readController(req, res, container);
69+
70+
expect(res.set).toHaveBeenCalledWith('X-Errno', String(FuseCodes.EINVAL));
71+
expect(res.send).toHaveBeenCalledWith(Buffer.alloc(0));
72+
expect(readMock).not.toHaveBeenCalled();
73+
});
74+
5575
it('should normalize path by adding leading slash', async () => {
5676
req.body = { path: 'file.mp4', length: 10, offset: 0, processName: 'vlc' };
5777
readMock.mockResolvedValue({ data: Buffer.alloc(0) });

src/backend/features/virtual-drive/controllers/operations/read.controller.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ export async function readController(req: Request, res: Response, container: Con
99
const { path: rawPath, length, offset } = req.body;
1010
const processName = typeof req.body.processName === 'string' ? req.body.processName : '';
1111

12-
if (rawPath === undefined || length === undefined || offset === undefined) {
13-
logger.error({ msg: '[FUSE DAEMON] Read: missing required fields', body: req.body });
12+
if (!isValidReadPayload(rawPath, length, offset)) {
13+
logger.error({ msg: '[FUSE DAEMON] Read: invalid payload', body: req.body });
1414
res.set('X-Errno', String(FuseCodes.EINVAL));
1515
res.send(Buffer.alloc(0));
1616
return;
@@ -34,3 +34,24 @@ export async function readController(req: Request, res: Response, container: Con
3434
res.set('Content-Type', 'application/octet-stream');
3535
res.send(result.data);
3636
}
37+
38+
function isValidReadPayload(path: unknown, length: unknown, offset: unknown): boolean {
39+
if (typeof path !== 'string' || path.length === 0) return false;
40+
if (!isValidReadLength(length)) return false;
41+
if (!isValidReadOffset(offset)) return false;
42+
return true;
43+
}
44+
45+
function isValidReadLength(length: unknown): length is number {
46+
if (typeof length !== 'number') return false;
47+
if (!Number.isInteger(length)) return false;
48+
if (length < 0) return false;
49+
return true;
50+
}
51+
52+
function isValidReadOffset(offset: unknown): offset is number {
53+
if (typeof offset !== 'number') return false;
54+
if (!Number.isInteger(offset)) return false;
55+
if (offset < 0) return false;
56+
return true;
57+
}

src/infra/drive-server/client/interceptors/rate-limiter/wait-for-delay.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { delay } from './delay';
1+
import { delay } from '../../../../../shared/async/delay';
22
import { DelayState } from './rate-limiter.types';
33

44
export async function waitForDelay(delayState: DelayState, ms: number): Promise<void> {

src/infra/drive-server/client/interceptors/rate-limiter/delay.ts renamed to src/shared/async/delay.ts

File renamed without changes.

0 commit comments

Comments
 (0)