fs: add flush option to writeFile() functions

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

Refs: #49886
PR-URL: #50009
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>

Author: cjihrig

doc/api/fs.md

@@ -1718,6 +1718,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
@@ -1741,6 +1744,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.
 
@@ -4849,6 +4855,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
@@ -4906,6 +4915,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}
@@ -6131,6 +6143,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
@@ -6165,8 +6180,9 @@ changes:
   * `encoding` {string|null} **Default:** `'utf8'`
   * `mode` {integer} **Default:** `0o666`
   * `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
-
-Returns `undefined`.
+  * `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()`][]
 for more details.

lib/fs.js

@@ -2214,7 +2214,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) {
@@ -2237,15 +2237,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);
     }
   });
 }
@@ -2259,14 +2277,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');
@@ -2276,7 +2303,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;
   }
 
@@ -2289,7 +2316,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);
     }
   });
 }
@@ -2302,11 +2329,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');
@@ -2326,6 +2363,10 @@ function writeFileSync(path, data, options) {
       offset += written;
       length -= written;
     }
+
+    if (flush) {
+      fs.fsyncSync(fd);
+    }
   } finally {
     if (!isUserFd) fs.closeSync(fd);
   }

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');
@@ -1003,8 +1015,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');
@@ -1018,8 +1038,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) {

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);
+    }
+  });
+});