From d1d9e2fce521ae1deae4fa2b75a7f93a9c5bdafb Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Tue, 6 Feb 2018 18:38:31 +0200 Subject: [PATCH 01/18] doc: fix typo in http2.md PR-URL: https://github.com/nodejs/node/pull/18602 Reviewed-By: Anatoli Papirovski Reviewed-By: Luigi Pinca --- doc/api/http2.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index 3e62108048e..f1d4b1590d7 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1701,7 +1701,7 @@ changes: and the total frame length will *not* necessarily be aligned at 8 bytes. * `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent streams for the remote peer as if a SETTINGS frame had been received. Will - be overridden if the remote peer sets its own value for. + be overridden if the remote peer sets its own value for `maxConcurrentStreams`. **Default:** `100` * `selectPadding` {Function} When `options.paddingStrategy` is equal to `http2.constants.PADDING_STRATEGY_CALLBACK`, provides the callback function From 610cac26f0f0d98df2541bdc4251c624cca52702 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Tue, 6 Feb 2018 22:34:50 +0100 Subject: [PATCH 02/18] doc: add missing "changes" key in YAML comment PR-URL: https://github.com/nodejs/node/pull/18605 Reviewed-By: Anna Henningsen Reviewed-By: Anatoli Papirovski --- doc/api/http.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/api/http.md b/doc/api/http.md index 5659bb2edab..341a08c5aeb 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1666,6 +1666,7 @@ Found'`. ## http.createServer([options][, requestListener]) -* `file` {string|Buffer|URL|[FileHandle][]} filename or `FileHandle` +* `file` {string|Buffer|URL|FileHandle} filename or `FileHandle` * `data` {string|Buffer} * `options` {Object|string} * `encoding` {string|null} **Default:** `'utf8'` @@ -3640,7 +3640,7 @@ fs.promises.copyFile('source.txt', 'destination.txt', COPYFILE_EXCL) added: REPLACEME --> -* `filehandle` {[FileHandle][]} +* `filehandle` {FileHandle} * `mode` {integer} * Returns: {Promise} @@ -3652,7 +3652,7 @@ success. added: REPLACEME --> -* `filehandle` {[FileHandle][]} +* `filehandle` {FileHandle} * `uid` {integer} * `gid` {integer} * Returns: {Promise} @@ -3665,7 +3665,7 @@ the `Promise` with no arguments upon success. added: REPLACEME --> -* `filehandle` {[FileHandle][]} +* `filehandle` {FileHandle} * Returns: {Promise} Asynchronous fdatasync(2). The `Promise` is resolved with no arguments upon @@ -3676,7 +3676,7 @@ success. added: REPLACEME --> -* `filehandle` {[FileHandle][]} +* `filehandle` {FileHandle} * Returns: {Promise} Retrieves the [`fs.Stats`][] for the given `filehandle`. @@ -3686,7 +3686,7 @@ Retrieves the [`fs.Stats`][] for the given `filehandle`. added: REPLACEME --> -* `filehandle` {[FileHandle][]} +* `filehandle` {FileHandle} * Returns: {Promise} Asynchronous fsync(2). The `Promise` is resolved with no arguments upon @@ -3697,7 +3697,7 @@ success. added: REPLACEME --> -* `filehandle` {[FileHandle][]} +* `filehandle` {FileHandle} * `len` {integer} **Default:** `0` * Returns: {Promise} @@ -3746,7 +3746,7 @@ The last three bytes are null bytes ('\0'), to compensate the over-truncation. added: REPLACEME --> -* `filehandle` {[FileHandle][]} +* `filehandle` {FileHandle} * `atime` {number|string|Date} * `mtime` {number|string|Date}` * Returns: {Promise} @@ -3934,7 +3934,7 @@ files can be opened for writing with the `r+` flag. A call to added: REPLACEME --> -* `filehandle` {[FileHandle][]} +* `filehandle` {FileHandle} * `buffer` {Buffer|Uint8Array} * `offset` {integer} * `length` {integer} @@ -3981,7 +3981,7 @@ will be passed as `Buffer` objects. added: REPLACEME --> -* `path` {string|Buffer|URL|[FileHandle][]} filename or `FileHandle` +* `path` {string|Buffer|URL|FileHandle} filename or `FileHandle` * `options` {Object|string} * `encoding` {string|null} **Default:** `null` * `flag` {string} **Default:** `'r'` @@ -4147,7 +4147,7 @@ The `atime` and `mtime` arguments follow these rules: added: REPLACEME --> -* `filehandle` {[FileHandle][]} +* `filehandle` {FileHandle} * `buffer` {Buffer|Uint8Array} * `offset` {integer} * `length` {integer} @@ -4180,7 +4180,7 @@ the end of the file. added: REPLACEME --> -* `file` {string|Buffer|URL|[FileHandle][]} filename or `FileHandle` +* `file` {string|Buffer|URL|FileHandle} filename or `FileHandle` * `data` {string|Buffer|Uint8Array} * `options` {Object|string} * `encoding` {string|null} **Default:** `'utf8'` @@ -4469,7 +4469,6 @@ The following constants are meant for use with the [`fs.Stats`][] object's [`util.promisify()`]: util.html#util_util_promisify_original [Caveats]: #fs_caveats [Common System Errors]: errors.html#errors_common_system_errors -[FileHandle]: #fs_class_filehandle [FS Constants]: #fs_fs_constants_1 [MDN-Date]: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Date [MDN-Number]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type diff --git a/tools/doc/type-parser.js b/tools/doc/type-parser.js index 0ab73162dd5..1a7698ad6a3 100644 --- a/tools/doc/type-parser.js +++ b/tools/doc/type-parser.js @@ -45,6 +45,7 @@ const typeMap = { 'EventEmitter': 'events.html#events_class_eventemitter', + 'FileHandle': 'fs.html#fs_class_filehandle', 'fs.Stats': 'fs.html#fs_class_fs_stats', 'http.Agent': 'http.html#http_class_http_agent', From 12ae33476a69a3b13477ffd60a7497088e769870 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 3 Feb 2018 17:30:48 +0800 Subject: [PATCH 06/18] util: skip type checks in internal getSystemErrorName PR-URL: https://github.com/nodejs/node/pull/18546 Reviewed-By: James M Snell --- lib/internal/util.js | 8 -------- lib/util.js | 13 ++++++++++++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 1e48eedfd59..3de8090cfa2 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -221,14 +221,6 @@ function getConstructorOf(obj) { } function getSystemErrorName(err) { - if (typeof err !== 'number') { - throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'err', 'number', err); - } - if (err >= 0 || !Number.isSafeInteger(err)) { - throw new errors.RangeError('ERR_OUT_OF_RANGE', 'err', - 'a negative integer', err); - } - const entry = errmap.get(err); return entry ? entry[0] : `Unknown system error ${err}`; } diff --git a/lib/util.js b/lib/util.js index 4525792b2ec..14c99bd454c 100644 --- a/lib/util.js +++ b/lib/util.js @@ -55,7 +55,7 @@ const { const { customInspectSymbol, deprecate, - getSystemErrorName, + getSystemErrorName: internalErrorName, getIdentificationOf, isError, promisify, @@ -1139,6 +1139,17 @@ function callbackify(original) { return callbackified; } +function getSystemErrorName(err) { + if (typeof err !== 'number') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'err', 'number', err); + } + if (err >= 0 || !Number.isSafeInteger(err)) { + throw new errors.RangeError('ERR_OUT_OF_RANGE', 'err', + 'a negative integer', err); + } + return internalErrorName(err); +} + // Keep the `exports =` so that various functions can still be monkeypatched module.exports = exports = { _errnoException, From 030384833f95ee9f61fc194cdcab4075df083926 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 3 Feb 2018 17:35:04 +0800 Subject: [PATCH 07/18] fs: do not call new when creating uvException We cannot make uvException a proper class due to compatibility reasons for now, so there is no need to call new since it only returns a newly-created Error. PR-URL: https://github.com/nodejs/node/pull/18546 Reviewed-By: James M Snell --- lib/fs.js | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 7087a5f03c5..7136911e60e 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -407,7 +407,7 @@ fs.accessSync = function(path, mode) { binding.access(pathModule.toNamespacedPath(path), mode, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } }; @@ -638,7 +638,7 @@ function tryStatSync(fd, isUserFd) { binding.fstat(fd, undefined, ctx); if (ctx.errno !== undefined && !isUserFd) { fs.closeSync(fd); - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } } @@ -736,7 +736,7 @@ fs.closeSync = function(fd) { const ctx = {}; binding.close(fd, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } }; @@ -929,7 +929,7 @@ fs.renameSync = function(oldPath, newPath) { binding.rename(pathModule.toNamespacedPath(oldPath), pathModule.toNamespacedPath(newPath), undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } }; @@ -999,7 +999,7 @@ fs.ftruncateSync = function(fd, len = 0) { const ctx = {}; binding.ftruncate(fd, len, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } }; @@ -1033,7 +1033,7 @@ fs.fdatasyncSync = function(fd) { const ctx = {}; binding.fdatasync(fd, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } }; @@ -1049,7 +1049,7 @@ fs.fsyncSync = function(fd) { const ctx = {}; binding.fsync(fd, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } }; @@ -1134,7 +1134,7 @@ fs.fstatSync = function(fd) { const ctx = { fd }; binding.fstat(fd, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } return statsFromValues(); }; @@ -1146,7 +1146,7 @@ fs.lstatSync = function(path) { const ctx = { path }; binding.lstat(pathModule.toNamespacedPath(path), undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } return statsFromValues(); }; @@ -1158,7 +1158,7 @@ fs.statSync = function(path) { const ctx = { path }; binding.stat(pathModule.toNamespacedPath(path), undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } return statsFromValues(); }; @@ -1184,7 +1184,7 @@ fs.readlinkSync = function(path, options) { const result = binding.readlink(pathModule.toNamespacedPath(path), options.encoding, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } return result; }; @@ -1264,7 +1264,7 @@ fs.symlinkSync = function(target, path, type) { pathModule.toNamespacedPath(path), flags, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } else if (ctx.error) { // TODO(joyeecheung): this is an encoding error usually caused by memory // problems. We need to figure out proper error code(s) for this. @@ -1309,7 +1309,7 @@ fs.linkSync = function(existingPath, newPath) { pathModule.toNamespacedPath(newPath), undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } return result; }; @@ -1332,7 +1332,7 @@ fs.unlinkSync = function(path) { const ctx = { path }; binding.unlink(pathModule.toNamespacedPath(path), undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } }; @@ -1939,7 +1939,7 @@ fs.realpathSync = function realpathSync(p, options) { const ctx = { path: base }; binding.lstat(pathModule.toNamespacedPath(base), undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } knownHard[base] = true; } @@ -1983,7 +1983,7 @@ fs.realpathSync = function realpathSync(p, options) { const ctx = { path: base }; binding.lstat(baseLong, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } if ((statValues[1/*mode*/] & S_IFMT) !== S_IFLNK) { @@ -2008,11 +2008,11 @@ fs.realpathSync = function realpathSync(p, options) { const ctx = { path: base }; binding.stat(baseLong, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } linkTarget = binding.readlink(baseLong, undefined, undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } } resolvedLink = pathModule.resolve(previous, linkTarget); @@ -2033,7 +2033,7 @@ fs.realpathSync = function realpathSync(p, options) { const ctx = { path: base }; binding.lstat(pathModule.toNamespacedPath(base), undefined, ctx); if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); + throw errors.uvException(ctx); } knownHard[base] = true; } From c0762c2f54f80b92f2f5e0f9af51f4c048c53e4f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 3 Feb 2018 17:09:15 +0800 Subject: [PATCH 08/18] 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 | 47 +++--------- lib/fs.js | 2 +- lib/internal/child_process.js | 2 +- lib/internal/errors.js | 131 ++++++++++++++++++++++++++++++++-- lib/internal/http2/core.js | 4 +- lib/internal/process.js | 4 +- lib/net.js | 4 +- lib/tty.js | 4 +- lib/util.js | 38 +--------- 10 files changed, 148 insertions(+), 92 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index 0c4990cac5d..bed6129b47b 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 979d724c3e4..51c144f4e39 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 { 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 }); diff --git a/lib/fs.js b/lib/fs.js index 7136911e60e..04d3d0a5bd4 100644 --- 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; diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 2bade01f95f..ac2b75f2457 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 a41df135407..c07920ae5d0 100644 --- 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), diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index f735e2fcc92..5f811ca8af4 100644 --- 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); } diff --git a/lib/internal/process.js b/lib/internal/process.js index 757c8de8e68..776129a140f 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 0ba7457b7f8..5d99b00662d 100644 --- 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, diff --git a/lib/tty.js b/lib/tty.js index 584ff9d8709..79e61e981b5 100644 --- 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; diff --git a/lib/util.js b/lib/util.js index 14c99bd454c..e13f5b8b800 100644 --- 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, From d797775fb82caae5f721140625dfb0f984ce929e Mon Sep 17 00:00:00 2001 From: Shobhit Chittora Date: Wed, 1 Nov 2017 01:31:02 +0530 Subject: [PATCH 09/18] tools: add fixer for prefer-assert-iferror.js PR-URL: https://github.com/nodejs/node/pull/16648 Refs: https://github.com/nodejs/node/issues/16636 Reviewed-By: Anatoli Papirovski Reviewed-By: Gibson Fahnestock Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- .../test-eslint-prefer-assert-iferror.js | 14 +++++++---- tools/eslint-rules/prefer-assert-iferror.js | 23 +++++++++++++++++-- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-eslint-prefer-assert-iferror.js b/test/parallel/test-eslint-prefer-assert-iferror.js index 6e577b1fc35..8a9723f4b21 100644 --- a/test/parallel/test-eslint-prefer-assert-iferror.js +++ b/test/parallel/test-eslint-prefer-assert-iferror.js @@ -14,12 +14,18 @@ new RuleTester().run('prefer-assert-iferror', rule, { ], invalid: [ { - code: 'if (err) throw err;', - errors: [{ message: 'Use assert.ifError(err) instead.' }] + code: 'require("assert");\n' + + 'if (err) throw err;', + errors: [{ message: 'Use assert.ifError(err) instead.' }], + output: 'require("assert");\n' + + 'assert.ifError(err);' }, { - code: 'if (error) { throw error; }', - errors: [{ message: 'Use assert.ifError(error) instead.' }] + code: 'require("assert");\n' + + 'if (error) { throw error; }', + errors: [{ message: 'Use assert.ifError(error) instead.' }], + output: 'require("assert");\n' + + 'assert.ifError(error);' } ] }); diff --git a/tools/eslint-rules/prefer-assert-iferror.js b/tools/eslint-rules/prefer-assert-iferror.js index e1528741769..399ee7403a6 100644 --- a/tools/eslint-rules/prefer-assert-iferror.js +++ b/tools/eslint-rules/prefer-assert-iferror.js @@ -5,9 +5,12 @@ 'use strict'; +const utils = require('./rules-utils.js'); + module.exports = { create(context) { const sourceCode = context.getSourceCode(); + var assertImported = false; function hasSameTokens(nodeA, nodeB) { const aTokens = sourceCode.getTokens(nodeA); @@ -20,8 +23,15 @@ module.exports = { }); } + function checkAssertNode(node) { + if (utils.isRequired(node, ['assert'])) { + assertImported = true; + } + } + return { - IfStatement(node) { + 'CallExpression': (node) => checkAssertNode(node), + 'IfStatement': (node) => { const firstStatement = node.consequent.type === 'BlockStatement' ? node.consequent.body[0] : node.consequent; @@ -30,10 +40,19 @@ module.exports = { firstStatement.type === 'ThrowStatement' && hasSameTokens(node.test, firstStatement.argument) ) { + const argument = sourceCode.getText(node.test); context.report({ node: firstStatement, message: 'Use assert.ifError({{argument}}) instead.', - data: { argument: sourceCode.getText(node.test) } + data: { argument }, + fix: (fixer) => { + if (assertImported) { + return fixer.replaceText( + node, + `assert.ifError(${argument});` + ); + } + } }); } } From e5429204679cab10cd4ee28d5dfec9026a91a6d1 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 3 Feb 2018 18:27:21 +0800 Subject: [PATCH 10/18] test: do not check TXT content in test-dns-any google.com added another TXT record which broke this test. This removes the check on the content of the TXT record since that depends on an external state subject to change. PR-URL: https://github.com/nodejs/node/pull/18547 Reviewed-By: Khaidi Chu Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: Jon Moss Reviewed-By: Anatoli Papirovski Reviewed-By: James M Snell --- test/internet/test-dns-any.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/internet/test-dns-any.js b/test/internet/test-dns-any.js index dd80e48bf44..a83040801f3 100644 --- a/test/internet/test-dns-any.js +++ b/test/internet/test-dns-any.js @@ -59,9 +59,6 @@ const checkers = { checkTXT(r) { assert.ok(Array.isArray(r.entries)); assert.ok(r.entries.length > 0); - r.entries.forEach((txt) => { - assert(txt.startsWith('v=spf1')); - }); assert.strictEqual(r.type, 'TXT'); }, checkSOA(r) { From b8f7f84fc233a5876b1d275fed63775bdbb7932f Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Mon, 5 Feb 2018 15:55:15 -0500 Subject: [PATCH 11/18] test: fix flaky test-http2-session-unref This test should exit naturally or will timeout on its own, a separate unrefed timer is not necessary. PR-URL: https://github.com/nodejs/node/pull/18589 Fixes: https://github.com/nodejs/node/issues/18587 Reviewed-By: Michael Dawson Reviewed-By: James M Snell Reviewed-By: Yuta Hiroto Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig --- test/parallel/test-http2-session-unref.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/parallel/test-http2-session-unref.js b/test/parallel/test-http2-session-unref.js index e765352cdc6..e63cd0d208e 100644 --- a/test/parallel/test-http2-session-unref.js +++ b/test/parallel/test-http2-session-unref.js @@ -49,5 +49,3 @@ server.listen(0, common.mustCall(() => { })); server.emit('connection', serverSide); server.unref(); - -setTimeout(common.mustNotCall(() => {}), 1000).unref(); From 93df1169ea451983f09611c0acc9d3c98760dd28 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Tue, 6 Feb 2018 11:19:04 +0200 Subject: [PATCH 12/18] lib,doc: revert format name to cjs over commonjs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/18596 Reviewed-By: Michaël Zasso Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Myles Borins Reviewed-By: Ruben Bridgewater --- doc/api/esm.md | 2 +- lib/internal/loader/DefaultResolve.js | 2 +- lib/internal/loader/Translators.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 10f20958593..2cfa5a9f29c 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -137,7 +137,7 @@ module. This can be one of the following: | `format` | Description | | --- | --- | | `"esm"` | Load a standard JavaScript module | -| `"commonjs"` | Load a node-style CommonJS module | +| `"cjs"` | Load a node-style CommonJS module | | `"builtin"` | Load a node builtin CommonJS module | | `"json"` | Load a JSON file | | `"addon"` | Load a [C++ Addon][addons] | diff --git a/lib/internal/loader/DefaultResolve.js b/lib/internal/loader/DefaultResolve.js index faadf0bc99e..69dd9537c18 100644 --- a/lib/internal/loader/DefaultResolve.js +++ b/lib/internal/loader/DefaultResolve.js @@ -44,7 +44,7 @@ const extensionFormatMap = { '.mjs': 'esm', '.json': 'json', '.node': 'addon', - '.js': 'commonjs' + '.js': 'cjs' }; function resolve(specifier, parentURL) { diff --git a/lib/internal/loader/Translators.js b/lib/internal/loader/Translators.js index f1db8334dd5..d2f28774177 100644 --- a/lib/internal/loader/Translators.js +++ b/lib/internal/loader/Translators.js @@ -32,7 +32,7 @@ translators.set('esm', async (url) => { // Strategy for loading a node-style CommonJS module const isWindows = process.platform === 'win32'; const winSepRegEx = /\//g; -translators.set('commonjs', async (url) => { +translators.set('cjs', async (url) => { debug(`Translating CJSModule ${url}`); const pathname = internalURLModule.getPathFromURL(new URL(url)); const module = CJSModule._cache[ From 4ac7be94f004811a57545c5a191381156af23e43 Mon Sep 17 00:00:00 2001 From: "Tim O. Peters" Date: Thu, 4 Jan 2018 11:17:01 +0100 Subject: [PATCH 13/18] doc: be more explicit in the sypnosis Assuming less knowledge on the part of the reader, making it easier to get start using Node.js. PR-URL: https://github.com/nodejs/node/pull/17977 Fixes: https://github.com/nodejs/node/issues/17970, Reviewed-By: Vse Mozhet Byt Reviewed-By: Gireesh Punathil Reviewed-By: Ruben Bridgewater Reviewed-By: Jeremiah Senkpiel --- doc/api/synopsis.md | 76 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 8 deletions(-) diff --git a/doc/api/synopsis.md b/doc/api/synopsis.md index ada2437387b..508dde1ff48 100644 --- a/doc/api/synopsis.md +++ b/doc/api/synopsis.md @@ -9,9 +9,58 @@ Please see the [Command Line Options][] document for information about different options and ways to run scripts with Node.js. ## Example - An example of a [web server][] written with Node.js which responds with -`'Hello World'`: +`'Hello World!'`: + +Commands displayed in this document are shown starting with `$` or `>` +to replicate how they would appear in a user's terminal. +Do not include the `$` and `>` character they are there to +indicate the start of each command. + +There are many tutorials and examples that follow this +convention: `$` or `>` for commands run as a regular user, and `#` +for commands that should be executed as an administrator. + +Lines that don’t start with `$` or `>` character are typically showing +the output of the previous command. + +Firstly, make sure to have downloaded and installed Node.js. +See [this guide][] for further install information. + +Now, create an empty project folder called `projects`, navigate into it: +Project folder can be named base on user's current project title but +this example will use `projects` as the project folder. + +Linux and Mac: + +```console +$ mkdir ~/projects +$ cd ~/projects +``` + +Windows CMD: + +```console +> mkdir %USERPROFILE%\projects +> cd %USERPROFILE%\projects +``` + +Windows PowerShell: + +```console +> mkdir $env:USERPROFILE\projects +> cd $env:USERPROFILE\projects +``` + +Next, create a new source file in the `projects` + folder and call it `hello-world.js`. + +In Node.js it is considered good style to use +hyphens (`-`) or underscores (`_`) to separate + multiple words in filenames. + +Open `hello-world.js` in any preferred text editor and +paste in the following content. ```js const http = require('http'); @@ -22,7 +71,7 @@ const port = 3000; const server = http.createServer((req, res) => { res.statusCode = 200; res.setHeader('Content-Type', 'text/plain'); - res.end('Hello World\n'); + res.end('Hello World!\n'); }); server.listen(port, hostname, () => { @@ -30,15 +79,26 @@ server.listen(port, hostname, () => { }); ``` -To run the server, put the code into a file called `example.js` and execute -it with Node.js: +Save the file, go back to the terminal window enter the following command: -```txt -$ node example.js -Server running at http://127.0.0.1:3000/ +```console +$ node hello-world.js ``` +An output like this should appear in the terminal to indicate Node.js +server is running: + + ```console + Server running at http://127.0.0.1:3000/ + ```` + +Now, open any preferred web browser and visit `http://127.0.0.1:3000`. + +If the browser displays the string `Hello, world!`, that indicates +the server is working. + Many of the examples in the documentation can be run similarly. [Command Line Options]: cli.html#cli_command_line_options [web server]: http.html +[this guide]: https://nodejs.org/en/download/package-manager/ From 5ecd2e15b8725f67219b746a6291992dbef62f06 Mon Sep 17 00:00:00 2001 From: Bhavani Shankar Date: Thu, 1 Feb 2018 15:32:41 +0530 Subject: [PATCH 14/18] test: improve error message output PR-URL: https://github.com/nodejs/node/pull/18498 Reviewed-By: Ruben Bridgewater Reviewed-By: Anatoli Papirovski Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Gireesh Punathil Reviewed-By: Michael Dawson --- test/addons-napi/test_general/test.js | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/test/addons-napi/test_general/test.js b/test/addons-napi/test_general/test.js index c89d718ca18..bcaa13d894b 100644 --- a/test/addons-napi/test_general/test.js +++ b/test/addons-napi/test_general/test.js @@ -63,22 +63,25 @@ let w = {}; test_general.wrap(w); w = null; global.gc(); -assert.strictEqual(test_general.derefItemWasCalled(), true, +const derefItemWasCalled = test_general.derefItemWasCalled(); +assert.strictEqual(derefItemWasCalled, true, 'deref_item() was called upon garbage collecting a ' + - 'wrapped object'); + 'wrapped object. test_general.derefItemWasCalled() ' + + `returned ${derefItemWasCalled}`); + // Assert that wrapping twice fails. const x = {}; test_general.wrap(x); -assert.throws(() => test_general.wrap(x), Error); +common.expectsError(() => test_general.wrap(x), + { type: Error, message: 'Invalid argument' }); // Ensure that wrapping, removing the wrap, and then wrapping again works. const y = {}; test_general.wrap(y); test_general.removeWrap(y); -assert.doesNotThrow(() => test_general.wrap(y), Error, - 'Wrapping twice succeeds if a remove_wrap()' + - ' separates the instances'); +// Wrapping twice succeeds if a remove_wrap() separates the instances +assert.doesNotThrow(() => test_general.wrap(y)); // Ensure that removing a wrap and garbage collecting does not fire the // finalize callback. @@ -87,8 +90,11 @@ test_general.testFinalizeWrap(z); test_general.removeWrap(z); z = null; global.gc(); -assert.strictEqual(test_general.finalizeWasCalled(), false, - 'finalize callback was not called upon garbage collection'); +const finalizeWasCalled = test_general.finalizeWasCalled(); +assert.strictEqual(finalizeWasCalled, false, + 'finalize callback was not called upon garbage collection.' + + ' test_general.finalizeWasCalled() ' + + `returned ${finalizeWasCalled}`); // test napi_adjust_external_memory const adjustedValue = test_general.testAdjustExternalMemory(); From af5632e4f40b2462139ae1534bc5ef99ce39ebbb Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 4 Feb 2018 13:43:21 -0500 Subject: [PATCH 15/18] readline: use Date.now() and move test to parallel The readline module wants a truthy time while using Timer.now() doesn't necessarily guarantee that early on in the process' life. It also doesn't actually resolve the timing issues experienced in an earlier issue. Instead, this PR fixes the related tests and moves them back to parallel. Refs: https://github.com/nodejs/node/issues/14674 PR-URL: https://github.com/nodejs/node/pull/18563 Reviewed-By: Anna Henningsen Reviewed-By: Luigi Pinca Reviewed-By: Ruben Bridgewater --- lib/readline.js | 10 +- test/parallel/test-readline-interface.js | 77 +++++++++++++++ test/sequential/test-readline-interface.js | 108 --------------------- 3 files changed, 81 insertions(+), 114 deletions(-) delete mode 100644 test/sequential/test-readline-interface.js diff --git a/lib/readline.js b/lib/readline.js index bca9c25ab23..5f28f4c43f1 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -48,8 +48,6 @@ const { kClearScreenDown } = CSI; -const { now } = process.binding('timer_wrap').Timer; - const kHistorySize = 30; const kMincrlfDelay = 100; // \r\n, \n, or \r followed by something other than \n @@ -411,7 +409,7 @@ Interface.prototype._normalWrite = function(b) { } var string = this._decoder.write(b); if (this._sawReturnAt && - now() - this._sawReturnAt <= this.crlfDelay) { + Date.now() - this._sawReturnAt <= this.crlfDelay) { string = string.replace(/^\n/, ''); this._sawReturnAt = 0; } @@ -424,7 +422,7 @@ Interface.prototype._normalWrite = function(b) { this._line_buffer = null; } if (newPartContainsEnding) { - this._sawReturnAt = string.endsWith('\r') ? now() : 0; + this._sawReturnAt = string.endsWith('\r') ? Date.now() : 0; // got one or more newlines; process into "line" events var lines = string.split(lineEnding); @@ -917,14 +915,14 @@ Interface.prototype._ttyWrite = function(s, key) { switch (key.name) { case 'return': // carriage return, i.e. \r - this._sawReturnAt = now(); + this._sawReturnAt = Date.now(); this._line(); break; case 'enter': // When key interval > crlfDelay if (this._sawReturnAt === 0 || - now() - this._sawReturnAt > this.crlfDelay) { + Date.now() - this._sawReturnAt > this.crlfDelay) { this._line(); } this._sawReturnAt = 0; diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 69c0f113ff1..fe33e244ceb 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -869,3 +869,80 @@ function isWarned(emitter) { assert.strictEqual(rl._prompt, '$ '); } }); + +// For the purposes of the following tests, we do not care about the exact +// value of crlfDelay, only that the behaviour conforms to what's expected. +// Setting it to Infinity allows the test to succeed even under extreme +// CPU stress. +const crlfDelay = Infinity; + +[ true, false ].forEach(function(terminal) { + // sending multiple newlines at once that does not end with a new line + // and a `end` event(last line is) + + // \r\n should emit one line event, not two + { + const fi = new FakeInput(); + const rli = new readline.Interface( + { + input: fi, + output: fi, + terminal: terminal, + crlfDelay + } + ); + const expectedLines = ['foo', 'bar', 'baz', 'bat']; + let callCount = 0; + rli.on('line', function(line) { + assert.strictEqual(line, expectedLines[callCount]); + callCount++; + }); + fi.emit('data', expectedLines.join('\r\n')); + assert.strictEqual(callCount, expectedLines.length - 1); + rli.close(); + } + + // \r\n should emit one line event when split across multiple writes. + { + const fi = new FakeInput(); + const rli = new readline.Interface({ + input: fi, + output: fi, + terminal: terminal, + crlfDelay + }); + const expectedLines = ['foo', 'bar', 'baz', 'bat']; + let callCount = 0; + rli.on('line', function(line) { + assert.strictEqual(line, expectedLines[callCount]); + callCount++; + }); + expectedLines.forEach(function(line) { + fi.emit('data', `${line}\r`); + fi.emit('data', '\n'); + }); + assert.strictEqual(callCount, expectedLines.length); + rli.close(); + } + + // Emit one line event when the delay between \r and \n is + // over the default crlfDelay but within the setting value. + { + const fi = new FakeInput(); + const delay = 125; + const rli = new readline.Interface({ + input: fi, + output: fi, + terminal: terminal, + crlfDelay + }); + let callCount = 0; + rli.on('line', () => callCount++); + fi.emit('data', '\r'); + setTimeout(common.mustCall(() => { + fi.emit('data', '\n'); + assert.strictEqual(callCount, 1); + rli.close(); + }), delay); + } +}); diff --git a/test/sequential/test-readline-interface.js b/test/sequential/test-readline-interface.js deleted file mode 100644 index 488dd8338da..00000000000 --- a/test/sequential/test-readline-interface.js +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -// Flags: --expose_internals -'use strict'; -const common = require('../common'); - -// These test cases are in `sequential` rather than the analogous test file in -// `parallel` because they become unreliable under load. The unreliability under -// load was determined empirically when the test cases were in `parallel` by -// running: -// tools/test.py -j 96 --repeat 192 test/parallel/test-readline-interface.js - -const assert = require('assert'); -const readline = require('readline'); -const EventEmitter = require('events').EventEmitter; -const inherits = require('util').inherits; - -function FakeInput() { - EventEmitter.call(this); -} -inherits(FakeInput, EventEmitter); -FakeInput.prototype.resume = () => {}; -FakeInput.prototype.pause = () => {}; -FakeInput.prototype.write = () => {}; -FakeInput.prototype.end = () => {}; - -[ true, false ].forEach(function(terminal) { - // sending multiple newlines at once that does not end with a new line - // and a `end` event(last line is) - - // \r\n should emit one line event, not two - { - const fi = new FakeInput(); - const rli = new readline.Interface( - { input: fi, output: fi, terminal: terminal } - ); - const expectedLines = ['foo', 'bar', 'baz', 'bat']; - let callCount = 0; - rli.on('line', function(line) { - assert.strictEqual(line, expectedLines[callCount]); - callCount++; - }); - fi.emit('data', expectedLines.join('\r\n')); - assert.strictEqual(callCount, expectedLines.length - 1); - rli.close(); - } - - // \r\n should emit one line event when split across multiple writes. - { - const fi = new FakeInput(); - const rli = new readline.Interface( - { input: fi, output: fi, terminal: terminal } - ); - const expectedLines = ['foo', 'bar', 'baz', 'bat']; - let callCount = 0; - rli.on('line', function(line) { - assert.strictEqual(line, expectedLines[callCount]); - callCount++; - }); - expectedLines.forEach(function(line) { - fi.emit('data', `${line}\r`); - fi.emit('data', '\n'); - }); - assert.strictEqual(callCount, expectedLines.length); - rli.close(); - } - - // Emit one line event when the delay between \r and \n is - // over the default crlfDelay but within the setting value. - { - const fi = new FakeInput(); - const delay = 125; - const crlfDelay = common.platformTimeout(1000); - const rli = new readline.Interface({ - input: fi, - output: fi, - terminal: terminal, - crlfDelay - }); - let callCount = 0; - rli.on('line', () => callCount++); - fi.emit('data', '\r'); - setTimeout(common.mustCall(() => { - fi.emit('data', '\n'); - assert.strictEqual(callCount, 1); - rli.close(); - }), delay); - } -}); From d0e4d4e0a1ffd665f24ecd2ae91445adbc2b011d Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Mon, 5 Feb 2018 11:17:07 -0500 Subject: [PATCH 16/18] deps: patch V8 to 6.4.388.42 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/18578 Refs: https://github.com/v8/v8/compare/6.4.388.41...6.4.388.42 Reviewed-By: Michaël Zasso Reviewed-By: Colin Ihrig Reviewed-By: Ruben Bridgewater --- deps/v8/include/v8-version.h | 2 +- deps/v8/src/frames.cc | 9 ++++ deps/v8/src/frames.h | 1 + .../v8/src/profiler/sampling-heap-profiler.cc | 23 +++++++-- deps/v8/test/cctest/test-heap-profiler.cc | 48 ++++++++++++++++++- 5 files changed, 77 insertions(+), 6 deletions(-) diff --git a/deps/v8/include/v8-version.h b/deps/v8/include/v8-version.h index dafa83627b2..8c4bc023770 100644 --- a/deps/v8/include/v8-version.h +++ b/deps/v8/include/v8-version.h @@ -11,7 +11,7 @@ #define V8_MAJOR_VERSION 6 #define V8_MINOR_VERSION 4 #define V8_BUILD_NUMBER 388 -#define V8_PATCH_LEVEL 41 +#define V8_PATCH_LEVEL 42 // Use 1 for candidates and 0 otherwise. // (Boolean macro values are not supported by all preprocessors.) diff --git a/deps/v8/src/frames.cc b/deps/v8/src/frames.cc index bd1abda4de8..23713197f59 100644 --- a/deps/v8/src/frames.cc +++ b/deps/v8/src/frames.cc @@ -1013,6 +1013,15 @@ JSFunction* JavaScriptFrame::function() const { return JSFunction::cast(function_slot_object()); } +Object* JavaScriptFrame::unchecked_function() const { + // During deoptimization of an optimized function, we may have yet to + // materialize some closures on the stack. The arguments marker object + // marks this case. + DCHECK(function_slot_object()->IsJSFunction() || + isolate()->heap()->arguments_marker() == function_slot_object()); + return function_slot_object(); +} + Object* JavaScriptFrame::receiver() const { return GetParameter(-1); } Object* JavaScriptFrame::context() const { diff --git a/deps/v8/src/frames.h b/deps/v8/src/frames.h index a72832af06d..e21d62764b3 100644 --- a/deps/v8/src/frames.h +++ b/deps/v8/src/frames.h @@ -684,6 +684,7 @@ class JavaScriptFrame : public StandardFrame { // Accessors. virtual JSFunction* function() const; + Object* unchecked_function() const; Object* receiver() const override; Object* context() const override; Script* script() const override; diff --git a/deps/v8/src/profiler/sampling-heap-profiler.cc b/deps/v8/src/profiler/sampling-heap-profiler.cc index 0aa0525633c..51fe8866fa1 100644 --- a/deps/v8/src/profiler/sampling-heap-profiler.cc +++ b/deps/v8/src/profiler/sampling-heap-profiler.cc @@ -157,12 +157,21 @@ SamplingHeapProfiler::AllocationNode* SamplingHeapProfiler::AddStack() { std::vector stack; JavaScriptFrameIterator it(isolate_); int frames_captured = 0; + bool found_arguments_marker_frames = false; while (!it.done() && frames_captured < stack_depth_) { JavaScriptFrame* frame = it.frame(); - SharedFunctionInfo* shared = frame->function()->shared(); - stack.push_back(shared); - - frames_captured++; + // If we are materializing objects during deoptimization, inlined + // closures may not yet be materialized, and this includes the + // closure on the stack. Skip over any such frames (they'll be + // in the top frames of the stack). The allocations made in this + // sensitive moment belong to the formerly optimized frame anyway. + if (frame->unchecked_function()->IsJSFunction()) { + SharedFunctionInfo* shared = frame->function()->shared(); + stack.push_back(shared); + frames_captured++; + } else { + found_arguments_marker_frames = true; + } it.Advance(); } @@ -209,6 +218,12 @@ SamplingHeapProfiler::AllocationNode* SamplingHeapProfiler::AddStack() { } node = node->FindOrAddChildNode(name, script_id, shared->start_position()); } + + if (found_arguments_marker_frames) { + node = + node->FindOrAddChildNode("(deopt)", v8::UnboundScript::kNoScriptId, 0); + } + return node; } diff --git a/deps/v8/test/cctest/test-heap-profiler.cc b/deps/v8/test/cctest/test-heap-profiler.cc index ce015777e5d..a0796ccd56c 100644 --- a/deps/v8/test/cctest/test-heap-profiler.cc +++ b/deps/v8/test/cctest/test-heap-profiler.cc @@ -3083,7 +3083,7 @@ TEST(SamplingHeapProfilerPretenuredInlineAllocations) { // Suppress randomness to avoid flakiness in tests. v8::internal::FLAG_sampling_heap_profiler_suppress_randomness = true; - // Grow new space unitl maximum capacity reached. + // Grow new space until maximum capacity reached. while (!CcTest::heap()->new_space()->IsAtMaximumCapacity()) { CcTest::heap()->new_space()->Grow(); } @@ -3138,3 +3138,49 @@ TEST(SamplingHeapProfilerPretenuredInlineAllocations) { CHECK_GE(count, 8000); } + +TEST(SamplingHeapProfilerSampleDuringDeopt) { + i::FLAG_allow_natives_syntax = true; + + v8::HandleScope scope(v8::Isolate::GetCurrent()); + LocalContext env; + v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler(); + + // Suppress randomness to avoid flakiness in tests. + v8::internal::FLAG_sampling_heap_profiler_suppress_randomness = true; + + // Small sample interval to force each object to be sampled. + heap_profiler->StartSamplingHeapProfiler(i::kPointerSize); + + // Lazy deopt from runtime call from inlined callback function. + const char* source = + "var b = " + " [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25];" + "(function f() {" + " var result = 0;" + " var lazyDeopt = function(deopt) {" + " var callback = function(v,i,o) {" + " result += i;" + " if (i == 13 && deopt) {" + " %DeoptimizeNow();" + " }" + " return v;" + " };" + " b.map(callback);" + " };" + " lazyDeopt();" + " lazyDeopt();" + " %OptimizeFunctionOnNextCall(lazyDeopt);" + " lazyDeopt();" + " lazyDeopt(true);" + " lazyDeopt();" + "})();"; + + CompileRun(source); + // Should not crash. + + std::unique_ptr profile( + heap_profiler->GetAllocationProfile()); + CHECK(profile); + heap_profiler->StopSamplingHeapProfiler(); +} From 1573e4563a0d3f6c08a1dd3ab3d161bece532db5 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 4 Feb 2018 11:48:21 -0500 Subject: [PATCH 17/18] src: move GetNow to Environment PR-URL: https://github.com/nodejs/node/pull/18562 Reviewed-By: Ben Noordhuis Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- src/env.cc | 15 +++++++++++++++ src/env.h | 2 ++ src/timer_wrap.cc | 16 ++-------------- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/env.cc b/src/env.cc index 1b3323eb26c..07e37498c89 100644 --- a/src/env.cc +++ b/src/env.cc @@ -11,13 +11,16 @@ namespace node { using v8::Context; using v8::FunctionTemplate; using v8::HandleScope; +using v8::Integer; using v8::Isolate; using v8::Local; using v8::Message; +using v8::Number; using v8::Private; using v8::StackFrame; using v8::StackTrace; using v8::String; +using v8::Value; IsolateData::IsolateData(Isolate* isolate, uv_loop_t* event_loop, @@ -362,6 +365,18 @@ void Environment::ToggleImmediateRef(bool ref) { } +Local Environment::GetNow() { + uv_update_time(event_loop()); + uint64_t now = uv_now(event_loop()); + CHECK_GE(now, timer_base()); + now -= timer_base(); + if (now <= 0xffffffff) + return Integer::New(isolate(), static_cast(now)); + else + return Number::New(isolate(), static_cast(now)); +} + + void CollectExceptionInfo(Environment* env, v8::Local obj, int errorno, diff --git a/src/env.h b/src/env.h index 4701b58e8a9..b621a54e378 100644 --- a/src/env.h +++ b/src/env.h @@ -735,6 +735,8 @@ class Environment { static inline Environment* ForAsyncHooks(AsyncHooks* hooks); + v8::Local GetNow(); + private: inline void CreateImmediate(native_immediate_callback cb, void* data, diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc index f1b423d3669..441974ae77b 100644 --- a/src/timer_wrap.cc +++ b/src/timer_wrap.cc @@ -37,7 +37,6 @@ using v8::FunctionTemplate; using v8::HandleScope; using v8::Integer; using v8::Local; -using v8::Number; using v8::Object; using v8::String; using v8::Value; @@ -142,7 +141,7 @@ class TimerWrap : public HandleWrap { Local ret; Local args[1]; do { - args[0] = GetNow(env); + args[0] = env->GetNow(); ret = wrap->MakeCallback(kOnTimeout, 1, args).ToLocalChecked(); } while (ret->IsUndefined() && !env->tick_info()->has_thrown() && @@ -153,18 +152,7 @@ class TimerWrap : public HandleWrap { static void Now(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - args.GetReturnValue().Set(GetNow(env)); - } - - static Local GetNow(Environment* env) { - uv_update_time(env->event_loop()); - uint64_t now = uv_now(env->event_loop()); - CHECK(now >= env->timer_base()); - now -= env->timer_base(); - if (now <= 0xfffffff) - return Integer::New(env->isolate(), static_cast(now)); - else - return Number::New(env->isolate(), static_cast(now)); + args.GetReturnValue().Set(env->GetNow()); } uv_timer_t handle_; From 92ba624fa1da61c4061aa554877bfa3c94c83324 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 4 Feb 2018 11:51:18 -0500 Subject: [PATCH 18/18] tls: provide now value from C++ Instead of separately calling into C++ from JS to retrieve the Timer.now() value, pass it in as an argument. PR-URL: https://github.com/nodejs/node/pull/18562 Reviewed-By: Ben Noordhuis Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- lib/_tls_wrap.js | 8 +++----- src/tls_wrap.cc | 3 ++- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 16d8b24c297..fcd447bb590 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -32,7 +32,6 @@ const common = require('_tls_common'); const { StreamWrap } = require('_stream_wrap'); const { Buffer } = require('buffer'); const debug = util.debuglog('tls'); -const { Timer } = process.binding('timer_wrap'); const tls_wrap = process.binding('tls_wrap'); const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap'); @@ -49,14 +48,13 @@ const kSNICallback = Symbol('snicallback'); const noop = () => {}; -function onhandshakestart() { +function onhandshakestart(now) { debug('onhandshakestart'); - const owner = this.owner; - const now = Timer.now(); - assert(now >= this.lastHandshakeTime); + const owner = this.owner; + if ((now - this.lastHandshakeTime) >= tls.CLIENT_RENEG_WINDOW * 1000) { this.handshakes = 0; } diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 971dbb857f7..0cba1898fba 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -230,7 +230,8 @@ void TLSWrap::SSLInfoCallback(const SSL* ssl_, int where, int ret) { if (where & SSL_CB_HANDSHAKE_START) { Local callback = object->Get(env->onhandshakestart_string()); if (callback->IsFunction()) { - c->MakeCallback(callback.As(), 0, nullptr); + Local argv[] = { env->GetNow() }; + c->MakeCallback(callback.As(), arraysize(argv), argv); } }