Skip to content

Commit

Permalink
errors: move error creation helpers to errors.js
Browse files Browse the repository at this point in the history
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: nodejs#18546
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung committed May 2, 2018
1 parent 2179547 commit d8b637f
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 86 deletions.
4 changes: 2 additions & 2 deletions lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
43 changes: 9 additions & 34 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@

'use strict';

const util = require('util');

const cares = process.binding('cares_wrap');
const uv = process.binding('uv');
const { isLegalPort } = require('internal/net');
const { customPromisifyArgs } = require('internal/util');
const errors = require('internal/errors');

const {
GetAddrInfoReqWrap,
Expand All @@ -36,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.UV_EAI_MEMORY ||
err === uv.UV_EAI_NODATA ||
err === uv.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
Expand All @@ -86,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);
Expand All @@ -101,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;
Expand Down Expand Up @@ -181,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;
Expand All @@ -193,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);
}
Expand Down Expand Up @@ -222,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;
}

Expand All @@ -235,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);
}
Expand Down Expand Up @@ -272,7 +247,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 });
Expand Down
2 changes: 1 addition & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,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;

function getOptions(options, defaultOptions) {
if (options === null || options === undefined ||
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const SocketList = require('internal/socket_list');
const { convertToValidSignal } = require('internal/util');
const { isUint8Array } = require('internal/util/types');

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

const MAX_HANDLE_RETRANSMISSIONS = 3;
Expand Down
116 changes: 116 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
const kCode = Symbol('code');
const messages = new Map();

const {
UV_EAI_MEMORY,
UV_EAI_NODATA,
UV_EAI_NONAME
} = process.binding('uv');
const { defineProperty } = Object;

// Lazily loaded
Expand All @@ -22,6 +27,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) {
Expand Down Expand Up @@ -125,7 +138,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),
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1647,7 +1647,7 @@ class Http2Stream extends Duplex {
req.async = false;
const err = createWriteReq(req, handle, data, encoding);
if (err)
return this.destroy(util._errnoException(err, 'write', req.error), cb);
return this.destroy(errors.errnoException(err, 'write', req.error), cb);
trackWriteState(this, req.bytes);
}

Expand Down Expand Up @@ -1690,7 +1690,7 @@ class Http2Stream extends Duplex {
}
const err = handle.writev(req, chunks);
if (err)
return this.destroy(util._errnoException(err, 'write', req.error), cb);
return this.destroy(errors.errnoException(err, 'write', req.error), cb);
trackWriteState(this, req.bytes);
}

Expand Down
5 changes: 3 additions & 2 deletions lib/internal/process.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

const errors = require('internal/errors');
const util = require('util');
const constants = process.binding('constants').os.signals;

Expand Down Expand Up @@ -180,7 +181,7 @@ function setupKillAndExit() {
}

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

return true;
};
Expand Down Expand Up @@ -211,7 +212,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;
Expand Down
4 changes: 2 additions & 2 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ const dns = require('dns');
// 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() {}

Expand Down
8 changes: 3 additions & 5 deletions lib/tty.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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,
Expand Down Expand Up @@ -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];
Expand Down
Loading

0 comments on commit d8b637f

Please sign in to comment.