errors: move error creation helpers to errors.js

This commit moves error creation helpers scattered around under lib/ into lib/internal/errors.js in the hope of being clearer about the differences of errors that we throw into the user land. - Move util._errnoException and util._exceptionWithHostPort into internal/errors.js and simplify their logic so it's clearer what the properties these helpers create. - Move the errnoException helper in dns.js to internal/errors.js into internal/errors.js and rename it to dnsException. Simplify it's logic so it no longer calls errnoException and skips the unnecessary argument checks. PR-URL: https://github.com/nodejs/node/pull/18546 Reviewed-By: James M Snell <jasnell@gmail.com>
2018-02-03 17:09:15 +08:00 · 2018-02-03 17:09:15 +08:00 · c0762c2f54
--- a/lib/dgram.js
+++ b/lib/dgram.js
@ -45,8 +45,8 @@ const SEND_BUFFER = false;
 // Lazily loaded
 var cluster = null;

-const errnoException = util._errnoException;
-const exceptionWithHostPort = util._exceptionWithHostPort;
+const errnoException = errors.errnoException;
+const exceptionWithHostPort = errors.exceptionWithHostPort;


 function lookup4(lookup, address, callback) {
--- a/lib/dns.js
+++ b/lib/dns.js
@ -21,17 +21,10 @@

 'use strict';

-const util = require('util');
-
 const cares = process.binding('cares_wrap');
 const { isIP, isIPv4, isLegalPort } = require('internal/net');
 const { customPromisifyArgs } = require('internal/util');
 const errors = require('internal/errors');
-const {
-  UV_EAI_MEMORY,
-  UV_EAI_NODATA,
-  UV_EAI_NONAME
-} = process.binding('uv');

 const {
  GetAddrInfoReqWrap,
@ -40,36 +33,12 @@ const {
  ChannelWrap,
 } = cares;

-function errnoException(err, syscall, hostname) {
-  // FIXME(bnoordhuis) Remove this backwards compatibility nonsense and pass
-  // the true error to the user. ENOTFOUND is not even a proper POSIX error!
-  if (err === UV_EAI_MEMORY ||
-      err === UV_EAI_NODATA ||
-      err === UV_EAI_NONAME) {
-    err = 'ENOTFOUND';
-  }
-  var ex = null;
-  if (typeof err === 'string') {  // c-ares error code.
-    const errHost = hostname ? ` ${hostname}` : '';
-    ex = new Error(`${syscall} ${err}${errHost}`);
-    ex.code = err;
-    ex.errno = err;
-    ex.syscall = syscall;
-  } else {
-    ex = util._errnoException(err, syscall);
-  }
-  if (hostname) {
-    ex.hostname = hostname;
-  }
-  return ex;
-}
-
 const IANA_DNS_PORT = 53;
-
+const dnsException = errors.dnsException;

 function onlookup(err, addresses) {
  if (err) {
-    return this.callback(errnoException(err, 'getaddrinfo', this.hostname));
+    return this.callback(dnsException(err, 'getaddrinfo', this.hostname));
  }
  if (this.family) {
    this.callback(null, addresses[0], this.family);
@ -81,7 +50,7 @@ function onlookup(err, addresses) {

 function onlookupall(err, addresses) {
  if (err) {
-    return this.callback(errnoException(err, 'getaddrinfo', this.hostname));
+    return this.callback(dnsException(err, 'getaddrinfo', this.hostname));
  }

  var family = this.family;
@ -161,7 +130,7 @@ function lookup(hostname, options, callback) {

  var err = cares.getaddrinfo(req, hostname, family, hints, verbatim);
  if (err) {
-    process.nextTick(callback, errnoException(err, 'getaddrinfo', hostname));
+    process.nextTick(callback, dnsException(err, 'getaddrinfo', hostname));
    return {};
  }
  return req;
@ -173,7 +142,7 @@ Object.defineProperty(lookup, customPromisifyArgs,

 function onlookupservice(err, host, service) {
  if (err)
-    return this.callback(errnoException(err, 'getnameinfo', this.host));
+    return this.callback(dnsException(err, 'getnameinfo', this.host));

  this.callback(null, host, service);
 }
@ -202,7 +171,7 @@ function lookupService(host, port, callback) {
  req.oncomplete = onlookupservice;

  var err = cares.getnameinfo(req, host, port);
-  if (err) throw errnoException(err, 'getnameinfo', host);
+  if (err) throw dnsException(err, 'getnameinfo', host);
  return req;
 }

@ -215,7 +184,7 @@ function onresolve(err, result, ttls) {
    result = result.map((address, index) => ({ address, ttl: ttls[index] }));

  if (err)
-    this.callback(errnoException(err, this.bindingName, this.hostname));
+    this.callback(dnsException(err, this.bindingName, this.hostname));
  else
    this.callback(null, result);
 }
@ -253,7 +222,7 @@ function resolver(bindingName) {
    req.oncomplete = onresolve;
    req.ttl = !!(options && options.ttl);
    var err = this._handle[bindingName](req, name);
-    if (err) throw errnoException(err, bindingName);
+    if (err) throw dnsException(err, bindingName);
    return req;
  }
  Object.defineProperty(query, 'name', { value: bindingName });
--- a/lib/fs.js
+++ b/lib/fs.js
@ -62,7 +62,7 @@ const { kMaxLength } = require('buffer');
 const isWindows = process.platform === 'win32';

 const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG);
-const errnoException = util._errnoException;
+const errnoException = errors.errnoException;

 let truncateWarn = true;

--- a/lib/internal/child_process.js
+++ b/lib/internal/child_process.js
@ -29,7 +29,7 @@ const {
  UV_ESRCH
 } = process.binding('uv');

-const errnoException = util._errnoException;
+const errnoException = errors.errnoException;
 const { SocketListSend, SocketListReceive } = SocketList;

 const MAX_HANDLE_RETRANSMISSIONS = 3;
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@ -18,7 +18,12 @@ var green = '';
 var red = '';
 var white = '';

-const { errmap } = process.binding('uv');
+const {
+  errmap,
+  UV_EAI_MEMORY,
+  UV_EAI_NODATA,
+  UV_EAI_NONAME
+} = process.binding('uv');
 const { kMaxLength } = process.binding('buffer');
 const { defineProperty } = Object;

@ -33,6 +38,14 @@ function lazyUtil() {
  return util_;
 }

+var internalUtil = null;
+function lazyInternalUtil() {
+  if (!internalUtil) {
+    internalUtil = require('internal/util');
+  }
+  return internalUtil;
+}
+
 function makeNodeError(Base) {
  return class NodeError extends Base {
    constructor(key, ...args) {
@ -356,10 +369,15 @@ function E(sym, val) {
  messages.set(sym, typeof val === 'function' ? val : String(val));
 }

-// 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
+/**
+ * This creates an error compatible with errors produced in the C++
+ * function UVException using a context object with data assembled in C++.
+ * The goal is to migrate them to ERR_* errors later when compatibility is
+ * not a concern.
+ *
+ * @param {Object} ctx
+ * @returns {Error}
+ */
 function uvException(ctx) {
  const err = new Error();

@ -389,7 +407,110 @@ function uvException(ctx) {
  return err;
 }

+/**
+ * This used to be util._errnoException().
+ *
+ * @param {number} err - A libuv error number
+ * @param {string} syscall
+ * @param {string} [original]
+ * @returns {Error}
+ */
+function errnoException(err, syscall, original) {
+  // TODO(joyeecheung): We have to use the type-checked
+  // getSystemErrorName(err) to guard against invalid arguments from users.
+  // This can be replaced with [ code ] = errmap.get(err) when this method
+  // is no longer exposed to user land.
+  const code = lazyUtil().getSystemErrorName(err);
+  const message = original ?
+    `${syscall} ${code} ${original}` : `${syscall} ${code}`;
+
+  const ex = new Error(message);
+  // TODO(joyeecheung): errno is supposed to err, like in uvException
+  ex.code = ex.errno = code;
+  ex.syscall = syscall;
+
+  Error.captureStackTrace(ex, errnoException);
+  return ex;
+}
+
+/**
+ * This used to be util._exceptionWithHostPort().
+ *
+ * @param {number} err - A libuv error number
+ * @param {string} syscall
+ * @param {string} address
+ * @param {number} [port]
+ * @param {string} [additional]
+ * @returns {Error}
+ */
+function exceptionWithHostPort(err, syscall, address, port, additional) {
+  // TODO(joyeecheung): We have to use the type-checked
+  // getSystemErrorName(err) to guard against invalid arguments from users.
+  // This can be replaced with [ code ] = errmap.get(err) when this method
+  // is no longer exposed to user land.
+  const code = lazyUtil().getSystemErrorName(err);
+  let details = '';
+  if (port && port > 0) {
+    details = ` ${address}:${port}`;
+  } else if (address) {
+    details = ` ${address}`;
+  }
+  if (additional) {
+    details += ` - Local (${additional})`;
+  }
+
+  const ex = new Error(`${syscall} ${code}${details}`);
+  // TODO(joyeecheung): errno is supposed to err, like in uvException
+  ex.code = ex.errno = code;
+  ex.syscall = syscall;
+  ex.address = address;
+  if (port) {
+    ex.port = port;
+  }
+
+  Error.captureStackTrace(ex, exceptionWithHostPort);
+  return ex;
+}
+
+/**
+ * @param {number|string} err - A libuv error number or a c-ares error code
+ * @param {string} syscall
+ * @param {string} [hostname]
+ * @returns {Error}
+ */
+function dnsException(err, syscall, hostname) {
+  const ex = new Error();
+  // FIXME(bnoordhuis) Remove this backwards compatibility nonsense and pass
+  // the true error to the user. ENOTFOUND is not even a proper POSIX error!
+  if (err === UV_EAI_MEMORY ||
+      err === UV_EAI_NODATA ||
+      err === UV_EAI_NONAME) {
+    err = 'ENOTFOUND';  // Fabricated error name.
+  }
+  if (typeof err === 'string') {  // c-ares error code.
+    const errHost = hostname ? ` ${hostname}` : '';
+    ex.message = `${syscall} ${err}${errHost}`;
+    // TODO(joyeecheung): errno is supposed to be a number, like in uvException
+    ex.code = ex.errno = err;
+    ex.syscall = syscall;
+  } else {  // libuv error number
+    const code = lazyInternalUtil().getSystemErrorName(err);
+    ex.message = `${syscall} ${code}`;
+    // TODO(joyeecheung): errno is supposed to be err, like in uvException
+    ex.code = ex.errno = code;
+    ex.syscall = syscall;
+  }
+  if (hostname) {
+    ex.hostname = hostname;
+  }
+  Error.captureStackTrace(ex, dnsException);
+  return ex;
+}
+
 module.exports = exports = {
+  dnsException,
+  errnoException,
+  exceptionWithHostPort,
  uvException,
  message,
  Error: makeNodeError(Error),
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@ -1631,7 +1631,7 @@ class Http2Stream extends Duplex {
    req.async = false;
    const err = createWriteReq(req, handle, data, encoding);
    if (err)
-      throw util._errnoException(err, 'write', req.error);
+      throw errors.errnoException(err, 'write', req.error);
    trackWriteState(this, req.bytes);
  }

@ -1674,7 +1674,7 @@ class Http2Stream extends Duplex {
    }
    const err = handle.writev(req, chunks);
    if (err)
-      throw util._errnoException(err, 'write', req.error);
+      throw errors.errnoException(err, 'write', req.error);
    trackWriteState(this, req.bytes);
  }

--- a/lib/internal/process.js
+++ b/lib/internal/process.js
@ -170,7 +170,7 @@ function setupKillAndExit() {
    }

    if (err)
-      throw util._errnoException(err, 'kill');
+      throw errors.errnoException(err, 'kill');

    return true;
  };
@ -200,7 +200,7 @@ function setupSignalHandlers() {
      const err = wrap.start(signum);
      if (err) {
        wrap.close();
-        throw util._errnoException(err, 'uv_signal_start');
+        throw errors.errnoException(err, 'uv_signal_start');
      }

      signalWraps[type] = wrap;
--- a/lib/net.js
+++ b/lib/net.js
@ -59,8 +59,8 @@ const kLastWriteQueueSize = Symbol('lastWriteQueueSize');
 // reasons it's lazy loaded.
 var cluster = null;

-const errnoException = util._errnoException;
-const exceptionWithHostPort = util._exceptionWithHostPort;
+const errnoException = errors.errnoException;
+const exceptionWithHostPort = errors.exceptionWithHostPort;

 const {
  kTimeout,
--- a/lib/tty.js
+++ b/lib/tty.js
@ -21,7 +21,7 @@

 'use strict';

-const { inherits, _errnoException, _extend } = require('util');
+const { inherits, _extend } = require('util');
 const net = require('net');
 const { TTY, isTTY } = process.binding('tty_wrap');
 const errors = require('internal/errors');
@ -178,7 +178,7 @@ WriteStream.prototype._refreshSize = function() {
  const winSize = new Array(2);
  const err = this._handle.getWindowSize(winSize);
  if (err) {
-    this.emit('error', _errnoException(err, 'getWindowSize'));
+    this.emit('error', errors.errnoException(err, 'getWindowSize'));
    return;
  }
  const [newCols, newRows] = winSize;
--- a/lib/util.js
+++ b/lib/util.js
@ -1058,40 +1058,6 @@ function error(...args) {
  }
 }

-function _errnoException(err, syscall, original) {
-  const name = getSystemErrorName(err);
-  var message = `${syscall} ${name}`;
-  if (original)
-    message += ` ${original}`;
-  const e = new Error(message);
-  e.code = e.errno = name;
-  e.syscall = syscall;
-  return e;
-}
-
-function _exceptionWithHostPort(err,
-                                syscall,
-                                address,
-                                port,
-                                additional) {
-  var details;
-  if (port && port > 0) {
-    details = `${address}:${port}`;
-  } else {
-    details = address;
-  }
-
-  if (additional) {
-    details += ` - Local (${additional})`;
-  }
-  var ex = exports._errnoException(err, syscall, details);
-  ex.address = address;
-  if (port) {
-    ex.port = port;
-  }
-  return ex;
-}
-
 function callbackifyOnRejected(reason, cb) {
  // `!reason` guard inspired by bluebird (Ref: https://goo.gl/t5IS6M).
  // Because `null` is a special error value in callbacks which means "no error
@ -1152,8 +1118,8 @@ function getSystemErrorName(err) {

 // Keep the `exports =` so that various functions can still be monkeypatched
 module.exports = exports = {
-  _errnoException,
-  _exceptionWithHostPort,
+  _errnoException: errors.errnoException,
+  _exceptionWithHostPort: errors.exceptionWithHostPort,
  _extend,
  callbackify,
  debuglog,