From 09a036abc58d76a3e202f16272f1965c9aec7713 Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 11 Aug 2026 10:42:21 -0400 Subject: [PATCH 1/2] feat(blob): add openStream() so pre-streaming blob errors are catchable before the response commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stream() must return synchronously (the web Blob contract), so a missing or corrupt backing file only surfaces through the stream itself — by then an HTTP caller has typically already sent a 200 for a body that will never arrive. openStream() awaits the underlying file open and first read (where the header is validated), so missing files (404), in-flight writes/half-replicated blobs (503), and error stubs/truncation (500) reject as catchable BlobReadErrors while still streaming the content without buffering more than one chunk. Co-Authored-By: Claude Fable 5 --- resources/blob.ts | 34 +++++++++++++++++++++ unitTests/resources/blob.test.js | 51 ++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/resources/blob.ts b/resources/blob.ts index 094bb47fe9..f72855e7f4 100644 --- a/resources/blob.ts +++ b/resources/blob.ts @@ -816,6 +816,40 @@ class FileBackedBlob extends (Blob as unknown as { new (): Blob }) implements Bl return isBeingWritten; } } + /** + * Open the blob for streaming, resolving only once the backing content is confirmed readable. + * stream() must return synchronously (the web Blob contract), so a missing or corrupt backing + * file can only surface through the stream itself — by then an HTTP caller has typically already + * committed a 200 status for a body that will never arrive. This awaits the underlying file open + * and the first read (where the header is validated), so pre-streaming failures reject here as + * BlobReadErrors carrying their statusCode — missing file (404), in-flight write or + * half-replicated blob (503), error stub/truncation (500) — while holding at most one chunk in + * memory. Mid-stream failures still surface through the returned stream and any 'error' + * listeners registered via on(). + */ + async openStream(): Promise { + const storageInfo = storageInfoForBlob.get(this); + // in-memory content cannot fail to open, use the buffer-backed stream directly + if (storageInfo?.contentBuffer) return this.stream(); + const reader = this.stream().getReader(); + // the first read drives the file open (with its retry/timeout classification) and the header + // checks in the first pull, so everything wrong before content flows rejects here + const firstChunk = await reader.read(); + return new ReadableStream({ + start(controller) { + if (firstChunk.done) controller.close(); + else controller.enqueue(firstChunk.value); + }, + async pull(controller) { + const { done, value } = await reader.read(); + if (done) controller.close(); + else controller.enqueue(value); + }, + cancel(reason) { + return reader.cancel(reason); + }, + }); + } slice(start: number, end: number, type?: string): Blob { const sourceStorageInfo = storageInfoForBlob.get(this); const slicedBlob = new FileBackedBlob(type && { type }); diff --git a/unitTests/resources/blob.test.js b/unitTests/resources/blob.test.js index 8a19a26b22..7971cb5605 100644 --- a/unitTests/resources/blob.test.js +++ b/unitTests/resources/blob.test.js @@ -1199,6 +1199,57 @@ describe('Blob test', () => { env.setProperty(CONFIG_PARAMS.STORAGE_BLOBREADTIMEOUT, undefined); } }); + it('openStream() resolves for a healthy disk-backed blob and streams the full content', async () => { + const payload = randomBytes(20000); + const store = BlobTest.primaryStore.rootStore; + const blob = await createBlob(Readable.from(payload), { size: payload.length }); + await decodeFromDatabase(() => saveBlob(blob).saving, store); + const streamed = await streamToBytes(await blob.openStream()); + assert(streamed.equals(payload)); + // a slice shares the same backing file and must stream just the slice content + const slicedStream = await blob.slice(300, 400).openStream(); + assert((await streamToBytes(slicedStream)).equals(payload.subarray(300, 400))); + }); + it('openStream() resolves for a small in-memory blob', async () => { + const payload = randomBytes(100); + const blob = await createBlob(payload); + const streamed = await streamToBytes(await blob.openStream()); + assert(streamed.equals(payload)); + }); + it('openStream() rejects with a 404 for a cleanly-missing blob file, before any read', async () => { + const { blob, filePath } = await makeDiskBackedBlob(); + unlinkSync(filePath); + await assert.rejects(blob.openStream(), (error) => { + assert.equal(error.statusCode, 404); + assert.equal(error.code, 'ENOENT'); + return true; + }); + }); + it('openStream() rejects with a 503 while a write is in flight', async () => { + const { blob, filePath, store } = await makeDiskBackedBlob(); + const lockKey = getFileId(blob) + ':blob'; + assert(store.tryLock(lockKey), 'should be able to take the blob write lock for the test'); + try { + unlinkSync(filePath); // file gone while a "writer" still holds the lock + env.setProperty(CONFIG_PARAMS.STORAGE_BLOBREADTIMEOUT, '150'); + await assert.rejects(blob.openStream(), (error) => { + assert.equal(error.statusCode, 503); + return true; + }); + } finally { + store.unlock(lockKey); + env.setProperty(CONFIG_PARAMS.STORAGE_BLOBREADTIMEOUT, undefined); + } + }); + it('openStream() rejects with a 500 for a blob truncated to a self-consistent smaller size', async () => { + const { blob, filePath } = await makeDiskBackedBlob(); + truncateBlobConsistently(filePath, 256); + await assert.rejects(blob.openStream(), (error) => { + assert.equal(error.statusCode, 500); + assert.match(error.message, /size mismatch/); + return true; + }); + }); afterEach(function () { setAuditRetention(60000); setDeletionDelay(50); // restore shorter, but need to have it happen for the last test From 0e993c6be1fa1fa87317ffdf9cef35b8951863f9 Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 11 Aug 2026 10:47:36 -0400 Subject: [PATCH 2/2] review: .then() chain in the per-chunk pull, strictEqual in new tests Co-Authored-By: Claude Fable 5 --- resources/blob.ts | 11 +++++++---- unitTests/resources/blob.test.js | 8 ++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/resources/blob.ts b/resources/blob.ts index f72855e7f4..bc51749742 100644 --- a/resources/blob.ts +++ b/resources/blob.ts @@ -840,10 +840,13 @@ class FileBackedBlob extends (Blob as unknown as { new (): Blob }) implements Bl if (firstChunk.done) controller.close(); else controller.enqueue(firstChunk.value); }, - async pull(controller) { - const { done, value } = await reader.read(); - if (done) controller.close(); - else controller.enqueue(value); + pull(controller) { + // .then() chain rather than async/await: this runs per chunk, so skip the async state + // machine and extra promise allocations on the hot path + return reader.read().then(({ done, value }) => { + if (done) controller.close(); + else controller.enqueue(value); + }); }, cancel(reason) { return reader.cancel(reason); diff --git a/unitTests/resources/blob.test.js b/unitTests/resources/blob.test.js index 7971cb5605..5c51e1fc67 100644 --- a/unitTests/resources/blob.test.js +++ b/unitTests/resources/blob.test.js @@ -1220,8 +1220,8 @@ describe('Blob test', () => { const { blob, filePath } = await makeDiskBackedBlob(); unlinkSync(filePath); await assert.rejects(blob.openStream(), (error) => { - assert.equal(error.statusCode, 404); - assert.equal(error.code, 'ENOENT'); + assert.strictEqual(error.statusCode, 404); + assert.strictEqual(error.code, 'ENOENT'); return true; }); }); @@ -1233,7 +1233,7 @@ describe('Blob test', () => { unlinkSync(filePath); // file gone while a "writer" still holds the lock env.setProperty(CONFIG_PARAMS.STORAGE_BLOBREADTIMEOUT, '150'); await assert.rejects(blob.openStream(), (error) => { - assert.equal(error.statusCode, 503); + assert.strictEqual(error.statusCode, 503); return true; }); } finally { @@ -1245,7 +1245,7 @@ describe('Blob test', () => { const { blob, filePath } = await makeDiskBackedBlob(); truncateBlobConsistently(filePath, 256); await assert.rejects(blob.openStream(), (error) => { - assert.equal(error.statusCode, 500); + assert.strictEqual(error.statusCode, 500); assert.match(error.message, /size mismatch/); return true; });