node icon indicating copy to clipboard operation
node copied to clipboard

fs: add support for async iterators to `fs.writeFile`

Open yagipy opened this issue 4 years ago • 4 comments

Fixes: https://github.com/nodejs/node/issues/38075

Checklist

  • [ ] make -j4 test (UNIX), or vcbuild test (Windows) passes
  • [x] tests and/or benchmarks are included
  • [x] documentation is changed or added
  • [x] commit message follows commit guidelines

yagipy avatar May 03 '21 23:05 yagipy

@Linkgoron I am currently working on the following response. Are the following mistakes feasible in testing? https://github.com/nodejs/node/pull/38525#discussion_r628760262

I added the following test based on this test, but I was not able to reproduce the mistake. 😓 (The current implementation will pass.)

{
  const filenameLargeBuffer = join(tmpdir.path, 'testLargeBuffer.txt');
  const largeBuffer = {
    expected: 'dogs running'.repeat(512 * 1024),
    *[Symbol.iterator]() {
      yield Buffer.from('dogs running'.repeat(512 * 1024), 'utf8');
    }
  };

  fs.writeFile(
    filenameLargeBuffer, largeBuffer, common.mustSucceed(() => {
      const data = fs.readFileSync(filenameLargeBuffer, 'utf-8');
      assert.strictEqual(largeBuffer.expected, data);
    })
  );
}

yagipy avatar May 15 '21 10:05 yagipy

@Linkgoron I am currently working on the following response. Are the following mistakes feasible in testing? #38525 (comment)

I added the following test based on this test, but I was not able to reproduce the mistake. 😓 (The current implementation will pass.)

{
  const filenameLargeBuffer = join(tmpdir.path, 'testLargeBuffer.txt');
  const largeBuffer = {
    expected: 'dogs running'.repeat(512 * 1024),
    *[Symbol.iterator]() {
      yield Buffer.from('dogs running'.repeat(512 * 1024), 'utf8');
    }
  };

  fs.writeFile(
    filenameLargeBuffer, largeBuffer, common.mustSucceed(() => {
      const data = fs.readFileSync(filenameLargeBuffer, 'utf-8');
      assert.strictEqual(largeBuffer.expected, data);
    })
  );
}

It's quite difficult to reproduce, and I'm not sure in which cases write actually stops in the middle.

Linkgoron avatar May 16 '21 14:05 Linkgoron

@Linkgoron Fixed an issue where chunks might be partially written, referring to https://github.com/nodejs/node/pull/38615.

yagipy avatar May 22 '21 22:05 yagipy

@ronag @mcollina ... would definitely appreciate you taking a look at this one.

jasnell avatar May 24 '21 14:05 jasnell