fixup! update logic, add tests and doc

doc/api/fs.md

@@ -450,6 +450,10 @@ number of bytes read is zero.
 
 <!-- YAML
 added: v17.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/46933
+    description: Added option to create a 'bytes' stream.
 -->
 
 > Stability: 1 - Experimental

lib/internal/fs/promises.js

@@ -286,11 +286,10 @@ class FileHandle extends EventEmitterMixin(JSTransferable) {
         readableStreamCancel,
         ReadableStream,
       } = require('internal/webstreams/readablestream');
-      this[kRef]();
-      this.once('close', () => {
-        readableStreamCancel(readable);
-      });
+
       const readFn = FunctionPrototypeBind(this.read, this);
+      const ondone = FunctionPrototypeBind(this[kUnref], this);
+
       readable = new ReadableStream({
         type: 'bytes',
         autoAllocateChunkSize: 16384,
@@ -300,16 +299,22 @@ class FileHandle extends EventEmitterMixin(JSTransferable) {
           const { bytesRead } = await readFn(view, view.byteOffset, view.byteLength);
 
           if (bytesRead === 0) {
-            try {
-              controller.close();
-              this[kUnref]();
-            } catch (error) {
-              controller.error(error);
-            }
+            ondone();
+            controller.close();
           }
 
           controller.byobRequest.respond(bytesRead);
         },
+
+        cancel() {
+          ondone();
+        },
+      });
+
+      this[kRef]();
+
+      this.once('close', () => {
+        readableStreamCancel(readable);
       });
     }
 

test/parallel/test-filehandle-readablestream.js

@@ -86,3 +86,87 @@ const check = readFileSync(__filename, { encoding: 'utf8' });
   mc.port1.close();
   await file.close();
 })().then(common.mustCall());
+
+// Make sure 'bytes' stream works
+(async () => {
+  const file = await open(__filename);
+  const dec = new TextDecoder();
+  const readable = file.readableWebStream({ type: 'bytes' });
+  const reader = readable.getReader({ mode: 'byob' });
+
+  let data = '';
+  let result;
+  do {
+    const buff = new ArrayBuffer(100);
+    result = await reader.read(new DataView(buff));
+    if (result.value !== undefined) {
+      data += dec.decode(result.value);
+      assert.ok(result.value.byteLength <= 100);
+    }
+  } while (!result.done);
+
+  assert.strictEqual(check, data);
+
+  assert.throws(() => file.readableWebStream(), {
+    code: 'ERR_INVALID_STATE',
+  });
+
+  await file.close();
+})().then(common.mustCall());
+
+// Make sure that acquiring a ReadableStream 'bytes' stream
+// fails if the FileHandle is already closed.
+(async () => {
+  const file = await open(__filename);
+  await file.close();
+
+  assert.throws(() => file.readableWebStream({ type: 'bytes' }), {
+    code: 'ERR_INVALID_STATE',
+  });
+})().then(common.mustCall());
+
+// Make sure that acquiring a ReadableStream 'bytes' stream
+// fails if the FileHandle is already closing.
+(async () => {
+  const file = await open(__filename);
+  file.close();
+
+  assert.throws(() => file.readableWebStream({ type: 'bytes' }), {
+    code: 'ERR_INVALID_STATE',
+  });
+})().then(common.mustCall());
+
+// Make sure the 'bytes' ReadableStream is closed when the underlying
+// FileHandle is closed.
+(async () => {
+  const file = await open(__filename);
+  const readable = file.readableWebStream({ type: 'bytes' });
+  const reader = readable.getReader({ mode: 'byob' });
+  file.close();
+  await reader.closed;
+})().then(common.mustCall());
+
+// Make sure the 'bytes' ReadableStream is closed when the underlying
+// FileHandle is closed.
+(async () => {
+  const file = await open(__filename);
+  const readable = file.readableWebStream({ type: 'bytes' });
+  file.close();
+  const reader = readable.getReader({ mode: 'byob' });
+  await reader.closed;
+})().then(common.mustCall());
+
+// Make sure that the FileHandle is properly marked "in use"
+// when a 'bytes' ReadableStream has been acquired for it.
+(async () => {
+  const file = await open(__filename);
+  file.readableWebStream({ type: 'bytes' });
+  const mc = new MessageChannel();
+  mc.port1.onmessage = common.mustNotCall();
+  assert.throws(() => mc.port2.postMessage(file, [file]), {
+    code: 25,
+    name: 'DataCloneError',
+  });
+  mc.port1.close();
+  await file.close();
+})().then(common.mustCall());