Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fileHandle.read argument defaults don’t seem to work #39034

Closed
lydell opened this issue Jun 14, 2021 · 4 comments
Closed

fileHandle.read argument defaults don’t seem to work #39034

lydell opened this issue Jun 14, 2021 · 4 comments
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.

Comments

@lydell
Copy link

lydell commented Jun 14, 2021

  • Version: v16.1.0
  • Platform: Darwin 20.5.0 x86_64
  • Subsystem: fs/promises

What steps will reproduce the bug?

const fs = require("fs");
const fsPromises = require("fs/promises");

async function run() {
  const fileHandle = await fsPromises.open(__filename);
  const buffer = Buffer.alloc(16);

  // Logs `{ bytesRead: 0, buffer: ... }`:
  console.log(await fileHandle.read(buffer));

  // Logs `{ bytesRead: 16, buffer: ... }`:
  console.log(await fileHandle.read(buffer, 0, buffer.byteLength));

  // According to the docs, the first call should read 16 bytes too, because
  // the parameters left out default to the values in the second call.
  // `fs.readSync` works that way:

  // Logs `16`.
  console.log(fs.readSync(fs.openSync(__filename), buffer));
}

run().catch(console.error);

Save the above as test.cjs and run node test.cjs.

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior?

https://nodejs.org/api/fs.html#fs_filehandle_read_buffer_offset_length_position

filehandle.read(buffer, offset, length, position)

  • buffer <Buffer> | <TypedArray> | <DataView> A buffer that will be filled with the file data read.
  • offset <integer> The location in the buffer at which to start filling. Default: 0
  • length <integer> The number of bytes to read. Default: buffer.byteLength
  • position <integer> The location where to begin reading data from the file. If null, data will be read from the current file position, and the position will be updated. If position is an integer, the current file position will remain unchanged.

Calling fileHandle.read(buffer) should work like fileHandle.read(buffer, 0, buffer.byteLength).

What do you see instead?

0 bytes are read.

Additional information

fs.readSync has similar documentation and works as expected.

Potential problems:

  • I’m misunderstanding the docs.
  • The docs are wrong.
  • There’s a bug.
@Linkgoron
Copy link
Member

Linkgoron commented Jun 14, 2021

This is IMO an issue with the docs. The readSync call works as the overload that's called is actually as if you called the [options] overload, as buffer is not part of the options in readSync.

For fileHandle.read the parameters are not marked as optional, and length actually defaults to 0 when it's not specifically provided. The change to the docs is a relatively recent change, and I believe that it was done by mistake and the defaults accidentally copied from the options overload. The docs for the callback version are wrong as well, in a similar way.

@Ayase-252 Ayase-252 added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors. labels Jun 18, 2021
@rbrishabh
Copy link
Contributor

I can help with this!

@Nazeeh21
Copy link

I want to work on this issue.

BethGriggs pushed a commit that referenced this issue Sep 21, 2021
PR-URL: #39163
Fixes: #39034
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Qingyu Deng <i@ayase-lab.com>
BethGriggs pushed a commit that referenced this issue Sep 21, 2021
PR-URL: #39163
Fixes: #39034
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Qingyu Deng <i@ayase-lab.com>
@dasdeo
Copy link

dasdeo commented Feb 29, 2024

Unless I'm missing something here, the docs still show the wrong description:

Screenshot 2024-02-29 at 18 44 39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants