From ad3a583bc45fceb8d2f08160b7c10727e9a891c9 Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Sun, 1 Oct 2023 21:59:52 -0400
Subject: [PATCH 1/2] fs: add flush option to writeFile() functions

This commit adds a 'flush' option to the fs.writeFile family of
functions.

Refs: https://github.com/nodejs/node/issues/49886
---
 doc/api/fs.md                             |  18 +++-
 lib/fs.js                                 |  59 +++++++++--
 lib/internal/fs/promises.js               |  31 +++++-
 test/parallel/test-fs-write-file-flush.js | 114 ++++++++++++++++++++++
 4 files changed, 209 insertions(+), 13 deletions(-)
 create mode 100644 test/parallel/test-fs-write-file-flush.js

diff --git a/doc/api/fs.md b/doc/api/fs.md
index 34d10418b2acc8..7d170613a7baf8 100644
--- a/doc/api/fs.md
+++ b/doc/api/fs.md
@@ -1742,6 +1742,9 @@ All the [caveats][] for `fs.watch()` also apply to `fsPromises.watch()`.
 <!-- YAML
 added: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/50009
+    description: The `flush` option is now supported.
   - version:
       - v15.14.0
       - v14.18.0
@@ -1765,6 +1768,9 @@ changes:
   * `encoding` {string|null} **Default:** `'utf8'`
   * `mode` {integer} **Default:** `0o666`
   * `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
+  * `flush` {boolean} If all data is successfully written to the file, and
+    `flush` is `true`, `filehandle.sync()` is used to flush the data.
+    **Default:** `false`.
   * `signal` {AbortSignal} allows aborting an in-progress writeFile
 * Returns: {Promise} Fulfills with `undefined` upon success.
 
@@ -4879,6 +4885,9 @@ details.
 <!-- YAML
 added: v0.1.29
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/50009
+    description: The `flush` option is now supported.
   - version: v19.0.0
     pr-url: https://github.com/nodejs/node/pull/42796
     description: Passing to the `string` parameter an object with an own
@@ -4936,6 +4945,9 @@ changes:
   * `encoding` {string|null} **Default:** `'utf8'`
   * `mode` {integer} **Default:** `0o666`
   * `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
+  * `flush` {boolean} If all data is successfully written to the file, and
+    `flush` is `true`, `fs.fsync()` is used to flush the data.
+    **Default:** `false`.
   * `signal` {AbortSignal} allows aborting an in-progress writeFile
 * `callback` {Function}
   * `err` {Error|AggregateError}
@@ -6167,6 +6179,9 @@ this API: [`fs.utimes()`][].
 <!-- YAML
 added: v0.1.29
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/50009
+    description: The `flush` option is now supported.
   - version: v19.0.0
     pr-url: https://github.com/nodejs/node/pull/42796
     description: Passing to the `data` parameter an object with an own
@@ -6201,7 +6216,8 @@ changes:
   * `encoding` {string|null} **Default:** `'utf8'`
   * `mode` {integer} **Default:** `0o666`
   * `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
-
+  * `flush` {boolean} If all data is successfully written to the file, and
+    `flush` is `true`, `fs.fsyncSync()` is used to flush the data.
 Returns `undefined`.
 
 The `mode` option only affects the newly created file. See [`fs.open()`][]
diff --git a/lib/fs.js b/lib/fs.js
index d682759d8b0a14..3bf1022d2f5e8f 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -2217,7 +2217,7 @@ function lutimesSync(path, atime, mtime) {
   handleErrorFromBinding(ctx);
 }
 
-function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
+function writeAll(fd, isUserFd, buffer, offset, length, signal, flush, callback) {
   if (signal?.aborted) {
     const abortError = new AbortError(undefined, { cause: signal?.reason });
     if (isUserFd) {
@@ -2240,15 +2240,33 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
         });
       }
     } else if (written === length) {
-      if (isUserFd) {
-        callback(null);
+      if (!flush) {
+        if (isUserFd) {
+          callback(null);
+        } else {
+          fs.close(fd, callback);
+        }
       } else {
-        fs.close(fd, callback);
+        fs.fsync(fd, (syncErr) => {
+          if (syncErr) {
+            if (isUserFd) {
+              callback(syncErr);
+            } else {
+              fs.close(fd, (err) => {
+                callback(aggregateTwoErrors(err, syncErr));
+              });
+            }
+          } else if (isUserFd) {
+            callback(null);
+          } else {
+            fs.close(fd, callback);
+          }
+        });
       }
     } else {
       offset += written;
       length -= written;
-      writeAll(fd, isUserFd, buffer, offset, length, signal, callback);
+      writeAll(fd, isUserFd, buffer, offset, length, signal, flush, callback);
     }
   });
 }
@@ -2262,14 +2280,23 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
  *   mode?: number;
  *   flag?: string;
  *   signal?: AbortSignal;
+ *   flush?: boolean;
  *   } | string} [options]
  * @param {(err?: Error) => any} callback
  * @returns {void}
  */
 function writeFile(path, data, options, callback) {
   callback = maybeCallback(callback || options);
-  options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
+  options = getOptions(options, {
+    encoding: 'utf8',
+    mode: 0o666,
+    flag: 'w',
+    flush: false,
+  });
   const flag = options.flag || 'w';
+  const flush = options.flush ?? false;
+
+  validateBoolean(flush, 'options.flush');
 
   if (!isArrayBufferView(data)) {
     validateStringAfterArrayBufferView(data, 'data');
@@ -2279,7 +2306,7 @@ function writeFile(path, data, options, callback) {
   if (isFd(path)) {
     const isUserFd = true;
     const signal = options.signal;
-    writeAll(path, isUserFd, data, 0, data.byteLength, signal, callback);
+    writeAll(path, isUserFd, data, 0, data.byteLength, signal, flush, callback);
     return;
   }
 
@@ -2292,7 +2319,7 @@ function writeFile(path, data, options, callback) {
     } else {
       const isUserFd = false;
       const signal = options.signal;
-      writeAll(fd, isUserFd, data, 0, data.byteLength, signal, callback);
+      writeAll(fd, isUserFd, data, 0, data.byteLength, signal, flush, callback);
     }
   });
 }
@@ -2305,11 +2332,21 @@ function writeFile(path, data, options, callback) {
  *   encoding?: string | null;
  *   mode?: number;
  *   flag?: string;
+ *   flush?: boolean;
  *   } | string} [options]
  * @returns {void}
  */
 function writeFileSync(path, data, options) {
-  options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
+  options = getOptions(options, {
+    encoding: 'utf8',
+    mode: 0o666,
+    flag: 'w',
+    flush: false,
+  });
+
+  const flush = options.flush ?? false;
+
+  validateBoolean(flush, 'options.flush');
 
   if (!isArrayBufferView(data)) {
     validateStringAfterArrayBufferView(data, 'data');
@@ -2329,6 +2366,10 @@ function writeFileSync(path, data, options) {
       offset += written;
       length -= written;
     }
+
+    if (flush) {
+      fs.fsyncSync(fd);
+    }
   } finally {
     if (!isUserFd) fs.closeSync(fd);
   }
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
index a656ca411584b2..eda7b4bb21e9ed 100644
--- a/lib/internal/fs/promises.js
+++ b/lib/internal/fs/promises.js
@@ -413,6 +413,18 @@ async function handleFdClose(fileOpPromise, closeFunc) {
   );
 }
 
+async function handleFdSync(fileOpPromise, handle) {
+  return PromisePrototypeThen(
+    fileOpPromise,
+    (result) => PromisePrototypeThen(
+      handle.sync(),
+      () => result,
+      (syncError) => PromiseReject(syncError),
+    ),
+    (opError) => PromiseReject(opError),
+  );
+}
+
 async function fsCall(fn, handle, ...args) {
   assert(handle[kRefs] !== undefined,
          'handle must be an instance of FileHandle');
@@ -1006,8 +1018,16 @@ async function mkdtemp(prefix, options) {
 }
 
 async function writeFile(path, data, options) {
-  options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
+  options = getOptions(options, {
+    encoding: 'utf8',
+    mode: 0o666,
+    flag: 'w',
+    flush: false,
+  });
   const flag = options.flag || 'w';
+  const flush = options.flush ?? false;
+
+  validateBoolean(flush, 'options.flush');
 
   if (!isArrayBufferView(data) && !isCustomIterable(data)) {
     validateStringAfterArrayBufferView(data, 'data');
@@ -1021,8 +1041,13 @@ async function writeFile(path, data, options) {
   checkAborted(options.signal);
 
   const fd = await open(path, flag, options.mode);
-  return handleFdClose(
-    writeFileHandle(fd, data, options.signal, options.encoding), fd.close);
+  let writeOp = writeFileHandle(fd, data, options.signal, options.encoding);
+
+  if (flush) {
+    writeOp = handleFdSync(writeOp, fd);
+  }
+
+  return handleFdClose(writeOp, fd.close);
 }
 
 function isCustomIterable(obj) {
diff --git a/test/parallel/test-fs-write-file-flush.js b/test/parallel/test-fs-write-file-flush.js
new file mode 100644
index 00000000000000..98a8d637c5fa28
--- /dev/null
+++ b/test/parallel/test-fs-write-file-flush.js
@@ -0,0 +1,114 @@
+'use strict';
+const common = require('../common');
+const tmpdir = require('../common/tmpdir');
+const assert = require('node:assert');
+const fs = require('node:fs');
+const fsp = require('node:fs/promises');
+const test = require('node:test');
+const data = 'foo';
+let cnt = 0;
+
+function nextFile() {
+  return tmpdir.resolve(`${cnt++}.out`);
+}
+
+tmpdir.refresh();
+
+test('synchronous version', async (t) => {
+  await t.test('validation', (t) => {
+    for (const v of ['true', '', 0, 1, [], {}, Symbol()]) {
+      assert.throws(() => {
+        fs.writeFileSync(nextFile(), data, { flush: v });
+      }, { code: 'ERR_INVALID_ARG_TYPE' });
+    }
+  });
+
+  await t.test('performs flush', (t) => {
+    const spy = t.mock.method(fs, 'fsyncSync');
+    const file = nextFile();
+    fs.writeFileSync(file, data, { flush: true });
+    const calls = spy.mock.calls;
+    assert.strictEqual(calls.length, 1);
+    assert.strictEqual(calls[0].result, undefined);
+    assert.strictEqual(calls[0].error, undefined);
+    assert.strictEqual(calls[0].arguments.length, 1);
+    assert.strictEqual(typeof calls[0].arguments[0], 'number');
+    assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
+  });
+
+  await t.test('does not perform flush', (t) => {
+    const spy = t.mock.method(fs, 'fsyncSync');
+
+    for (const v of [undefined, null, false]) {
+      const file = nextFile();
+      fs.writeFileSync(file, data, { flush: v });
+      assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
+    }
+
+    assert.strictEqual(spy.mock.calls.length, 0);
+  });
+});
+
+test('callback version', async (t) => {
+  await t.test('validation', (t) => {
+    for (const v of ['true', '', 0, 1, [], {}, Symbol()]) {
+      assert.throws(() => {
+        fs.writeFileSync(nextFile(), data, { flush: v });
+      }, { code: 'ERR_INVALID_ARG_TYPE' });
+    }
+  });
+
+  await t.test('performs flush', (t, done) => {
+    const spy = t.mock.method(fs, 'fsync');
+    const file = nextFile();
+    fs.writeFile(file, data, { flush: true }, common.mustSucceed(() => {
+      const calls = spy.mock.calls;
+      assert.strictEqual(calls.length, 1);
+      assert.strictEqual(calls[0].result, undefined);
+      assert.strictEqual(calls[0].error, undefined);
+      assert.strictEqual(calls[0].arguments.length, 2);
+      assert.strictEqual(typeof calls[0].arguments[0], 'number');
+      assert.strictEqual(typeof calls[0].arguments[1], 'function');
+      assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
+      done();
+    }));
+  });
+
+  await t.test('does not perform flush', (t, done) => {
+    const values = [undefined, null, false];
+    const spy = t.mock.method(fs, 'fsync');
+    let cnt = 0;
+
+    for (const v of values) {
+      const file = nextFile();
+
+      fs.writeFile(file, data, { flush: v }, common.mustSucceed(() => {
+        assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
+        cnt++;
+
+        if (cnt === values.length) {
+          assert.strictEqual(spy.mock.calls.length, 0);
+          done();
+        }
+      }));
+    }
+  });
+});
+
+test('promise based version', async (t) => {
+  await t.test('validation', async (t) => {
+    for (const v of ['true', '', 0, 1, [], {}, Symbol()]) {
+      await assert.rejects(() => {
+        return fsp.writeFile(nextFile(), data, { flush: v });
+      }, { code: 'ERR_INVALID_ARG_TYPE' });
+    }
+  });
+
+  await t.test('success path', async (t) => {
+    for (const v of [undefined, null, false, true]) {
+      const file = nextFile();
+      await fsp.writeFile(file, data, { flush: v });
+      assert.strictEqual(await fsp.readFile(file, 'utf8'), data);
+    }
+  });
+});

From 78ab583dde570d1aec5341cc3e0d597b09f3413a Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Mon, 2 Oct 2023 23:37:01 -0400
Subject: [PATCH 2/2] run `make format-md`

---
 doc/api/fs.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/api/fs.md b/doc/api/fs.md
index 7d170613a7baf8..76f6432212654f 100644
--- a/doc/api/fs.md
+++ b/doc/api/fs.md
@@ -6218,7 +6218,7 @@ changes:
   * `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
   * `flush` {boolean} If all data is successfully written to the file, and
     `flush` is `true`, `fs.fsyncSync()` is used to flush the data.
-Returns `undefined`.
+    Returns `undefined`.
 
 The `mode` option only affects the newly created file. See [`fs.open()`][]
 for more details.