fs: throw errors on invalid paths synchronously

- Throw getPathFromURL() and nullCheck() errors synchronously instead
  of deferring them to the next tick, since we already throw
  validatePath() errors synchronously.
- Merge nullCheck() into validatePath()
- Never throws in `fs.exists()`, instead, invoke the callback with
  false, or emit a warning when the callback is not a function.
  This is to bring it inline with fs.existsSync(), which never throws.
- Updates the comment of rethrow()
- Throw ERR_INVALID_ARG_VALUE for null checks

PR-URL: https://github.com/nodejs/node/pull/18308
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/fs.js

@@ -108,8 +108,9 @@ function copyObject(source) {
   return target;
 }
 
+// TODO(joyeecheung): explore how the deprecation could be solved via linting
+// rules. See https://github.com/nodejs/node/pull/12976
 function rethrow() {
-  // TODO(thefourtheye) Throw error instead of warning in major version > 7
   process.emitWarning(
     'Calling an asynchronous function without callback is deprecated.',
     'DeprecationWarning', 'DEP0013', rethrow
@@ -207,6 +208,34 @@ function validateOffsetLengthWrite(offset, length, byteLength) {
   }
 }
 
+// Check if the path contains null types if it is a string nor Uint8Array,
+// otherwise return silently.
+function nullCheck(path, propName, throwError = true) {
+  const pathIsString = typeof path === 'string';
+  const pathIsUint8Array = isUint8Array(path);
+
+  // We can only perform meaningful checks on strings and Uint8Arrays.
+  if (!pathIsString && !pathIsUint8Array) {
+    return;
+  }
+
+  if (pathIsString && path.indexOf('\u0000') === -1) {
+    return;
+  } else if (pathIsUint8Array && path.indexOf(0) === -1) {
+    return;
+  }
+
+  const err = new errors.Error(
+    'ERR_INVALID_ARG_VALUE', propName, path,
+    'must be a string or Uint8Array without null bytes');
+
+  if (throwError) {
+    Error.captureStackTrace(err, nullCheck);
+    throw err;
+  }
+  return err;
+}
+
 function validatePath(path, propName) {
   let err;
 
@@ -217,6 +246,8 @@ function validatePath(path, propName) {
   if (typeof path !== 'string' && !isUint8Array(path)) {
     err = new errors.TypeError('ERR_INVALID_ARG_TYPE', propName,
                                ['string', 'Buffer', 'URL']);
+  } else {
+    err = nullCheck(path, propName, false);
   }
 
   if (err !== undefined) {
@@ -255,21 +286,6 @@ function makeStatsCallback(cb) {
   };
 }
 
-function nullCheck(path, callback) {
-  if (('' + path).indexOf('\u0000') !== -1) {
-    const er = new errors.Error('ERR_INVALID_ARG_TYPE',
-                                'path',
-                                'string without null bytes',
-                                path);
-
-    if (typeof callback !== 'function')
-      throw er;
-    process.nextTick(callback, er);
-    return false;
-  }
-  return true;
-}
-
 function isFd(path) {
   return (path >>> 0) === path;
 }
@@ -361,16 +377,6 @@ Object.defineProperties(fs, {
   X_OK: { enumerable: true, value: constants.X_OK || 0 },
 });
 
-function handleError(val, callback) {
-  if (val instanceof Error) {
-    if (typeof callback === 'function') {
-      process.nextTick(callback, val);
-      return true;
-    } else throw val;
-  }
-  return false;
-}
-
 fs.access = function(path, mode, callback) {
   if (typeof mode === 'function') {
     callback = mode;
@@ -379,14 +385,9 @@ fs.access = function(path, mode, callback) {
     throw new errors.TypeError('ERR_INVALID_CALLBACK');
   }
 
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-
   validatePath(path);
 
-  if (!nullCheck(path, callback))
-    return;
-
   mode = mode | 0;
   var req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
@@ -394,9 +395,8 @@ fs.access = function(path, mode, callback) {
 };
 
 fs.accessSync = function(path, mode) {
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
   validatePath(path);
-  nullCheck(path);
 
   if (mode === undefined)
     mode = fs.F_OK;
@@ -411,17 +411,30 @@ fs.accessSync = function(path, mode) {
   }
 };
 
+// fs.exists never throws even when the arguments are invalid - if there is
+// a callback it would invoke it with false, otherwise it emits a warning
+// (see the comments of rethrow()).
+// This is to bring it inline with fs.existsSync, which never throws.
+// TODO(joyeecheung): deprecate the never-throw-on-invalid-arguments behavior
 fs.exists = function(path, callback) {
-  if (handleError((path = getPathFromURL(path)), cb))
+  if (typeof callback !== 'function') {
+    rethrow();
     return;
-  validatePath(path);
-  if (!nullCheck(path, cb)) return;
-  var req = new FSReqWrap();
-  req.oncomplete = cb;
-  binding.stat(pathModule.toNamespacedPath(path), req);
-  function cb(err) {
-    if (callback) callback(err ? false : true);
   }
+
+  function suppressedCallback(err) {
+    callback(err ? false : true);
+  }
+
+  try {
+    path = getPathFromURL(path);
+    validatePath(path);
+  } catch (err) {
+    return callback(false);
+  }
+  var req = new FSReqWrap();
+  req.oncomplete = suppressedCallback;
+  binding.stat(pathModule.toNamespacedPath(path), req);
 };
 
 Object.defineProperty(fs.exists, internalUtil.promisify.custom, {
@@ -432,16 +445,16 @@ Object.defineProperty(fs.exists, internalUtil.promisify.custom, {
   }
 });
 
-
+// fs.existsSync never throws, it only returns true or false.
+// Since fs.existsSync never throws, users have established
+// the expectation that passing invalid arguments to it, even like
+// fs.existsSync(), would only get a false in return, so we cannot signal
+// validation errors to users properly out of compatibility concerns.
+// TODO(joyeecheung): deprecate the never-throw-on-invalid-arguments behavior
 fs.existsSync = function(path) {
   try {
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    try {
+    validatePath(path);
-      validatePath(path);
-    } catch (e) {
-      return false;
-    }
-    nullCheck(path);
     const ctx = { path };
     binding.stat(pathModule.toNamespacedPath(path), undefined, ctx);
     if (ctx.errno !== undefined) {
@@ -456,12 +469,6 @@ fs.existsSync = function(path) {
 fs.readFile = function(path, options, callback) {
   callback = maybeCallback(callback || options);
   options = getOptions(options, { flag: 'r' });
-
-  if (handleError((path = getPathFromURL(path)), callback))
-    return;
-  if (!nullCheck(path, callback))
-    return;
-
   var context = new ReadFileContext(callback, options.encoding);
   context.isUserFd = isFd(path); // file descriptor ownership
   var req = new FSReqWrap();
@@ -475,8 +482,8 @@ fs.readFile = function(path, options, callback) {
     return;
   }
 
+  path = getPathFromURL(path);
   validatePath(path);
-
   binding.open(pathModule.toNamespacedPath(path),
                stringToFlags(options.flag || 'r'),
                0o666,
@@ -754,9 +761,7 @@ fs.open = function(path, flags, mode, callback_) {
   var callback = makeCallback(arguments[arguments.length - 1]);
   mode = modeNum(mode, 0o666);
 
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
   validatePath(path);
   validateUint32(mode, 'mode');
 
@@ -771,8 +776,7 @@ fs.open = function(path, flags, mode, callback_) {
 
 fs.openSync = function(path, flags, mode) {
   mode = modeNum(mode, 0o666);
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   validateUint32(mode, 'mode');
 
@@ -901,15 +905,9 @@ fs.writeSync = function(fd, buffer, offset, length, position) {
 
 fs.rename = function(oldPath, newPath, callback) {
   callback = makeCallback(callback);
-  if (handleError((oldPath = getPathFromURL(oldPath)), callback))
+  oldPath = getPathFromURL(oldPath);
-    return;
-
-  if (handleError((newPath = getPathFromURL(newPath)), callback))
-    return;
-
-  if (!nullCheck(oldPath, callback)) return;
-  if (!nullCheck(newPath, callback)) return;
   validatePath(oldPath, 'oldPath');
+  newPath = getPathFromURL(newPath);
   validatePath(newPath, 'newPath');
   const req = new FSReqWrap();
   req.oncomplete = callback;
@@ -919,11 +917,9 @@ fs.rename = function(oldPath, newPath, callback) {
 };
 
 fs.renameSync = function(oldPath, newPath) {
-  handleError((oldPath = getPathFromURL(oldPath)));
+  oldPath = getPathFromURL(oldPath);
-  handleError((newPath = getPathFromURL(newPath)));
-  nullCheck(oldPath);
-  nullCheck(newPath);
   validatePath(oldPath, 'oldPath');
+  newPath = getPathFromURL(newPath);
   validatePath(newPath, 'newPath');
   const ctx = { path: oldPath, dest: newPath };
   binding.rename(pathModule.toNamespacedPath(oldPath),
@@ -1005,9 +1001,7 @@ fs.ftruncateSync = function(fd, len = 0) {
 
 fs.rmdir = function(path, callback) {
   callback = maybeCallback(callback);
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
   validatePath(path);
   const req = new FSReqWrap();
   req.oncomplete = callback;
@@ -1015,8 +1009,7 @@ fs.rmdir = function(path, callback) {
 };
 
 fs.rmdirSync = function(path) {
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   return binding.rmdir(pathModule.toNamespacedPath(path));
 };
@@ -1056,10 +1049,7 @@ fs.fsyncSync = function(fd) {
 fs.mkdir = function(path, mode, callback) {
   if (typeof mode === 'function') callback = mode;
   callback = makeCallback(callback);
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
-
   validatePath(path);
   mode = modeNum(mode, 0o777);
   validateUint32(mode, 'mode');
@@ -1070,8 +1060,7 @@ fs.mkdir = function(path, mode, callback) {
 };
 
 fs.mkdirSync = function(path, mode) {
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   mode = modeNum(mode, 0o777);
   validateUint32(mode, 'mode');
@@ -1081,10 +1070,7 @@ fs.mkdirSync = function(path, mode) {
 fs.readdir = function(path, options, callback) {
   callback = makeCallback(typeof options === 'function' ? options : callback);
   options = getOptions(options, {});
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
-
   validatePath(path);
 
   const req = new FSReqWrap();
@@ -1094,8 +1080,7 @@ fs.readdir = function(path, options, callback) {
 
 fs.readdirSync = function(path, options) {
   options = getOptions(options, {});
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   return binding.readdir(pathModule.toNamespacedPath(path), options.encoding);
 };
@@ -1109,9 +1094,7 @@ fs.fstat = function(fd, callback) {
 
 fs.lstat = function(path, callback) {
   callback = makeStatsCallback(callback);
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
   validatePath(path);
   const req = new FSReqWrap();
   req.oncomplete = callback;
@@ -1120,9 +1103,7 @@ fs.lstat = function(path, callback) {
 
 fs.stat = function(path, callback) {
   callback = makeStatsCallback(callback);
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
   validatePath(path);
   const req = new FSReqWrap();
   req.oncomplete = callback;
@@ -1140,8 +1121,7 @@ fs.fstatSync = function(fd) {
 };
 
 fs.lstatSync = function(path) {
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   const ctx = { path };
   binding.lstat(pathModule.toNamespacedPath(path), undefined, ctx);
@@ -1152,8 +1132,7 @@ fs.lstatSync = function(path) {
 };
 
 fs.statSync = function(path) {
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   const ctx = { path };
   binding.stat(pathModule.toNamespacedPath(path), undefined, ctx);
@@ -1166,9 +1145,7 @@ fs.statSync = function(path) {
 fs.readlink = function(path, options, callback) {
   callback = makeCallback(typeof options === 'function' ? options : callback);
   options = getOptions(options, {});
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
   validatePath(path, 'oldPath');
   const req = new FSReqWrap();
   req.oncomplete = callback;
@@ -1177,8 +1154,7 @@ fs.readlink = function(path, options, callback) {
 
 fs.readlinkSync = function(path, options) {
   options = getOptions(options, {});
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path, 'oldPath');
   const ctx = { path };
   const result = binding.readlink(pathModule.toNamespacedPath(path),
@@ -1229,14 +1205,8 @@ fs.symlink = function(target, path, type_, callback_) {
   var type = (typeof type_ === 'string' ? type_ : null);
   var callback = makeCallback(arguments[arguments.length - 1]);
 
-  if (handleError((target = getPathFromURL(target)), callback))
+  target = getPathFromURL(target);
-    return;
+  path = getPathFromURL(path);
-
-  if (handleError((path = getPathFromURL(path)), callback))
-    return;
-
-  if (!nullCheck(target, callback)) return;
-  if (!nullCheck(path, callback)) return;
   validatePath(target, 'target');
   validatePath(path);
 
@@ -1250,11 +1220,8 @@ fs.symlink = function(target, path, type_, callback_) {
 
 fs.symlinkSync = function(target, path, type) {
   type = (typeof type === 'string' ? type : null);
-  handleError((target = getPathFromURL(target)));
+  target = getPathFromURL(target);
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(target);
-  nullCheck(path);
-
   validatePath(target, 'target');
   validatePath(path);
   const flags = stringToSymlinkType(type);
@@ -1276,15 +1243,8 @@ fs.symlinkSync = function(target, path, type) {
 fs.link = function(existingPath, newPath, callback) {
   callback = makeCallback(callback);
 
-  if (handleError((existingPath = getPathFromURL(existingPath)), callback))
+  existingPath = getPathFromURL(existingPath);
-    return;
+  newPath = getPathFromURL(newPath);
-
-  if (handleError((newPath = getPathFromURL(newPath)), callback))
-    return;
-
-  if (!nullCheck(existingPath, callback)) return;
-  if (!nullCheck(newPath, callback)) return;
-
   validatePath(existingPath, 'existingPath');
   validatePath(newPath, 'newPath');
 
@@ -1297,10 +1257,8 @@ fs.link = function(existingPath, newPath, callback) {
 };
 
 fs.linkSync = function(existingPath, newPath) {
-  handleError((existingPath = getPathFromURL(existingPath)));
+  existingPath = getPathFromURL(existingPath);
-  handleError((newPath = getPathFromURL(newPath)));
+  newPath = getPathFromURL(newPath);
-  nullCheck(existingPath);
-  nullCheck(newPath);
   validatePath(existingPath, 'existingPath');
   validatePath(newPath, 'newPath');
 
@@ -1316,9 +1274,7 @@ fs.linkSync = function(existingPath, newPath) {
 
 fs.unlink = function(path, callback) {
   callback = makeCallback(callback);
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
   validatePath(path);
   const req = new FSReqWrap();
   req.oncomplete = callback;
@@ -1326,8 +1282,7 @@ fs.unlink = function(path, callback) {
 };
 
 fs.unlinkSync = function(path) {
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   const ctx = { path };
   binding.unlink(pathModule.toNamespacedPath(path), undefined, ctx);
@@ -1394,10 +1349,7 @@ if (constants.O_SYMLINK !== undefined) {
 
 fs.chmod = function(path, mode, callback) {
   callback = makeCallback(callback);
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
-
   validatePath(path);
   mode = modeNum(mode);
   validateUint32(mode, 'mode');
@@ -1408,8 +1360,7 @@ fs.chmod = function(path, mode, callback) {
 };
 
 fs.chmodSync = function(path, mode) {
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   mode = modeNum(mode);
   validateUint32(mode, 'mode');
@@ -1466,10 +1417,7 @@ fs.fchownSync = function(fd, uid, gid) {
 
 fs.chown = function(path, uid, gid, callback) {
   callback = makeCallback(callback);
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
-
   validatePath(path);
   validateUint32(uid, 'uid');
   validateUint32(gid, 'gid');
@@ -1480,8 +1428,7 @@ fs.chown = function(path, uid, gid, callback) {
 };
 
 fs.chownSync = function(path, uid, gid) {
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   validateUint32(uid, 'uid');
   validateUint32(gid, 'gid');
@@ -1515,10 +1462,7 @@ fs._toUnixTimestamp = toUnixTimestamp;
 
 fs.utimes = function(path, atime, mtime, callback) {
   callback = makeCallback(callback);
-  if (handleError((path = getPathFromURL(path)), callback))
+  path = getPathFromURL(path);
-    return;
-  if (!nullCheck(path, callback)) return;
-
   validatePath(path);
 
   const req = new FSReqWrap();
@@ -1530,8 +1474,7 @@ fs.utimes = function(path, atime, mtime, callback) {
 };
 
 fs.utimesSync = function(path, atime, mtime) {
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
   validatePath(path);
   binding.utimes(pathModule.toNamespacedPath(path),
                  toUnixTimestamp(atime),
@@ -1685,12 +1628,17 @@ function FSWatcher() {
 }
 util.inherits(FSWatcher, EventEmitter);
 
+// FIXME(joyeecheung): this method is not documented.
+// At the moment if filename is undefined, we
+// 1. Throw an Error from C++ land if it's the first time .start() is called
+// 2. Return silently from C++ land if .start() has already been called
+//    on a valid filename and the wrap has been initialized
 FSWatcher.prototype.start = function(filename,
                                      persistent,
                                      recursive,
                                      encoding) {
-  handleError((filename = getPathFromURL(filename)));
+  filename = getPathFromURL(filename);
-  nullCheck(filename);
+  nullCheck(filename, 'filename');
   var err = this._handle.start(pathModule.toNamespacedPath(filename),
                                persistent,
                                recursive,
@@ -1708,9 +1656,6 @@ FSWatcher.prototype.close = function() {
 };
 
 fs.watch = function(filename, options, listener) {
-  handleError((filename = getPathFromURL(filename)));
-  nullCheck(filename);
-
   if (typeof options === 'function') {
     listener = options;
   }
@@ -1777,9 +1722,14 @@ function StatWatcher() {
 util.inherits(StatWatcher, EventEmitter);
 
 
+// FIXME(joyeecheung): this method is not documented.
+// At the moment if filename is undefined, we
+// 1. Throw an Error from C++ land if it's the first time .start() is called
+// 2. Return silently from C++ land if .start() has already been called
+//    on a valid filename and the wrap has been initialized
 StatWatcher.prototype.start = function(filename, persistent, interval) {
-  handleError((filename = getPathFromURL(filename)));
+  filename = getPathFromURL(filename);
-  nullCheck(filename);
+  nullCheck(filename, 'filename');
   this._handle.start(pathModule.toNamespacedPath(filename),
                      persistent, interval);
 };
@@ -1793,8 +1743,8 @@ StatWatcher.prototype.stop = function() {
 const statWatchers = new Map();
 
 fs.watchFile = function(filename, options, listener) {
-  handleError((filename = getPathFromURL(filename)));
+  filename = getPathFromURL(filename);
-  nullCheck(filename);
+  validatePath(filename);
   filename = pathModule.resolve(filename);
   var stat;
 
@@ -1833,8 +1783,8 @@ fs.watchFile = function(filename, options, listener) {
 };
 
 fs.unwatchFile = function(filename, listener) {
-  handleError((filename = getPathFromURL(filename)));
+  filename = getPathFromURL(filename);
-  nullCheck(filename);
+  validatePath(filename);
   filename = pathModule.resolve(filename);
   var stat = statWatchers.get(filename);
 
@@ -1903,12 +1853,11 @@ fs.realpathSync = function realpathSync(p, options) {
     options = emptyObj;
   else
     options = getOptions(options, emptyObj);
+  p = getPathFromURL(p);
   if (typeof p !== 'string') {
-    handleError((p = getPathFromURL(p)));
+    p += '';
-    if (typeof p !== 'string')
-      p += '';
   }
-  nullCheck(p);
+  validatePath(p);
   p = pathModule.resolve(p);
 
   const cache = options[internalFS.realpathCacheKey];
@@ -2046,8 +1995,8 @@ fs.realpathSync = function realpathSync(p, options) {
 
 fs.realpathSync.native = function(path, options) {
   options = getOptions(options, {});
-  handleError((path = getPathFromURL(path)));
+  path = getPathFromURL(path);
-  nullCheck(path);
+  validatePath(path);
   return binding.realpath(path, options.encoding);
 };
 
@@ -2058,14 +2007,11 @@ fs.realpath = function realpath(p, options, callback) {
     options = emptyObj;
   else
     options = getOptions(options, emptyObj);
+  p = getPathFromURL(p);
   if (typeof p !== 'string') {
-    if (handleError((p = getPathFromURL(p)), callback))
+    p += '';
-      return;
-    if (typeof p !== 'string')
-      p += '';
   }
-  if (!nullCheck(p, callback))
+  validatePath(p);
-    return;
   p = pathModule.resolve(p);
 
   const seenLinks = Object.create(null);
@@ -2192,14 +2138,13 @@ fs.realpath = function realpath(p, options, callback) {
 fs.realpath.native = function(path, options, callback) {
   callback = maybeCallback(callback || options);
   options = getOptions(options, {});
-  if (handleError((path = getPathFromURL(path)), callback)) return;
+  path = getPathFromURL(path);
-  if (!nullCheck(path, callback)) return;
+  validatePath(path);
   const req = new FSReqWrap();
   req.oncomplete = callback;
   return binding.realpath(path, options.encoding, req);
 };
 
-
 fs.mkdtemp = function(prefix, options, callback) {
   callback = makeCallback(typeof options === 'function' ? options : callback);
   options = getOptions(options, {});
@@ -2209,26 +2154,22 @@ fs.mkdtemp = function(prefix, options, callback) {
                                'string',
                                prefix);
   }
-  if (!nullCheck(prefix, callback)) {
+  nullCheck(prefix, 'prefix');
-    return;
-  }
-
   var req = new FSReqWrap();
   req.oncomplete = callback;
-
   binding.mkdtemp(`${prefix}XXXXXX`, options.encoding, req);
 };
 
 
 fs.mkdtempSync = function(prefix, options) {
+  options = getOptions(options, {});
   if (!prefix || typeof prefix !== 'string') {
     throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
                                'prefix',
                                'string',
                                prefix);
   }
-  options = getOptions(options, {});
+  nullCheck(prefix, 'prefix');
-  nullCheck(prefix);
   return binding.mkdtemp(`${prefix}XXXXXX`, options.encoding);
 };
 
@@ -2248,21 +2189,7 @@ fs.copyFile = function(src, dest, flags, callback) {
   }
 
   src = getPathFromURL(src);
-
-  if (handleError(src, callback))
-    return;
-
-  if (!nullCheck(src, callback))
-    return;
-
   dest = getPathFromURL(dest);
-
-  if (handleError(dest, callback))
-    return;
-
-  if (!nullCheck(dest, callback))
-    return;
-
   validatePath(src, 'src');
   validatePath(dest, 'dest');
 
@@ -2277,13 +2204,7 @@ fs.copyFile = function(src, dest, flags, callback) {
 
 fs.copyFileSync = function(src, dest, flags) {
   src = getPathFromURL(src);
-  handleError(src);
-  nullCheck(src);
-
   dest = getPathFromURL(dest);
-  handleError(dest);
-  nullCheck(dest);
-
   validatePath(src, 'src');
   validatePath(dest, 'dest');
 
@@ -2320,7 +2241,8 @@ function ReadStream(path, options) {
 
   Readable.call(this, options);
 
-  handleError((this.path = getPathFromURL(path)));
+  // path will be ignored when fd is specified, so it can be falsy
+  this.path = getPathFromURL(path);
   this.fd = options.fd === undefined ? null : options.fd;
   this.flags = options.flags === undefined ? 'r' : options.flags;
   this.mode = options.mode === undefined ? 0o666 : options.mode;
@@ -2483,7 +2405,8 @@ function WriteStream(path, options) {
 
   Writable.call(this, options);
 
-  handleError((this.path = getPathFromURL(path)));
+  // path will be ignored when fd is specified, so it can be falsy
+  this.path = getPathFromURL(path);
   this.fd = options.fd === undefined ? null : options.fd;
   this.flags = options.flags === undefined ? 'w' : options.flags;
   this.mode = options.mode === undefined ? 0o666 : options.mode;
@@ -2783,8 +2706,7 @@ async function readFileHandle(filehandle, options) {
 // thrown synchronously
 const promises = {
   async access(path, mode = fs.F_OK) {
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
 
     mode = mode | 0;
@@ -2793,10 +2715,8 @@ const promises = {
   },
 
   async copyFile(src, dest, flags) {
-    handleError((src = getPathFromURL(src)));
+    src = getPathFromURL(src);
-    handleError((dest = getPathFromURL(dest)));
+    dest = getPathFromURL(dest);
-    nullCheck(src);
-    nullCheck(dest);
     validatePath(src, 'src');
     validatePath(dest, 'dest');
     flags = flags | 0;
@@ -2809,8 +2729,7 @@ const promises = {
   // promises.open() uses the fs.FileHandle class.
   async open(path, flags, mode) {
     mode = modeNum(mode, 0o666);
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     validateUint32(mode, 'mode');
     return new FileHandle(
@@ -2877,10 +2796,8 @@ const promises = {
   },
 
   async rename(oldPath, newPath) {
-    handleError((oldPath = getPathFromURL(oldPath)));
+    oldPath = getPathFromURL(oldPath);
-    handleError((newPath = getPathFromURL(newPath)));
+    newPath = getPathFromURL(newPath);
-    nullCheck(oldPath);
-    nullCheck(newPath);
     validatePath(oldPath, 'oldPath');
     validatePath(newPath, 'newPath');
     return binding.rename(pathModule.toNamespacedPath(oldPath),
@@ -2900,8 +2817,7 @@ const promises = {
   },
 
   async rmdir(path) {
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     return binding.rmdir(pathModule.toNamespacedPath(path), kUsePromises);
   },
@@ -2918,8 +2834,7 @@ const promises = {
 
   async mkdir(path, mode) {
     mode = modeNum(mode, 0o777);
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     validateUint32(mode, 'mode');
     return binding.mkdir(pathModule.toNamespacedPath(path), mode, kUsePromises);
@@ -2927,8 +2842,7 @@ const promises = {
 
   async readdir(path, options) {
     options = getOptions(options, {});
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     return binding.readdir(pathModule.toNamespacedPath(path),
                            options.encoding, kUsePromises);
@@ -2936,8 +2850,7 @@ const promises = {
 
   async readlink(path, options) {
     options = getOptions(options, {});
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path, 'oldPath');
     return binding.readlink(pathModule.toNamespacedPath(path),
                             options.encoding, kUsePromises);
@@ -2945,10 +2858,8 @@ const promises = {
 
   async symlink(target, path, type_) {
     const type = (typeof type_ === 'string' ? type_ : null);
-    handleError((target = getPathFromURL(target)));
+    target = getPathFromURL(target);
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(target);
-    nullCheck(path);
     validatePath(target, 'target');
     validatePath(path);
     return binding.symlink(preprocessSymlinkDestination(target, type, path),
@@ -2963,26 +2874,22 @@ const promises = {
   },
 
   async lstat(path) {
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     return statsFromValues(
       await binding.lstat(pathModule.toNamespacedPath(path), kUsePromises));
   },
 
   async stat(path) {
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     return statsFromValues(
       await binding.stat(pathModule.toNamespacedPath(path), kUsePromises));
   },
 
   async link(existingPath, newPath) {
-    handleError((existingPath = getPathFromURL(existingPath)));
+    existingPath = getPathFromURL(existingPath);
-    handleError((newPath = getPathFromURL(newPath)));
+    newPath = getPathFromURL(newPath);
-    nullCheck(existingPath);
-    nullCheck(newPath);
     validatePath(existingPath, 'existingPath');
     validatePath(newPath, 'newPath');
     return binding.link(pathModule.toNamespacedPath(existingPath),
@@ -2991,8 +2898,7 @@ const promises = {
   },
 
   async unlink(path) {
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     return binding.unlink(pathModule.toNamespacedPath(path), kUsePromises);
   },
@@ -3007,8 +2913,7 @@ const promises = {
   },
 
   async chmod(path, mode) {
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     mode = modeNum(mode);
     validateUint32(mode, 'mode');
@@ -3041,8 +2946,7 @@ const promises = {
   },
 
   async chown(path, uid, gid) {
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     validateUint32(uid, 'uid');
     validateUint32(gid, 'gid');
@@ -3051,8 +2955,7 @@ const promises = {
   },
 
   async utimes(path, atime, mtime) {
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     return binding.utimes(pathModule.toNamespacedPath(path),
                           toUnixTimestamp(atime),
@@ -3069,8 +2972,7 @@ const promises = {
 
   async realpath(path, options) {
     options = getOptions(options, {});
-    handleError((path = getPathFromURL(path)));
+    path = getPathFromURL(path);
-    nullCheck(path);
     validatePath(path);
     return binding.realpath(path, options.encoding, kUsePromises);
   },

lib/internal/url.js

@@ -1321,7 +1321,7 @@ function getPathFromURLWin32(url) {
       var third = pathname.codePointAt(n + 2) | 0x20;
       if ((pathname[n + 1] === '2' && third === 102) || // 2f 2F /
           (pathname[n + 1] === '5' && third === 99)) {  // 5c 5C \
-        return new errors.TypeError(
+        throw new errors.TypeError(
           'ERR_INVALID_FILE_URL_PATH',
           'must not include encoded \\ or / characters');
       }
@@ -1342,8 +1342,8 @@ function getPathFromURLWin32(url) {
     var sep = pathname[2];
     if (letter < 97 || letter > 122 ||   // a..z A..Z
         (sep !== ':')) {
-      return new errors.TypeError('ERR_INVALID_FILE_URL_PATH',
+      throw new errors.TypeError('ERR_INVALID_FILE_URL_PATH',
-                                  'must be absolute');
+                                 'must be absolute');
     }
     return pathname.slice(1);
   }
@@ -1351,15 +1351,15 @@ function getPathFromURLWin32(url) {
 
 function getPathFromURLPosix(url) {
   if (url.hostname !== '') {
-    return new errors.TypeError('ERR_INVALID_FILE_URL_HOST', platform);
+    throw new errors.TypeError('ERR_INVALID_FILE_URL_HOST', platform);
   }
   var pathname = url.pathname;
   for (var n = 0; n < pathname.length; n++) {
     if (pathname[n] === '%') {
       var third = pathname.codePointAt(n + 2) | 0x20;
       if (pathname[n + 1] === '2' && third === 102) {
-        return new errors.TypeError('ERR_INVALID_FILE_URL_PATH',
+        throw new errors.TypeError('ERR_INVALID_FILE_URL_PATH',
-                                    'must not include encoded / characters');
+                                   'must not include encoded / characters');
       }
     }
   }
@@ -1372,7 +1372,7 @@ function getPathFromURL(path) {
     return path;
   }
   if (path.protocol !== 'file:')
-    return new errors.TypeError('ERR_INVALID_URL_SCHEME', 'file');
+    throw new errors.TypeError('ERR_INVALID_URL_SCHEME', 'file');
   return isWindows ? getPathFromURLWin32(path) : getPathFromURLPosix(path);
 }
 

test/parallel/test-fs-copyfile.js

@@ -102,26 +102,6 @@ common.expectsError(() => {
   );
 });
 
-// Throws if the source path is an invalid path.
-common.expectsError(() => {
-  fs.copyFileSync('\u0000', dest);
-}, {
-  code: 'ERR_INVALID_ARG_TYPE',
-  type: Error,
-  message: 'The "path" argument must be of type string without null bytes.' +
-           ' Received type string'
-});
-
-// Throws if the destination path is an invalid path.
-common.expectsError(() => {
-  fs.copyFileSync(src, '\u0000');
-}, {
-  code: 'ERR_INVALID_ARG_TYPE',
-  type: Error,
-  message: 'The "path" argument must be of type string without null bytes.' +
-           ' Received type string'
-});
-
 // Errors if invalid flags are provided.
 assert.throws(() => {
   fs.copyFileSync(src, dest, -1);

test/parallel/test-fs-exists.js

@@ -26,30 +26,33 @@ const fs = require('fs');
 const { URL } = require('url');
 const f = __filename;
 
+// Only warnings are emitted when the callback is invalid
+assert.doesNotThrow(() => fs.exists(f));
+assert.doesNotThrow(() => fs.exists());
+assert.doesNotThrow(() => fs.exists(f, {}));
+
 fs.exists(f, common.mustCall(function(y) {
   assert.strictEqual(y, true);
 }));
 
-assert.doesNotThrow(() => fs.exists(f));
-
 fs.exists(`${f}-NO`, common.mustCall(function(y) {
   assert.strictEqual(y, false);
 }));
 
+// If the path is invalid, fs.exists will still invoke the callback with false
+// instead of throwing errors
 fs.exists(new URL('https://foo'), common.mustCall(function(y) {
   assert.strictEqual(y, false);
 }));
 
+fs.exists({}, common.mustCall(function(y) {
+  assert.strictEqual(y, false);
+}));
+
 assert(fs.existsSync(f));
 assert(!fs.existsSync(`${f}-NO`));
 
-common.expectsError(
+// fs.existsSync() never throws
-  () => { fs.exists(() => {}); },
-  {
-    code: 'ERR_INVALID_ARG_TYPE',
-    message: 'The "path" argument must be one of type string, Buffer, or URL',
-    type: TypeError
-  }
-);
-
 assert(!fs.existsSync());
+assert(!fs.existsSync({}));
+assert(!fs.existsSync(new URL('https://foo')));

test/parallel/test-fs-null-bytes.js

@@ -27,16 +27,7 @@ const URL = require('url').URL;
 
 function check(async, sync) {
   const argsSync = Array.prototype.slice.call(arguments, 2);
-  const argsAsync = argsSync.concat((er) => {
+  const argsAsync = argsSync.concat(common.mustNotCall());
-    common.expectsError(
-      () => {
-        throw er;
-      },
-      {
-        code: 'ERR_INVALID_ARG_TYPE',
-        type: Error
-      });
-  });
 
   if (sync) {
     common.expectsError(
@@ -44,13 +35,20 @@ function check(async, sync) {
         sync.apply(null, argsSync);
       },
       {
-        code: 'ERR_INVALID_ARG_TYPE',
+        code: 'ERR_INVALID_ARG_VALUE',
         type: Error,
       });
   }
 
   if (async) {
-    async.apply(null, argsAsync);
+    common.expectsError(
+      () => {
+        async.apply(null, argsAsync);
+      },
+      {
+        code: 'ERR_INVALID_ARG_VALUE',
+        type: Error
+      });
   }
 }
 
@@ -59,6 +57,8 @@ check(fs.access, fs.accessSync, 'foo\u0000bar', fs.F_OK);
 check(fs.appendFile, fs.appendFileSync, 'foo\u0000bar', 'abc');
 check(fs.chmod, fs.chmodSync, 'foo\u0000bar', '0644');
 check(fs.chown, fs.chownSync, 'foo\u0000bar', 12, 34);
+check(fs.copyFile, fs.copyFileSync, 'foo\u0000bar', 'abc');
+check(fs.copyFile, fs.copyFileSync, 'abc', 'foo\u0000bar');
 check(fs.link, fs.linkSync, 'foo\u0000bar', 'foobar');
 check(fs.link, fs.linkSync, 'foobar', 'foo\u0000bar');
 check(fs.lstat, fs.lstatSync, 'foo\u0000bar');
@@ -90,6 +90,8 @@ check(fs.access, fs.accessSync, fileUrl, fs.F_OK);
 check(fs.appendFile, fs.appendFileSync, fileUrl, 'abc');
 check(fs.chmod, fs.chmodSync, fileUrl, '0644');
 check(fs.chown, fs.chownSync, fileUrl, 12, 34);
+check(fs.copyFile, fs.copyFileSync, fileUrl, 'abc');
+check(fs.copyFile, fs.copyFileSync, 'abc', fileUrl);
 check(fs.link, fs.linkSync, fileUrl, 'foobar');
 check(fs.link, fs.linkSync, 'foobar', fileUrl);
 check(fs.lstat, fs.lstatSync, fileUrl);
@@ -118,6 +120,8 @@ check(fs.access, fs.accessSync, fileUrl2, fs.F_OK);
 check(fs.appendFile, fs.appendFileSync, fileUrl2, 'abc');
 check(fs.chmod, fs.chmodSync, fileUrl2, '0644');
 check(fs.chown, fs.chownSync, fileUrl2, 12, 34);
+check(fs.copyFile, fs.copyFileSync, fileUrl2, 'abc');
+check(fs.copyFile, fs.copyFileSync, 'abc', fileUrl2);
 check(fs.link, fs.linkSync, fileUrl2, 'foobar');
 check(fs.link, fs.linkSync, 'foobar', fileUrl2);
 check(fs.lstat, fs.lstatSync, fileUrl2);

test/parallel/test-fs-whatwg-url.js

@@ -29,46 +29,75 @@ fs.readFile(url, common.mustCall((err, data) => {
 
 // Check that using a non file:// URL reports an error
 const httpUrl = new URL('http://example.org');
-fs.readFile(httpUrl, common.expectsError({
+
-  code: 'ERR_INVALID_URL_SCHEME',
+common.expectsError(
-  type: TypeError,
+  () => {
-  message: 'The URL must be of scheme file'
+    fs.readFile(httpUrl, common.mustNotCall());
-}));
+  },
+  {
+    code: 'ERR_INVALID_URL_SCHEME',
+    type: TypeError,
+    message: 'The URL must be of scheme file'
+  });
 
 // pct-encoded characters in the path will be decoded and checked
-fs.readFile(new URL('file:///c:/tmp/%00test'), common.mustCall((err) => {
-  common.expectsError(
-    () => {
-      throw err;
-    },
-    {
-      code: 'ERR_INVALID_ARG_TYPE',
-      type: Error
-    });
-}));
-
 if (common.isWindows) {
   // encoded back and forward slashes are not permitted on windows
   ['%2f', '%2F', '%5c', '%5C'].forEach((i) => {
-    fs.readFile(new URL(`file:///c:/tmp/${i}`), common.expectsError({
+    common.expectsError(
-      code: 'ERR_INVALID_FILE_URL_PATH',
+      () => {
-      type: TypeError,
+        fs.readFile(new URL(`file:///c:/tmp/${i}`), common.mustNotCall());
-      message: 'File URL path must not include encoded \\ or / characters'
+      },
-    }));
+      {
+        code: 'ERR_INVALID_FILE_URL_PATH',
+        type: TypeError,
+        message: 'File URL path must not include encoded \\ or / characters'
+      }
+    );
   });
+  common.expectsError(
+    () => {
+      fs.readFile(new URL('file:///c:/tmp/%00test'), common.mustNotCall());
+    },
+    {
+      code: 'ERR_INVALID_ARG_VALUE',
+      type: Error,
+      message: 'The argument \'path\' must be a string or Uint8Array without ' +
+               'null bytes. Received \'c:/tmp/\\u0000test\''
+    }
+  );
 } else {
   // encoded forward slashes are not permitted on other platforms
   ['%2f', '%2F'].forEach((i) => {
-    fs.readFile(new URL(`file:///c:/tmp/${i}`), common.expectsError({
+    common.expectsError(
-      code: 'ERR_INVALID_FILE_URL_PATH',
+      () => {
-      type: TypeError,
+        fs.readFile(new URL(`file:///c:/tmp/${i}`), common.mustNotCall());
-      message: 'File URL path must not include encoded / characters'
+      },
-    }));
+      {
+        code: 'ERR_INVALID_FILE_URL_PATH',
+        type: TypeError,
+        message: 'File URL path must not include encoded / characters'
+      });
   });
-
+  common.expectsError(
-  fs.readFile(new URL('file://hostname/a/b/c'), common.expectsError({
+    () => {
-    code: 'ERR_INVALID_FILE_URL_HOST',
+      fs.readFile(new URL('file://hostname/a/b/c'), common.mustNotCall());
-    type: TypeError,
+    },
-    message: `File URL host must be "localhost" or empty on ${os.platform()}`
+    {
-  }));
+      code: 'ERR_INVALID_FILE_URL_HOST',
+      type: TypeError,
+      message: `File URL host must be "localhost" or empty on ${os.platform()}`
+    }
+  );
+  common.expectsError(
+    () => {
+      fs.readFile(new URL('file:///tmp/%00test'), common.mustNotCall());
+    },
+    {
+      code: 'ERR_INVALID_ARG_VALUE',
+      type: Error,
+      message: 'The argument \'path\' must be a string or Uint8Array without ' +
+               'null bytes. Received \'/tmp/\\u0000test\''
+    }
+  );
 }

test/parallel/test-repl-persistent-history.js

@@ -57,7 +57,7 @@ const CLEAR = { ctrl: true, name: 'u' };
 // File paths
 const historyFixturePath = fixtures.path('.node_repl_history');
 const historyPath = path.join(tmpdir.path, '.fixture_copy_repl_history');
-const historyPathFail = path.join(tmpdir.path, '.node_repl\u0000_history');
+const historyPathFail = fixtures.path('nonexistent_folder', 'filename');
 const oldHistoryPathObj = fixtures.path('old-repl-history-file-obj.json');
 const oldHistoryPathFaulty = fixtures.path('old-repl-history-file-faulty.json');
 const oldHistoryPath = fixtures.path('old-repl-history-file.json');