From 714c529a7e8a97dce2661f3c122aa6ff660d48b6 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 3 Feb 2018 17:09:15 +0800 Subject: [PATCH] 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 --- lib/dgram.js | 4 +- lib/dns.js | 46 +++----------- lib/fs.js | 2 +- lib/internal/child_process.js | 2 +- lib/internal/errors.js | 116 ++++++++++++++++++++++++++++++++++ lib/internal/http2/core.js | 4 +- lib/internal/process.js | 4 +- lib/net.js | 4 +- lib/tty.js | 8 +-- lib/util.js | 38 +---------- 10 files changed, 139 insertions(+), 89 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index cd70dc5be91f0d..56eb0ea29b0356 100644 --- 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) { diff --git a/lib/dns.js b/lib/dns.js index 92b55f03595f45..c2fa1169076b2e 100644 --- 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 { 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, @@ -41,30 +34,6 @@ const { isIP } = 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 digits = [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0-15 @@ -91,10 +60,11 @@ function isIPv4(str) { return (str.length > 3 && str.charCodeAt(3) === 46/*'.'*/); } +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); @@ -106,7 +76,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; @@ -186,7 +156,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; @@ -198,7 +168,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); } @@ -227,7 +197,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; } @@ -240,7 +210,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); } @@ -278,7 +248,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 }); diff --git a/lib/fs.js b/lib/fs.js index 1f752c071bdd81..d0a0280f4000b9 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -63,7 +63,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; diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 13d0a11aa1e705..827ef0770ac784 100644 --- 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; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 0927cee81d2d5a..f4a77e037bc491 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -13,6 +13,11 @@ const kCode = Symbol('code'); const messages = new Map(); +const { + UV_EAI_MEMORY, + UV_EAI_NODATA, + UV_EAI_NONAME +} = process.binding('uv'); const { kMaxLength } = process.binding('buffer'); const { defineProperty } = Object; @@ -25,6 +30,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) { @@ -128,7 +141,110 @@ function E(sym, val) { messages.set(sym, typeof val === 'function' ? val : String(val)); } +/** + * 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, message, Error: makeNodeError(Error), TypeError: makeNodeError(TypeError), diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index ec4862ab3842ee..f3f953e35d67d5 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1619,7 +1619,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); } @@ -1662,7 +1662,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); } diff --git a/lib/internal/process.js b/lib/internal/process.js index 93529ceca5f38b..70186f0e66deaa 100644 --- 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; diff --git a/lib/net.js b/lib/net.js index c04d04473900d0..b9f4bd127be5b8 100644 --- a/lib/net.js +++ b/lib/net.js @@ -54,8 +54,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; function noop() {} diff --git a/lib/tty.js b/lib/tty.js index 9595c79db39a33..29440d6d96e3d5 100644 --- a/lib/tty.js +++ b/lib/tty.js @@ -21,11 +21,9 @@ 'use strict'; -const util = require('util'); +const { inherits, _extend } = require('util'); const net = require('net'); const { TTY, isTTY } = process.binding('tty_wrap'); -const { inherits } = util; -const errnoException = util._errnoException; const errors = require('internal/errors'); const readline = require('readline'); @@ -40,7 +38,7 @@ function ReadStream(fd, options) { if (fd >> 0 !== fd || fd < 0) throw new errors.RangeError('ERR_INVALID_FD', fd); - options = util._extend({ + options = _extend({ highWaterMark: 0, readable: true, writable: false, @@ -99,7 +97,7 @@ WriteStream.prototype._refreshSize = function() { var winSize = new Array(2); var err = this._handle.getWindowSize(winSize); if (err) { - this.emit('error', errnoException(err, 'getWindowSize')); + this.emit('error', errors.errnoException(err, 'getWindowSize')); return; } var newCols = winSize[0]; diff --git a/lib/util.js b/lib/util.js index 539608c4164893..318a4cef24c93d 100644 --- a/lib/util.js +++ b/lib/util.js @@ -990,40 +990,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 @@ -1081,8 +1047,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,