From 0a1021177bfcf2b8dea3efcf2e117488cc1ecd1c Mon Sep 17 00:00:00 2001 From: Meinhard Benn Date: Mon, 10 Aug 2026 19:46:11 +0000 Subject: [PATCH 1/2] fix(main): stream hygiene for dweb fetch error paths Follow-up to #158, addressing the two non-blocking notes from its final review round: - Cancel the response body on a non-OK dweb response, and cancel the reader when a chunk callback throws (e.g. disk full mid-save), so the protocol handler stops pulling from the gateway instead of draining the rest of the transfer in the background until GC. - Loop on FileHandle.write until the whole chunk is on disk; a partial write without an error would previously truncate the saved file silently. --- src/main/http-fetch.js | 16 ++++++++++++- src/main/http-fetch.test.js | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/main/http-fetch.js b/src/main/http-fetch.js index 73a73e9f..37581045 100644 --- a/src/main/http-fetch.js +++ b/src/main/http-fetch.js @@ -47,6 +47,9 @@ async function sessionFetchStream(url, onChunk, { timeout = DEFAULT_TIMEOUT } = try { const response = await session.defaultSession.fetch(url, { signal: controller.signal }); if (!response.ok) { + // Release the connection — an abandoned body keeps the protocol + // handler pulling from the gateway until GC collects the stream. + response.body?.cancel().catch(() => {}); throw new Error(`Failed to download: HTTP ${response.status}`); } // Headers arriving is activity too — don't let a slow time-to-headers @@ -69,6 +72,11 @@ async function sessionFetchStream(url, onChunk, { timeout = DEFAULT_TIMEOUT } = touch(); await onChunk(Buffer.from(value)); } + } catch (err) { + // e.g. onChunk threw (disk full mid-write) — stop the transfer + // instead of letting the handler drain the rest in the background. + reader.cancel().catch(() => {}); + throw err; } finally { controller.signal.removeEventListener('abort', onAbort); } @@ -107,7 +115,13 @@ async function sessionFetchToFile(url, destPath, opts) { if (!handle) { handle = await fs.promises.open(destPath, 'w'); } - await handle.write(chunk); + // FileHandle.write may perform a partial write without throwing; + // loop until the whole chunk is on disk. + let offset = 0; + while (offset < chunk.length) { + const { bytesWritten } = await handle.write(chunk, offset, chunk.length - offset); + offset += bytesWritten; + } }, opts ); diff --git a/src/main/http-fetch.test.js b/src/main/http-fetch.test.js index 6f1ca9e9..af96b165 100644 --- a/src/main/http-fetch.test.js +++ b/src/main/http-fetch.test.js @@ -147,4 +147,51 @@ describe('http-fetch dweb schemes', () => { fs.rmSync(dir, { recursive: true, force: true }); } }); + + test('cancels the body stream on a non-OK response', async () => { + const cancel = jest.fn().mockResolvedValue(undefined); + session.defaultSession.fetch.mockResolvedValue({ ok: false, status: 503, body: { cancel } }); + + await expect(fetchBuffer('bzz://example.eth/missing.png')).rejects.toThrow( + 'Failed to download: HTTP 503' + ); + expect(cancel).toHaveBeenCalled(); + }); + + test('cancels the transfer when writing a chunk fails', async () => { + const cancelled = jest.fn(); + session.defaultSession.fetch.mockResolvedValue({ + ok: true, + status: 200, + body: new ReadableStream({ + pull(controller) { + controller.enqueue(Uint8Array.from(Buffer.from('chunk'))); + }, + cancel: cancelled, + }), + }); + // Parent directory does not exist, so opening the destination fails on + // the first chunk. + const destPath = path.join(os.tmpdir(), 'http-fetch-no-such-dir', 'nested', 'pic.png'); + + await expect(fetchToFile('bzz://example.eth/pic.png', destPath)).rejects.toThrow(); + expect(cancelled).toHaveBeenCalled(); + }); + + test('retries partial file writes until the whole chunk is on disk', async () => { + session.defaultSession.fetch.mockResolvedValue(okResponse('abcdef')); + const write = jest + .fn() + .mockImplementation(async (chunk, offset, length) => ({ bytesWritten: Math.min(2, length) })); + const close = jest.fn().mockResolvedValue(undefined); + const openSpy = jest.spyOn(fs.promises, 'open').mockResolvedValue({ write, close }); + + try { + await fetchToFile('bzz://example.eth/pic.png', '/tmp/http-fetch-partial-write.png'); + expect(write.mock.calls.map((call) => call[1])).toEqual([0, 2, 4]); + expect(close).toHaveBeenCalled(); + } finally { + openSpy.mockRestore(); + } + }); }); From 5d2ef6af1899d00063c63e5cabdb5a49beb81554 Mon Sep 17 00:00:00 2001 From: Meinhard Benn Date: Mon, 10 Aug 2026 19:54:54 +0000 Subject: [PATCH 2/2] fix(main): guard against zero-progress file writes Per review: if FileHandle.write ever returns bytesWritten 0 (exotic FUSE/network mounts), fail loudly instead of hanging in the retry loop until the inactivity timeout aborts the transfer. --- src/main/http-fetch.js | 3 +++ src/main/http-fetch.test.js | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/main/http-fetch.js b/src/main/http-fetch.js index 37581045..2e406f4f 100644 --- a/src/main/http-fetch.js +++ b/src/main/http-fetch.js @@ -120,6 +120,9 @@ async function sessionFetchToFile(url, destPath, opts) { let offset = 0; while (offset < chunk.length) { const { bytesWritten } = await handle.write(chunk, offset, chunk.length - offset); + if (bytesWritten === 0) { + throw new Error('File write made no progress'); + } offset += bytesWritten; } }, diff --git a/src/main/http-fetch.test.js b/src/main/http-fetch.test.js index af96b165..a3050f10 100644 --- a/src/main/http-fetch.test.js +++ b/src/main/http-fetch.test.js @@ -178,6 +178,22 @@ describe('http-fetch dweb schemes', () => { expect(cancelled).toHaveBeenCalled(); }); + test('fails loudly when a file write makes no progress', async () => { + session.defaultSession.fetch.mockResolvedValue(okResponse('abcdef')); + const write = jest.fn().mockResolvedValue({ bytesWritten: 0 }); + const close = jest.fn().mockResolvedValue(undefined); + const openSpy = jest.spyOn(fs.promises, 'open').mockResolvedValue({ write, close }); + + try { + await expect( + fetchToFile('bzz://example.eth/pic.png', '/tmp/http-fetch-no-progress.png') + ).rejects.toThrow('File write made no progress'); + expect(write).toHaveBeenCalledTimes(1); + } finally { + openSpy.mockRestore(); + } + }); + test('retries partial file writes until the whole chunk is on disk', async () => { session.defaultSession.fetch.mockResolvedValue(okResponse('abcdef')); const write = jest