fs: throw fs.access errors in JS

- Migrate the type check of path to ERR_INVALID_ARG_TYPE
- Add template counterparts of ASYNC_CALL, ASYNC_DEST_CALL,
  SYNC_CALL, SYNC_DEST_CALL
- Port StringFromPath and UVException to JavaScript
- Migrate the access binding to collect the error context in C++,
  then throw the error in JS

PR-URL: https://github.com/nodejs/node/pull/17160
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/fs.js

@@ -297,6 +297,11 @@ fs.access = function(path, mode, callback) {
   if (handleError((path = getPathFromURL(path)), callback))
     return;
 
+  if (typeof path !== 'string' && !(path instanceof Buffer)) {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'path',
+                               ['string', 'Buffer', 'URL']);
+  }
+
   if (!nullCheck(path, callback))
     return;
 
@@ -308,6 +313,12 @@ fs.access = function(path, mode, callback) {
 
 fs.accessSync = function(path, mode) {
   handleError((path = getPathFromURL(path)));
+
+  if (typeof path !== 'string' && !(path instanceof Buffer)) {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'path',
+                               ['string', 'Buffer', 'URL']);
+  }
+
   nullCheck(path);
 
   if (mode === undefined)
@@ -315,7 +326,12 @@ fs.accessSync = function(path, mode) {
   else
     mode = mode | 0;
 
-  binding.access(pathModule.toNamespacedPath(path), mode);
+  const ctx = {};
+  binding.access(pathModule.toNamespacedPath(path), mode, undefined, ctx);
+
+  if (ctx.code !== undefined) {
+    throw new errors.uvException(ctx);
+  }
 };
 
 fs.exists = function(path, callback) {

lib/internal/errors.js

@@ -194,7 +194,49 @@ function E(sym, val) {
   messages.set(sym, typeof val === 'function' ? val : String(val));
 }
 
+// JS counterpart of StringFromPath, although here path is a buffer.
+function stringFromPath(path) {
+  const str = path.toString();
+  if (process.platform !== 'win32') {
+    return str;
+  }
+
+  if (str.startsWith('\\\\?\\UNC\\')) {
+    return '\\\\' + str.slice(8);
+  } else if (str.startsWith('\\\\?\\')) {
+    return str.slice(4);
+  }
+  return str;
+}
+
+// This creates an error compatible with errors produced in UVException
+// using the context collected in CollectUVExceptionInfo
+// The goal is to migrate them to ERR_* errors later when
+// compatibility is not a concern
+function uvException(ctx) {
+  const err = new Error();
+  err.errno = ctx.errno;
+  err.code = ctx.code;
+  err.syscall = ctx.syscall;
+
+  let message = `${ctx.code}: ${ctx.message}, ${ctx.syscall}`;
+  if (ctx.path) {
+    const path = stringFromPath(ctx.path);
+    message += ` '${path}'`;
+    err.path = path;
+  }
+  if (ctx.dest) {
+    const dest = stringFromPath(ctx.dest);
+    message += ` -> '${dest}'`;
+    err.dest = dest;
+  }
+  err.message = message;
+  Error.captureStackTrace(err, uvException);
+  return err;
+}
+
 module.exports = exports = {
+  uvException,
   message,
   Error: makeNodeError(Error),
   TypeError: makeNodeError(TypeError),

src/node_file.cc

@@ -349,6 +349,31 @@ class fs_req_wrap {
   DISALLOW_COPY_AND_ASSIGN(fs_req_wrap);
 };
 
+// Template counterpart of ASYNC_DEST_CALL
+template <typename Func, typename... Args>
+inline FSReqWrap* AsyncDestCall(Environment* env, Local<Object> req,
+    const char* dest, enum encoding enc, const char* syscall,
+    Func fn, Args... args) {
+  FSReqWrap* req_wrap = FSReqWrap::New(env, req, syscall, dest, enc);
+  int err = fn(env->event_loop(), req_wrap->req(), args..., After);
+  req_wrap->Dispatched();
+  if (err < 0) {
+    uv_fs_t* uv_req = req_wrap->req();
+    uv_req->result = err;
+    uv_req->path = nullptr;
+    After(uv_req);
+    req_wrap = nullptr;
+  }
+
+  return req_wrap;
+}
+
+// Template counterpart of ASYNC_CALL
+template <typename Func, typename... Args>
+inline FSReqWrap* AsyncCall(Environment* env, Local<Object> req,
+    enum encoding enc, const char* syscall, Func fn, Args... args) {
+  return AsyncDestCall(env, req, nullptr, enc, syscall, fn, args...);
+}
 
 #define ASYNC_DEST_CALL(func, request, dest, encoding, ...)                   \
   Environment* env = Environment::GetCurrent(args);                           \
@@ -373,6 +398,28 @@ class fs_req_wrap {
 #define ASYNC_CALL(func, req, encoding, ...)                                  \
   ASYNC_DEST_CALL(func, req, nullptr, encoding, __VA_ARGS__)                  \
 
+// Template counterpart of SYNC_DEST_CALL
+template <typename Func, typename... Args>
+inline void SyncDestCall(Environment* env, Local<Value> ctx,
+    const char* path, const char* dest, const char* syscall,
+    Func fn, Args... args) {
+  fs_req_wrap req_wrap;
+  env->PrintSyncTrace();
+  int err = fn(env->event_loop(), &req_wrap.req, args..., nullptr);
+  if (err) {
+    Local<Context> context = env->context();
+    Local<Object> ctx_obj = ctx->ToObject(context).ToLocalChecked();
+    env->CollectUVExceptionInfo(ctx_obj, err, syscall, nullptr, path, dest);
+  }
+}
+
+// Template counterpart of SYNC_CALL
+template <typename Func, typename... Args>
+inline void SyncCall(Environment* env, Local<Value> ctx,
+    const char* path, const char* syscall, Func fn, Args... args) {
+  return SyncDestCall(env, ctx, path, nullptr, syscall, fn, args...);
+}
+
 #define SYNC_DEST_CALL(func, path, dest, ...)                                 \
   fs_req_wrap req_wrap;                                                       \
   env->PrintSyncTrace();                                                      \
@@ -394,21 +441,22 @@ class fs_req_wrap {
 void Access(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args.GetIsolate());
   HandleScope scope(env->isolate());
-
+  Local<Context> context = env->context();
-  if (args.Length() < 2)
+  CHECK_GE(args.Length(), 2);
-    return TYPE_ERROR("path and mode are required");
+  CHECK(args[1]->IsInt32());
-  if (!args[1]->IsInt32())
-    return TYPE_ERROR("mode must be an integer");
 
   BufferValue path(env->isolate(), args[0]);
-  ASSERT_PATH(path)
+  int mode = static_cast<int>(args[1]->Int32Value(context).FromJust());
-
-  int mode = static_cast<int>(args[1]->Int32Value());
 
   if (args[2]->IsObject()) {
-    ASYNC_CALL(access, args[2], UTF8, *path, mode);
+    Local<Object> req_obj = args[2]->ToObject(context).ToLocalChecked();
+    FSReqWrap* req_wrap = AsyncCall(
+        env, req_obj, UTF8, "access", uv_fs_access, *path, mode);
+    if (req_wrap != nullptr) {
+      args.GetReturnValue().Set(req_wrap->persistent());
+    }
   } else {
-    SYNC_CALL(access, *path, *path, mode);
+    SyncCall(env, args[3], *path, "access", uv_fs_access, *path, mode);
   }
 }
 

test/parallel/test-fs-access.js

@@ -81,9 +81,16 @@ fs.access(readOnlyFile, fs.W_OK, common.mustCall((err) => {
   }
 }));
 
-assert.throws(() => {
+common.expectsError(
-  fs.access(100, fs.F_OK, common.mustNotCall());
+  () => {
-}, /^TypeError: path must be a string or Buffer$/);
+    fs.access(100, fs.F_OK, common.mustNotCall());
+  },
+  {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError,
+    message: 'The "path" argument must be one of type string, Buffer, or URL'
+  }
+);
 
 common.expectsError(
   () => {

test/parallel/test-fs-buffer.js

@@ -25,9 +25,16 @@ assert.doesNotThrow(() => {
   }));
 });
 
-assert.throws(() => {
+common.expectsError(
-  fs.accessSync(true);
+  () => {
-}, /path must be a string or Buffer/);
+    fs.accessSync(true);
+  },
+  {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError,
+    message: 'The "path" argument must be one of type string, Buffer, or URL'
+  }
+);
 
 const dir = Buffer.from(fixtures.fixturesDir);
 fs.readdir(dir, 'hex', common.mustCall((err, hexList) => {