doc: fix filehandle.truncate sample codes

[shisama]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• documentation is changed or added
• commit message follows commit guidelines

[jasnell]

these should really be....

let filehandle;
try {
  const filehandle = await fsPromises.open('temp.txt', 'r+');
  await filehandle.truncate(4);
} finally {
  await filehandle.close();
}

While the filehandle will close automatically on garbage collection in order to prevent the leak, it should be closed manually. A process warning would be emitted if it is allowed to close on gc.

[benjamingr]

@jasnell sorry for the tangent: we really need a better resources story in general - if this is tricky consider how hard this is with 3 concurrent handles.

[benjamingr]

One thing that might work here API wise is a disposer:

await fsPromises.open('temp.txt', 'r+', async handle => {
  await handle.truncate(4);
});

This doesn't solve the harder problem of dealing with multiple resources at once though.

[jasnell]

I certainly don't disagree :-)

[shisama]

@jasnell
Thank you for your comment. The codes were fixed with filehandle.close. Please check them again.

[benjamingr]

@jasnell - what do you think the right avenue to discuss such a cross-cutting concern is? I'm not sure what the right team/working-group/avenue is.

I'm a little lost here :)

[benjamingr]

Collaborators - please 👍 if you agree with fast-tracking this.

[vsemozhetbyt]

What can make undefined filehandle be null?

[benjamingr]

The open throwing, since that was unclear it'll definitely be unclear for some users - we should probably add a comment.

[vsemozhetbyt]

If open throws, will not the filehandle remain undefined?

[benjamingr]

I could have sworn that was a != and not a !==, good spotting!

[vsemozhetbyt]

I mean, shoud not we initialize filehandle as null or use non-strict comparison or even if (filehandle) ?

[benjamingr]

We should. I was saying "good spotting" for spotting that.

[vsemozhetbyt]

Sorry, GitHub did not update timely with new comments)

[shisama]

@vsemozhetbyt @benjamingr
filehandle is not undefined. It is defined and initialized as null.

let filehandle;

But it is unclear for some users.
The below code may be more clearly for users. It looks better?

async function doTruncate() {
  let filehandle = null;
  try {
    filehandle = await fsPromises.open('temp.txt', 'r+');
    await filehandle.truncate(4);
  } finally {
    if (filehandle) {
      // close the file if it is opened.
      await filehandle.close();
    }
  }
  console.log(fs.readFileSync('temp.txt', 'utf8'));  // Prints: Node
}

[benjamingr]

let filehandle;

When you do that in JavaScript that initializes the value to undefined. You can verify this in the following script:

let filehandle;
console.log(filehandle);

The below code may be more clearly for users. It looks better?

Yes, thank you.

[shisama]

@benjamingr
oh, sorry! I was wrong and fixed them.

[vsemozhetbyt]

Nit: unneeded empty line.

[shisama]

@vsemozhetbyt Fixed it. Thanks.

[vsemozhetbyt]

CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite/749/ (one unrelated fail of a flaky test).

[Trott]

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite/751/

[vsemozhetbyt]

Landed in ac82261
Thank you!