From 7f9c925ffe8fa30038df12593eab454c44c42678 Mon Sep 17 00:00:00 2001 From: Travis Vachon Date: Tue, 16 Jan 2024 16:41:18 -0800 Subject: [PATCH 1/5] fix: try to improve error handling logic in unixfs streams right now errors in the `finalize` methods can leave a writer unclosed forever - this should help improve the DX issues described in https://github.com/web3-storage/console/issues/81#issuecomment-1894740447 by at least printing an error to the console with information about the problem --- packages/upload-client/src/unixfs.js | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/upload-client/src/unixfs.js b/packages/upload-client/src/unixfs.js index 3547ba71f..442b474ca 100644 --- a/packages/upload-client/src/unixfs.js +++ b/packages/upload-client/src/unixfs.js @@ -34,8 +34,13 @@ export function createFileEncoderStream(blob) { const unixfsWriter = UnixFS.createWriter({ writable, settings }) const fileBuilder = new UnixFSFileBuilder('', blob) void (async () => { - await fileBuilder.finalize(unixfsWriter) - await unixfsWriter.close() + try { + await fileBuilder.finalize(unixfsWriter) + } catch (e) { + console.log("Error finalizing file builder: ", e) + } finally { + await unixfsWriter.close() + } })() return readable } @@ -147,11 +152,16 @@ export function createDirectoryEncoderStream(files, options) { const { readable, writable } = new TransformStream({}, queuingStrategy) const unixfsWriter = UnixFS.createWriter({ writable, settings }) void (async () => { - const link = await rootDir.finalize(unixfsWriter) - if (options?.onDirectoryEntryLink) { - options.onDirectoryEntryLink({ name: '', ...link }) + try { + const link = await rootDir.finalize(unixfsWriter) + if (options?.onDirectoryEntryLink) { + options.onDirectoryEntryLink({ name: '', ...link }) + } + } catch (e) { + console.log("Error finalizing directory builder:", e) + } finally { + await unixfsWriter.close() } - await unixfsWriter.close() })() return readable From ed2b9b2312008381664bd48ff2e578c9f808278d Mon Sep 17 00:00:00 2001 From: Travis Vachon Date: Tue, 16 Jan 2024 16:43:05 -0800 Subject: [PATCH 2/5] fix: lint --- packages/upload-client/src/unixfs.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/upload-client/src/unixfs.js b/packages/upload-client/src/unixfs.js index 442b474ca..62a235cd1 100644 --- a/packages/upload-client/src/unixfs.js +++ b/packages/upload-client/src/unixfs.js @@ -37,7 +37,7 @@ export function createFileEncoderStream(blob) { try { await fileBuilder.finalize(unixfsWriter) } catch (e) { - console.log("Error finalizing file builder: ", e) + console.log('Error finalizing file builder: ', e) } finally { await unixfsWriter.close() } @@ -158,7 +158,7 @@ export function createDirectoryEncoderStream(files, options) { options.onDirectoryEntryLink({ name: '', ...link }) } } catch (e) { - console.log("Error finalizing directory builder:", e) + console.log('Error finalizing directory builder:', e) } finally { await unixfsWriter.close() } From bd9da8ec9b7e7abceb4e40b3117ea11bc8ea773d Mon Sep 17 00:00:00 2001 From: Travis Vachon Date: Tue, 16 Jan 2024 16:52:55 -0800 Subject: [PATCH 3/5] fix: ignore the catch clauses in code cov calcs --- packages/upload-client/src/unixfs.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/upload-client/src/unixfs.js b/packages/upload-client/src/unixfs.js index 62a235cd1..8dee8ee8c 100644 --- a/packages/upload-client/src/unixfs.js +++ b/packages/upload-client/src/unixfs.js @@ -36,6 +36,7 @@ export function createFileEncoderStream(blob) { void (async () => { try { await fileBuilder.finalize(unixfsWriter) + /* c8 ignore next 2 */ } catch (e) { console.log('Error finalizing file builder: ', e) } finally { @@ -157,6 +158,7 @@ export function createDirectoryEncoderStream(files, options) { if (options?.onDirectoryEntryLink) { options.onDirectoryEntryLink({ name: '', ...link }) } + /* c8 ignore next 2 */ } catch (e) { console.log('Error finalizing directory builder:', e) } finally { From 2cfa58b6a2a4d72c3c414852e0642699a064d937 Mon Sep 17 00:00:00 2001 From: Travis Vachon Date: Wed, 17 Jan 2024 08:40:47 -0800 Subject: [PATCH 4/5] fix: lint --- packages/upload-client/src/unixfs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/upload-client/src/unixfs.js b/packages/upload-client/src/unixfs.js index 8dee8ee8c..283282992 100644 --- a/packages/upload-client/src/unixfs.js +++ b/packages/upload-client/src/unixfs.js @@ -158,7 +158,7 @@ export function createDirectoryEncoderStream(files, options) { if (options?.onDirectoryEntryLink) { options.onDirectoryEntryLink({ name: '', ...link }) } - /* c8 ignore next 2 */ + /* c8 ignore next 2 */ } catch (e) { console.log('Error finalizing directory builder:', e) } finally { From 824ed1284f8ce2a6a23d36303c42f79c38797743 Mon Sep 17 00:00:00 2001 From: Travis Vachon Date: Thu, 18 Jan 2024 16:24:25 -0800 Subject: [PATCH 5/5] fix: use abort instead of squashing error confirmed this results in better behavior in the web console! was also able to test the error path here - I tried to create a `File` that represents a directory like in runtime error but could not reproduce - that seems to be a weird edge case that is hard to reproduce outside the browser context --- packages/upload-client/src/unixfs.js | 13 +++++-------- packages/upload-client/test/unixfs.test.js | 13 +++++++++++++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/upload-client/src/unixfs.js b/packages/upload-client/src/unixfs.js index 283282992..c8334401e 100644 --- a/packages/upload-client/src/unixfs.js +++ b/packages/upload-client/src/unixfs.js @@ -36,11 +36,10 @@ export function createFileEncoderStream(blob) { void (async () => { try { await fileBuilder.finalize(unixfsWriter) - /* c8 ignore next 2 */ - } catch (e) { - console.log('Error finalizing file builder: ', e) - } finally { await unixfsWriter.close() + /* c8 ignore next 3 */ + } catch (e) { + await unixfsWriter.writer.abort(/** @type {Error} */ (e)) } })() return readable @@ -158,11 +157,9 @@ export function createDirectoryEncoderStream(files, options) { if (options?.onDirectoryEntryLink) { options.onDirectoryEntryLink({ name: '', ...link }) } - /* c8 ignore next 2 */ - } catch (e) { - console.log('Error finalizing directory builder:', e) - } finally { await unixfsWriter.close() + } catch (e) { + await unixfsWriter.writer.abort(/** @type {Error} */ (e)) } })() diff --git a/packages/upload-client/test/unixfs.test.js b/packages/upload-client/test/unixfs.test.js index 2a45be0ad..de5868f3a 100644 --- a/packages/upload-client/test/unixfs.test.js +++ b/packages/upload-client/test/unixfs.test.js @@ -139,4 +139,17 @@ describe('UnixFS', () => { 'bafybeie4fxkioskwb4h7xpb5f6tbktm4vjxt7rtsqjit72jrv3ii5h26sy' ) }) + + describe('encodeDirectory', async () => { + it('throws an error when onDirectoryEntryLink throws an error', async () => { + await assert.rejects(async () => { + const files = [new File(['file'], 'file.txt')] + await encodeDirectory(files, { + onDirectoryEntryLink: () => { + throw new Error('') + }, + }) + }) + }) + }) })