Repository navigation
byob reader support for Blob.stream() #47993
Description
Activity
Hey! so I was trying to look around to solve this issue and while writing tests for it I noticed a weird behavior of node in comparison to browsers:
take the following script:
async function fn() { const b = new Blob([1, 2, 3, 4, 5, 5, 6, 6]).stream().getReader(); const chunks = []; while (true) { const { done, value } = await b.read(); if (done) break; chunks.push(value); console.log('read value', value); } console.log(chunks); } await fn();
If executed in browsers (tested on chrome and firefox) the output looks like this:
Whereas when trying this with node v20.1.0
debadreechatterjee@Debadree-MacBook-Pro node % node test.mjs read value Uint8Array(1) [ 49 ] debadreechatterjee@Debadree-MacBook-Pro node %
it appears the promise hangs which leads to the process exiting, is this a bug or is my script something wrong
Ok interestingly if we call
blob.arrayBufferbefore creating the stream this works fine (edit: this also seems flakey)Not really we have to change the way we respond too no? I was trying out something like this
I was trying something like this
diff --git a/lib/internal/blob.js b/lib/internal/blob.js index ee8e1c7581..47face7a93 100644 --- a/lib/internal/blob.js +++ b/lib/internal/blob.js @@ -321,6 +321,7 @@ class Blob { const reader = this[kHandle].getReader(); return new lazyReadableStream({ + type: 'bytes', start(c) { // There really should only be one read at a time so using an // array here is purely defensive. @@ -353,7 +354,20 @@ class Blob { return; } if (buffer !== undefined) { - c.enqueue(new Uint8Array(buffer)); + if (c.byobRequest) { + const byobRequestView = c.byobRequest.view; + const byteLength = buffer.byteLength; + const viewByteLength = byobRequestView.byteLength; + const viewByteOffset = byobRequestView.byteOffset; + for (let i = 0; i < byteLength;) { + const chunk = new Uint8Array(buffer.slice(i, i + viewByteLength)); + byobRequestView.set(chunk, viewByteOffset); + i += viewByteLength; + c.byobRequest.respond(viewByteLength); + } + } else { + c.enqueue(new Uint8Array(buffer)); + } } pending.resolve(); });
but this causes some error with detached array buffers which i dont yet understand 😅
When you enqueue a uint8array then the underlying ArrayBuffer gets detached.
And you can no longer use that ArrayBuffer for anything else.ab = new Uint8Array([97, 98]).buffer chunk1 = new Uint8Array(ab, 0, 1) chunk2 = new Uint8Array(ab, 1, 1) rs = new ReadableStream({ type: 'bytes', start(ctrl) { console.log(chunk1.byteLength, ab.byteLength) // 1, 2 ctrl.enqueue(chunk1) console.log(chunk1.byteLength, ab.byteLength) // 0, 0 // if you now try to enqueue chunk2 then it will fail // ctrl.enqueue(chunk2) ctrl.close() } }) into = new Uint8Array(2) rs.getReader({mode: 'byob'}).read(into)
It's as if you have transfered the arraybuffer to another thread using
postMessage(ab, [ ab ])
the arraybuffer byteLengths gets set to 0Reacted by Debadree ChatterjeeAh understood so what could be a work around for the byob mode?
respondWithNewViewmaybe?Haven't investigated the source code so much.
but i had one issue in earlier NodeJS version where it first read everything as a hole arrayBuffer
and then the ReadableStream would just slice them into smaller views with subarray so it was totally unexpected when i receive a byteOffset other then 0
and that eachchunk.bufferwhere the same instance.
that was abnormalso if you are for some reason reusing the same ArrayBuffer twice then that might be an issue.
anyhow. i'm not sure if you should enqueue a uint8array anyway. i suppose you are going to use
controller.byobRequest.respond(bytesRead)the stream spec have a good turtorial on how to use files + byte streams here
https://streams.spec.whatwg.org/#example-rbs-pullconst fs = require("fs").promises; const DEFAULT_CHUNK_SIZE = 1024; function makeReadableByteFileStream(filename) { let fileHandle; let position = 0; return new ReadableStream({ type: "bytes", async start() { fileHandle = await fs.open(filename, "r"); }, async pull(controller) { // Even when the consumer is using the default reader, the auto-allocation // feature allocates a buffer and passes it to us via byobRequest. const v = controller.byobRequest.view; const { bytesRead } = await fileHandle.read(v, 0, v.byteLength, position); if (bytesRead === 0) { await fileHandle.close(); controller.close(); controller.byobRequest.respond(0); } else { position += bytesRead; controller.byobRequest.respond(bytesRead); } }, cancel() { return fileHandle.close(); }, autoAllocateChunkSize: DEFAULT_CHUNK_SIZE }); }
Ah got it! i think responding multiple times by slicing the array buffer wont work, i think this would need changes on c++ side too, trying to investigate thank you for the inputs!!
I don't see why slicing the arraybuffer would not work. you are creating a copy after all. but doing so this way would probably also indicate that you probably are also are doing something wrong in the first place... it would be better to read into an existing arraybuffer view instead
Reacted by Debadree ChatterjeeOh ok @KhafraDev was right it seems to be as simple as adding type: 'bytes' heh I went of on unnecessary tangent but it seems the promise hang issue is what needs to be looked into
Oh ok @KhafraDev was right it seems to be as simple as adding type: 'bytes' heh I went of on unnecessary tangent but it seems the promise hang issue is what needs to be looked into
i tough so too.
i don't know how the stream spec works exactly. but maybe if youctrl.enqueue()something in a bytes stream
then the stream will do thecontroller.byobRequest.respond(x)automatically for you (internally). as well as setting thebyobRequestViewto what you enqueued.Reacted by Debadree Chatterjee- added a commit that references this issue
on May 28, 2023 Hey so I opened a PR to fix the hanging issue I pointed out in #47993 (comment) here #48232 but as for adding 'bytes' support there seems like we have another issue #48233
- added a commit that references this issue
on Jun 11, 2023 - added a commit that references this issue
on Jul 3, 2023 - added 2 commits that reference this issue
on Aug 14, 2023 - added a commit that references this issue
on Sep 19, 2023 - added a commit that references this issue
on Sep 22, 2023 - added a commit that references this issue
on Sep 28, 2023
Version
all
Platform
No response
Subsystem
No response
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
every time
What is the expected behavior? Why is that the expected behavior?
get a byob reader back
What do you see instead?
Additional information
Works in Firefox, Deno
stream()? w3c/FileAPI#186