fs: throw errors from sync branches instead of separate implementations

Previously to throw errors from C++ land, sync versions of the fs
were created by copying C++ code from the original implementation
and moving JS code to a separate file. This can lead to several
problems:

1. By moving code to a new file for the sake of moving, it would
  be harder to use git blame to trace changes and harder to backport
  changes to older branches.
2. Scattering the async and sync versions of fs methods in
  different files makes it harder to keep them in sync and
  share code in the prologues and epilogues.
3. Having two copies of code doing almost the same thing results
  in duplication and can be prone to out-of-sync problems when the
  prologue and epilogue get updated.
4. There is a minor cost to startup in adding an additional file.
  This can add up even with the help of snapshots.

This patch moves the JS code back to lib/fs.js to stop 1, 2 & 4
and introduces C++ helpers SyncCallAndThrowIf() and
SyncCallAndThrowOnError() so that the original implementations
can be easily tweaked to allow throwing from C++ and stop 3.

PR-URL: #49913
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>

lib/fs.js

@@ -141,7 +141,6 @@ const {
   validateObject,
   validateString,
 } = require('internal/validators');
-const syncFs = require('internal/fs/sync');
 
 let truncateWarn = true;
 let fs;
@@ -243,7 +242,10 @@ function access(path, mode, callback) {
  * @returns {void}
  */
 function accessSync(path, mode) {
-  syncFs.access(path, mode);
+  path = getValidatedPath(path);
+  mode = getValidMode(mode, 'access');
+
+  binding.access(pathModule.toNamespacedPath(path), mode);
 }
 
 /**
@@ -285,7 +287,13 @@ ObjectDefineProperty(exists, kCustomPromisifiedSymbol, {
  * @returns {boolean}
  */
 function existsSync(path) {
-  return syncFs.exists(path);
+  try {
+    path = getValidatedPath(path);
+  } catch {
+    return false;
+  }
+
+  return binding.existsSync(pathModule.toNamespacedPath(path));
 }
 
 function readFileAfterOpen(err, fd) {
@@ -438,7 +446,10 @@ function readFileSync(path, options) {
   options = getOptions(options, { flag: 'r' });
 
   if (options.encoding === 'utf8' || options.encoding === 'utf-8') {
-    return syncFs.readFileUtf8(path, options.flag);
+    if (!isInt32(path)) {
+      path = pathModule.toNamespacedPath(getValidatedPath(path));
+    }
+    return binding.readFileUtf8(path, stringToFlags(options.flag));
   }
 
   const isUserFd = isFd(path); // File descriptor ownership
@@ -516,7 +527,9 @@ function close(fd, callback = defaultCloseCallback) {
  * @returns {void}
  */
 function closeSync(fd) {
-  return syncFs.close(fd);
+  fd = getValidatedFd(fd);
+
+  return binding.close(fd);
 }
 
 /**
@@ -562,7 +575,13 @@ function open(path, flags, mode, callback) {
  * @returns {number}
  */
 function openSync(path, flags, mode) {
-  return syncFs.open(path, flags, mode);
+  path = getValidatedPath(path);
+
+  return binding.open(
+    pathModule.toNamespacedPath(path),
+    stringToFlags(flags),
+    parseFileMode(mode, 'mode', 0o666),
+  );
 }
 
 /**
@@ -1667,12 +1686,24 @@ function lstatSync(path, options = { bigint: false, throwIfNoEntry: true }) {
  *   }} [options]
  * @returns {Stats}
  */
-function statSync(path, options) {
-  return syncFs.stat(path, options);
+function statSync(path, options = { bigint: false, throwIfNoEntry: true }) {
+  path = getValidatedPath(path);
+  const stats = binding.stat(
+    pathModule.toNamespacedPath(path),
+    options.bigint,
+    undefined,
+    options.throwIfNoEntry,
+  );
+  if (stats === undefined) {
+    return undefined;
+  }
+  return getStatsFromBinding(stats);
 }
 
-function statfsSync(path, options) {
-  return syncFs.statfs(path, options);
+function statfsSync(path, options = { bigint: false }) {
+  path = getValidatedPath(path);
+  const stats = binding.statfs(pathModule.toNamespacedPath(path), options.bigint);
+  return getStatFsFromBinding(stats);
 }
 
 /**
@@ -1852,7 +1883,8 @@ function unlink(path, callback) {
  * @returns {void}
  */
 function unlinkSync(path) {
-  return syncFs.unlink(path);
+  path = pathModule.toNamespacedPath(getValidatedPath(path));
+  return binding.unlink(path);
 }
 
 /**
@@ -2652,8 +2684,7 @@ function realpathSync(p, options) {
       }
       if (linkTarget === null) {
         const ctx = { path: base };
-        binding.stat(baseLong, false, undefined, ctx);
-        handleErrorFromBinding(ctx);
+        binding.stat(baseLong, false, undefined, true);
         linkTarget = binding.readlink(baseLong, undefined, undefined, ctx);
         handleErrorFromBinding(ctx);
       }
@@ -2948,7 +2979,14 @@ function copyFile(src, dest, mode, callback) {
  * @returns {void}
  */
 function copyFileSync(src, dest, mode) {
-  syncFs.copyFile(src, dest, mode);
+  src = getValidatedPath(src, 'src');
+  dest = getValidatedPath(dest, 'dest');
+
+  binding.copyFile(
+    pathModule.toNamespacedPath(src),
+    pathModule.toNamespacedPath(dest),
+    getValidMode(mode, 'copyFile'),
+  );
 }
 
 /**

src/node_file-inl.h

@@ -349,6 +349,38 @@ int SyncCall(Environment* env, v8::Local<v8::Value> ctx,
   return err;
 }
 
+// Similar to SyncCall but throws immediately if there is an error.
+template <typename Predicate, typename Func, typename... Args>
+int SyncCallAndThrowIf(Predicate should_throw,
+                       Environment* env,
+                       FSReqWrapSync* req_wrap,
+                       Func fn,
+                       Args... args) {
+  env->PrintSyncTrace();
+  int result = fn(nullptr, &(req_wrap->req), args..., nullptr);
+  if (should_throw(result)) {
+    env->ThrowUVException(result,
+                          req_wrap->syscall_p,
+                          nullptr,
+                          req_wrap->path_p,
+                          req_wrap->dest_p);
+  }
+  return result;
+}
+
+constexpr bool is_uv_error(int result) {
+  return result < 0;
+}
+
+// Similar to SyncCall but throws immediately if there is an error.
+template <typename Func, typename... Args>
+int SyncCallAndThrowOnError(Environment* env,
+                            FSReqWrapSync* req_wrap,
+                            Func fn,
+                            Args... args) {
+  return SyncCallAndThrowIf(is_uv_error, env, req_wrap, fn, args...);
+}
+
 }  // namespace fs
 }  // namespace node