From bfbce289c33b12aafb82bd5b45bcb4412850b28f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 18 Mar 2019 02:29:39 +0100 Subject: [PATCH] lib: refactor Error.captureStackTrace() usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using `Errors.captureStackFrames` the error's stack property is set again. This adds a helper function that wraps this functionality in a simple API that does not only set the stack including the `code` property but it also improves the performance to create the error. The helper works for thrown errors and errors returned from wrapped functions in case they are Node.js core errors. PR-URL: https://github.com/nodejs/node/pull/26738 Fixes: https://github.com/nodejs/node/issues/26669 Fixes: https://github.com/nodejs/node/issues/20253 Reviewed-By: Gus Caplan Reviewed-By: Matteo Collina Reviewed-By: Michaël Zasso Reviewed-By: Joyee Cheung --- lib/_http_outgoing.js | 59 +++++++----------- lib/buffer.js | 37 ++++++------ lib/fs.js | 21 ++++--- lib/internal/crypto/pbkdf2.js | 4 +- lib/internal/errors.js | 79 +++++++++++++++++------- lib/internal/fs/utils.js | 109 ++++++++++++++-------------------- lib/internal/http2/compat.js | 49 +++++++-------- lib/internal/http2/core.js | 31 ++++------ lib/internal/http2/util.js | 66 ++++++++++---------- lib/internal/util/inspect.js | 1 + lib/internal/validators.js | 97 ++++++++++++++---------------- lib/os.js | 15 +++-- lib/util.js | 29 ++++----- lib/zlib.js | 49 ++++++++------- test/common/wpt.js | 5 -- 15 files changed, 314 insertions(+), 337 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 9d308525c6e681..ba5d226e7b7bda 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -34,16 +34,19 @@ const { symbols: { async_id_symbol } } = require('internal/async_hooks'); const { - ERR_HTTP_HEADERS_SENT, - ERR_HTTP_INVALID_HEADER_VALUE, - ERR_HTTP_TRAILER_INVALID, - ERR_INVALID_HTTP_TOKEN, - ERR_INVALID_ARG_TYPE, - ERR_INVALID_CHAR, - ERR_METHOD_NOT_IMPLEMENTED, - ERR_STREAM_CANNOT_PIPE, - ERR_STREAM_WRITE_AFTER_END -} = require('internal/errors').codes; + codes: { + ERR_HTTP_HEADERS_SENT, + ERR_HTTP_INVALID_HEADER_VALUE, + ERR_HTTP_TRAILER_INVALID, + ERR_INVALID_HTTP_TOKEN, + ERR_INVALID_ARG_TYPE, + ERR_INVALID_CHAR, + ERR_METHOD_NOT_IMPLEMENTED, + ERR_STREAM_CANNOT_PIPE, + ERR_STREAM_WRITE_AFTER_END + }, + hideStackFrames +} = require('internal/errors'); const { validateString } = require('internal/validators'); const { CRLF, debug } = common; @@ -443,39 +446,21 @@ function matchHeader(self, state, field, value) { } } -function validateHeaderName(name) { +const validateHeaderName = hideStackFrames((name) => { if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) { - // Reducing the limit improves the performance significantly. We do not - // lose the stack frames due to the `captureStackTrace()` function that is - // called later. - const tmpLimit = Error.stackTraceLimit; - Error.stackTraceLimit = 0; - const err = new ERR_INVALID_HTTP_TOKEN('Header name', name); - Error.stackTraceLimit = tmpLimit; - Error.captureStackTrace(err, validateHeaderName); - throw err; + throw new ERR_INVALID_HTTP_TOKEN('Header name', name); } -} +}); -function validateHeaderValue(name, value) { - let err; - // Reducing the limit improves the performance significantly. We do not loose - // the stack frames due to the `captureStackTrace()` function that is called - // later. - const tmpLimit = Error.stackTraceLimit; - Error.stackTraceLimit = 0; +const validateHeaderValue = hideStackFrames((name, value) => { if (value === undefined) { - err = new ERR_HTTP_INVALID_HEADER_VALUE(value, name); - } else if (checkInvalidHeaderChar(value)) { - debug('Header "%s" contains invalid characters', name); - err = new ERR_INVALID_CHAR('header content', name); + throw new ERR_HTTP_INVALID_HEADER_VALUE(value, name); } - Error.stackTraceLimit = tmpLimit; - if (err !== undefined) { - Error.captureStackTrace(err, validateHeaderValue); - throw err; + if (checkInvalidHeaderChar(value)) { + debug('Header "%s" contains invalid characters', name); + throw new ERR_INVALID_CHAR('header content', name); } -} +}); OutgoingMessage.prototype.setHeader = function setHeader(name, value) { if (this._header) { diff --git a/lib/buffer.js b/lib/buffer.js index c5497e18aa66d3..b072ac407d6ffc 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -62,15 +62,18 @@ const { } = require('internal/util/inspect'); const { - ERR_BUFFER_OUT_OF_BOUNDS, - ERR_OUT_OF_RANGE, - ERR_INVALID_ARG_TYPE, - ERR_INVALID_ARG_VALUE, - ERR_INVALID_BUFFER_SIZE, - ERR_INVALID_OPT_VALUE, - ERR_NO_LONGER_SUPPORTED, - ERR_UNKNOWN_ENCODING -} = require('internal/errors').codes; + codes: { + ERR_BUFFER_OUT_OF_BOUNDS, + ERR_OUT_OF_RANGE, + ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, + ERR_INVALID_BUFFER_SIZE, + ERR_INVALID_OPT_VALUE, + ERR_NO_LONGER_SUPPORTED, + ERR_UNKNOWN_ENCODING + }, + hideStackFrames +} = require('internal/errors'); const { validateString } = require('internal/validators'); const { @@ -247,20 +250,14 @@ Object.setPrototypeOf(Buffer, Uint8Array); // The 'assertSize' method will remove itself from the callstack when an error // occurs. This is done simply to keep the internal details of the // implementation from bleeding out to users. -function assertSize(size) { - let err = null; - +const assertSize = hideStackFrames((size) => { if (typeof size !== 'number') { - err = new ERR_INVALID_ARG_TYPE('size', 'number', size); - } else if (!(size >= 0 && size <= kMaxLength)) { - err = new ERR_INVALID_OPT_VALUE.RangeError('size', size); + throw new ERR_INVALID_ARG_TYPE('size', 'number', size); } - - if (err !== null) { - Error.captureStackTrace(err, assertSize); - throw err; + if (!(size >= 0 && size <= kMaxLength)) { + throw new ERR_INVALID_OPT_VALUE.RangeError('size', size); } -} +}); /** * Creates a new filled Buffer instance. diff --git a/lib/fs.js b/lib/fs.js index 46f26fe9ebc69c..46dd447d3efd5b 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -43,13 +43,15 @@ const pathModule = require('path'); const { isArrayBufferView } = require('internal/util/types'); const binding = internalBinding('fs'); const { Buffer, kMaxLength } = require('buffer'); -const errors = require('internal/errors'); const { - ERR_FS_FILE_TOO_LARGE, - ERR_INVALID_ARG_VALUE, - ERR_INVALID_ARG_TYPE, - ERR_INVALID_CALLBACK -} = errors.codes; + codes: { + ERR_FS_FILE_TOO_LARGE, + ERR_INVALID_ARG_VALUE, + ERR_INVALID_ARG_TYPE, + ERR_INVALID_CALLBACK + }, + uvException +} = require('internal/errors'); const { FSReqCallback, statValues } = binding; const { toPathIfFileURL } = require('internal/url'); @@ -114,10 +116,11 @@ function showTruncateDeprecation() { function handleErrorFromBinding(ctx) { if (ctx.errno !== undefined) { // libuv error numbers - const err = errors.uvException(ctx); + const err = uvException(ctx); Error.captureStackTrace(err, handleErrorFromBinding); throw err; - } else if (ctx.error !== undefined) { // errors created in C++ land. + } + if (ctx.error !== undefined) { // errors created in C++ land. // TODO(joyeecheung): currently, ctx.error are encoding errors // usually caused by memory problems. We need to figure out proper error // code(s) for this. @@ -310,7 +313,7 @@ function tryStatSync(fd, isUserFd) { const stats = binding.fstat(fd, false, undefined, ctx); if (ctx.errno !== undefined && !isUserFd) { fs.closeSync(fd); - throw errors.uvException(ctx); + throw uvException(ctx); } return stats; } diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js index e1a7a4811a2159..4694c6ce9a460f 100644 --- a/lib/internal/crypto/pbkdf2.js +++ b/lib/internal/crypto/pbkdf2.js @@ -65,8 +65,8 @@ function check(password, salt, iterations, keylen, digest) { password = validateArrayBufferView(password, 'password'); salt = validateArrayBufferView(salt, 'salt'); - iterations = validateUint32(iterations, 'iterations', 0); - keylen = validateUint32(keylen, 'keylen', 0); + validateUint32(iterations, 'iterations', 0); + validateUint32(keylen, 'keylen', 0); return { password, salt, iterations, keylen, digest }; } diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 536778db431df5..0112eb6278594a 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -18,7 +18,7 @@ const codes = {}; const { kMaxLength } = internalBinding('buffer'); const { defineProperty } = Object; -let useOriginalName = false; +let excludedStackFn; // Lazily loaded let util; @@ -49,7 +49,15 @@ function lazyBuffer() { // and may have .path and .dest. class SystemError extends Error { constructor(key, context) { - super(); + if (excludedStackFn === undefined) { + super(); + } else { + const limit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + super(); + // Reset the limit and setting the name property. + Error.stackTraceLimit = limit; + } const prefix = getMessage(key, [], this); let message = `${prefix}: ${context.syscall} returned ` + `${context.code} (${context.message})`; @@ -148,7 +156,15 @@ function makeSystemErrorWithCode(key) { function makeNodeErrorWithCode(Base, key) { return class NodeError extends Base { constructor(...args) { - super(); + if (excludedStackFn === undefined) { + super(); + } else { + const limit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + super(); + // Reset the limit and setting the name property. + Error.stackTraceLimit = limit; + } const message = getMessage(key, args, this); Object.defineProperty(this, 'message', { value: message, @@ -178,9 +194,30 @@ function makeNodeErrorWithCode(Base, key) { }; } +// This function removes unnecessary frames from Node.js core errors. +function hideStackFrames(fn) { + return function hidden(...args) { + // Make sure the most outer `hideStackFrames()` function is used. + let setStackFn = false; + if (excludedStackFn === undefined) { + excludedStackFn = hidden; + setStackFn = true; + } + try { + return fn(...args); + } finally { + if (setStackFn === true) { + excludedStackFn = undefined; + } + } + }; +} + function addCodeToName(err, name, code) { - if (useOriginalName) { - return; + // Set the stack + if (excludedStackFn !== undefined) { + // eslint-disable-next-line no-restricted-syntax + Error.captureStackTrace(err, excludedStackFn); } // Add the error code to the name to include it in the stack trace. err.name = `${name} [${code}]`; @@ -308,6 +345,7 @@ function uvException(ctx) { err[prop] = ctx[prop]; } + // TODO(BridgeAR): Show the `code` property as part of the stack. err.code = code; if (path) { err.path = path; @@ -316,7 +354,7 @@ function uvException(ctx) { err.dest = dest; } - Error.captureStackTrace(err, uvException); + Error.captureStackTrace(err, excludedStackFn || uvException); return err; } @@ -358,7 +396,7 @@ function uvExceptionWithHostPort(err, syscall, address, port) { ex.port = port; } - Error.captureStackTrace(ex, uvExceptionWithHostPort); + Error.captureStackTrace(ex, excludedStackFn || uvExceptionWithHostPort); return ex; } @@ -386,7 +424,7 @@ function errnoException(err, syscall, original) { ex.code = ex.errno = code; ex.syscall = syscall; - Error.captureStackTrace(ex, errnoException); + Error.captureStackTrace(ex, excludedStackFn || errnoException); return ex; } @@ -434,7 +472,7 @@ function exceptionWithHostPort(err, syscall, address, port, additional) { ex.port = port; } - Error.captureStackTrace(ex, exceptionWithHostPort); + Error.captureStackTrace(ex, excludedStackFn || exceptionWithHostPort); return ex; } @@ -473,7 +511,8 @@ function dnsException(code, syscall, hostname) { if (hostname) { ex.hostname = hostname; } - Error.captureStackTrace(ex, dnsException); + + Error.captureStackTrace(ex, excludedStackFn || dnsException); return ex; } @@ -523,21 +562,19 @@ function oneOf(expected, thing) { } module.exports = { + addCodeToName, // Exported for NghttpError + codes, dnsException, errnoException, exceptionWithHostPort, + getMessage, + hideStackFrames, + isStackOverflowError, uvException, uvExceptionWithHostPort, - isStackOverflowError, - getMessage, SystemError, - codes, // This is exported only to facilitate testing. - E, - // This allows us to tell the type of the errors without using - // instanceof, which is necessary in WPT harness. - get useOriginalName() { return useOriginalName; }, - set useOriginalName(value) { useOriginalName = value; } + E }; // To declare an error message, use the E(sym, val, def) function above. The sym @@ -556,7 +593,6 @@ module.exports = { // Note: Please try to keep these in alphabetical order // // Note: Node.js specific errors must begin with the prefix ERR_ - E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError); E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError); E('ERR_ASSERTION', '%s', Error); @@ -630,7 +666,10 @@ E('ERR_ENCODING_INVALID_ENCODED_DATA', function(encoding, ret) { }, TypeError); E('ERR_ENCODING_NOT_SUPPORTED', 'The "%s" encoding is not supported', RangeError); -E('ERR_FALSY_VALUE_REJECTION', 'Promise was rejected with falsy value', Error); +E('ERR_FALSY_VALUE_REJECTION', function(reason) { + this.reason = reason; + return 'Promise was rejected with falsy value'; +}, Error); E('ERR_FS_FILE_TOO_LARGE', 'File size (%s) is greater than possible Buffer: ' + `${kMaxLength} bytes`, RangeError); diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 0c5ce1ecb76b91..a0b3eec5f71949 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -2,13 +2,16 @@ const { Buffer, kMaxLength } = require('buffer'); const { - ERR_FS_INVALID_SYMLINK_TYPE, - ERR_INVALID_ARG_TYPE, - ERR_INVALID_ARG_VALUE, - ERR_INVALID_OPT_VALUE, - ERR_INVALID_OPT_VALUE_ENCODING, - ERR_OUT_OF_RANGE -} = require('internal/errors').codes; + codes: { + ERR_FS_INVALID_SYMLINK_TYPE, + ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, + ERR_INVALID_OPT_VALUE, + ERR_INVALID_OPT_VALUE_ENCODING, + ERR_OUT_OF_RANGE + }, + hideStackFrames +} = require('internal/errors'); const { isUint8Array, isArrayBufferView } = require('internal/util/types'); const { once } = require('internal/util'); const pathModule = require('path'); @@ -185,7 +188,7 @@ function getOptions(options, defaultOptions) { // Check if the path contains null types if it is a string nor Uint8Array, // otherwise return silently. -function nullCheck(path, propName, throwError = true) { +const nullCheck = hideStackFrames((path, propName, throwError = true) => { const pathIsString = typeof path === 'string'; const pathIsUint8Array = isUint8Array(path); @@ -196,26 +199,16 @@ function nullCheck(path, propName, throwError = true) { return; } - // Reducing the limit improves the performance significantly. We do not loose - // the stack frames due to the `captureStackTrace()` function that is called - // later. - const tmpLimit = Error.stackTraceLimit; - if (throwError) { - Error.stackTraceLimit = 0; - } const err = new ERR_INVALID_ARG_VALUE( propName, path, 'must be a string or Uint8Array without null bytes' ); - if (throwError) { - Error.stackTraceLimit = tmpLimit; - Error.captureStackTrace(err, nullCheck); throw err; } return err; -} +}); function preprocessSymlinkDestination(path, type, linkPath) { if (!isWindows) { @@ -359,7 +352,7 @@ function stringToFlags(flags) { throw new ERR_INVALID_OPT_VALUE('flags', flags); } -function stringToSymlinkType(type) { +const stringToSymlinkType = hideStackFrames((type) => { let flags = 0; if (typeof type === 'string') { switch (type) { @@ -372,13 +365,11 @@ function stringToSymlinkType(type) { case 'file': break; default: - const err = new ERR_FS_INVALID_SYMLINK_TYPE(type); - Error.captureStackTrace(err, stringToSymlinkType); - throw err; + throw new ERR_FS_INVALID_SYMLINK_TYPE(type); } } return flags; -} +}); // converts Date or number to a fractional UNIX timestamp function toUnixTimestamp(time, name = 'time') { @@ -399,65 +390,51 @@ function toUnixTimestamp(time, name = 'time') { throw new ERR_INVALID_ARG_TYPE(name, ['Date', 'Time in seconds'], time); } -function validateBuffer(buffer) { +const validateBuffer = hideStackFrames((buffer) => { if (!isArrayBufferView(buffer)) { - const err = new ERR_INVALID_ARG_TYPE('buffer', - ['Buffer', 'TypedArray', 'DataView'], - buffer); - Error.captureStackTrace(err, validateBuffer); - throw err; + throw new ERR_INVALID_ARG_TYPE('buffer', + ['Buffer', 'TypedArray', 'DataView'], + buffer); } -} +}); -function validateOffsetLengthRead(offset, length, bufferLength) { - let err; - - if (offset < 0 || offset >= bufferLength) { - err = new ERR_OUT_OF_RANGE('offset', - `>= 0 && <= ${bufferLength}`, offset); - } else if (length < 0 || offset + length > bufferLength) { - err = new ERR_OUT_OF_RANGE('length', - `>= 0 && <= ${bufferLength - offset}`, length); - } - - if (err !== undefined) { - Error.captureStackTrace(err, validateOffsetLengthRead); - throw err; +const validateOffsetLengthRead = hideStackFrames( + (offset, length, bufferLength) => { + if (offset < 0 || offset >= bufferLength) { + throw new ERR_OUT_OF_RANGE('offset', + `>= 0 && <= ${bufferLength}`, offset); + } + if (length < 0 || offset + length > bufferLength) { + throw new ERR_OUT_OF_RANGE('length', + `>= 0 && <= ${bufferLength - offset}`, length); + } } -} +); -function validateOffsetLengthWrite(offset, length, byteLength) { - let err; +const validateOffsetLengthWrite = hideStackFrames( + (offset, length, byteLength) => { + if (offset > byteLength) { + throw new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset); + } - if (offset > byteLength) { - err = new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset); - } else { const max = byteLength > kMaxLength ? kMaxLength : byteLength; if (length > max - offset) { - err = new ERR_OUT_OF_RANGE('length', `<= ${max - offset}`, length); + throw new ERR_OUT_OF_RANGE('length', `<= ${max - offset}`, length); } } +); - if (err !== undefined) { - Error.captureStackTrace(err, validateOffsetLengthWrite); - throw err; - } -} - -function validatePath(path, propName = 'path') { - let err; - +const validatePath = hideStackFrames((path, propName = 'path') => { if (typeof path !== 'string' && !isUint8Array(path)) { - err = new ERR_INVALID_ARG_TYPE(propName, ['string', 'Buffer', 'URL'], path); - } else { - err = nullCheck(path, propName, false); + throw new ERR_INVALID_ARG_TYPE(propName, ['string', 'Buffer', 'URL'], path); } + const err = nullCheck(path, propName, false); + if (err !== undefined) { - Error.captureStackTrace(err, validatePath); throw err; } -} +}); module.exports = { assertEncoding, diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 551c9f1efa6bba..23266eb6c2d469 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -6,17 +6,20 @@ const Readable = Stream.Readable; const binding = internalBinding('http2'); const constants = binding.constants; const { - ERR_HTTP2_HEADERS_SENT, - ERR_HTTP2_INFO_STATUS_NOT_ALLOWED, - ERR_HTTP2_INVALID_HEADER_VALUE, - ERR_HTTP2_INVALID_STREAM, - ERR_HTTP2_NO_SOCKET_MANIPULATION, - ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED, - ERR_HTTP2_STATUS_INVALID, - ERR_INVALID_ARG_VALUE, - ERR_INVALID_CALLBACK, - ERR_INVALID_HTTP_TOKEN -} = require('internal/errors').codes; + codes: { + ERR_HTTP2_HEADERS_SENT, + ERR_HTTP2_INFO_STATUS_NOT_ALLOWED, + ERR_HTTP2_INVALID_HEADER_VALUE, + ERR_HTTP2_INVALID_STREAM, + ERR_HTTP2_NO_SOCKET_MANIPULATION, + ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED, + ERR_HTTP2_STATUS_INVALID, + ERR_INVALID_ARG_VALUE, + ERR_INVALID_CALLBACK, + ERR_INVALID_HTTP_TOKEN + }, + hideStackFrames +} = require('internal/errors'); const { validateString } = require('internal/validators'); const { kSocket, kRequest, kProxySocket } = require('internal/http2/util'); @@ -51,22 +54,20 @@ let statusConnectionHeaderWarned = false; // HTTP/2 implementation, intended to provide an interface that is as // close as possible to the current require('http') API -function assertValidHeader(name, value) { - let err; +const assertValidHeader = hideStackFrames((name, value) => { if (name === '' || typeof name !== 'string') { - err = new ERR_INVALID_HTTP_TOKEN('Header name', name); - } else if (isPseudoHeader(name)) { - err = new ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED(); - } else if (value === undefined || value === null) { - err = new ERR_HTTP2_INVALID_HEADER_VALUE(value, name); - } else if (!isConnectionHeaderAllowed(name, value)) { - connectionHeaderMessageWarn(); + throw new ERR_INVALID_HTTP_TOKEN('Header name', name); } - if (err !== undefined) { - Error.captureStackTrace(err, assertValidHeader); - throw err; + if (isPseudoHeader(name)) { + throw new ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED(); } -} + if (value === undefined || value === null) { + throw new ERR_HTTP2_INVALID_HEADER_VALUE(value, name); + } + if (!isConnectionHeaderAllowed(name, value)) { + connectionHeaderMessageWarn(); + } +}); function isPseudoHeader(name) { switch (name) { diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 39e9dd625a505f..393970cf470eba 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -76,7 +76,8 @@ const { ERR_INVALID_OPT_VALUE, ERR_OUT_OF_RANGE, ERR_SOCKET_CLOSED - } + }, + hideStackFrames } = require('internal/errors'); const { validateNumber, validateString } = require('internal/validators'); const { utcDate } = require('internal/http'); @@ -606,37 +607,31 @@ function requestOnConnect(headers, options) { // 4. if specified, options.silent must be a boolean // // Also sets the default priority options if they are not set. -function validatePriorityOptions(options) { - let err; +const validatePriorityOptions = hideStackFrames((options) => { if (options.weight === undefined) { options.weight = NGHTTP2_DEFAULT_WEIGHT; } else if (typeof options.weight !== 'number') { - err = new ERR_INVALID_OPT_VALUE('weight', options.weight); + throw new ERR_INVALID_OPT_VALUE('weight', options.weight); } if (options.parent === undefined) { options.parent = 0; } else if (typeof options.parent !== 'number' || options.parent < 0) { - err = new ERR_INVALID_OPT_VALUE('parent', options.parent); + throw new ERR_INVALID_OPT_VALUE('parent', options.parent); } if (options.exclusive === undefined) { options.exclusive = false; } else if (typeof options.exclusive !== 'boolean') { - err = new ERR_INVALID_OPT_VALUE('exclusive', options.exclusive); + throw new ERR_INVALID_OPT_VALUE('exclusive', options.exclusive); } if (options.silent === undefined) { options.silent = false; } else if (typeof options.silent !== 'boolean') { - err = new ERR_INVALID_OPT_VALUE('silent', options.silent); - } - - if (err) { - Error.captureStackTrace(err, validatePriorityOptions); - throw err; + throw new ERR_INVALID_OPT_VALUE('silent', options.silent); } -} +}); // When an error occurs internally at the binding level, immediately // destroy the session. @@ -788,7 +783,7 @@ function pingCallback(cb) { // 5. maxHeaderListSize must be a number in the range 0 <= n <= kMaxInt // 6. enablePush must be a boolean // All settings are optional and may be left undefined -function validateSettings(settings) { +const validateSettings = hideStackFrames((settings) => { settings = { ...settings }; assertWithinRange('headerTableSize', settings.headerTableSize, @@ -807,13 +802,11 @@ function validateSettings(settings) { 0, kMaxInt); if (settings.enablePush !== undefined && typeof settings.enablePush !== 'boolean') { - const err = new ERR_HTTP2_INVALID_SETTING_VALUE('enablePush', - settings.enablePush); - Error.captureStackTrace(err, 'validateSettings'); - throw err; + throw new ERR_HTTP2_INVALID_SETTING_VALUE('enablePush', + settings.enablePush); } return settings; -} +}); // Creates the internal binding.Http2Session handle for an Http2Session // instance. This occurs only after the socket connection has been diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index d1a2a2b9adcf67..80422cdc8b8dc0 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -2,12 +2,16 @@ const binding = internalBinding('http2'); const { - ERR_HTTP2_HEADER_SINGLE_VALUE, - ERR_HTTP2_INVALID_CONNECTION_HEADERS, - ERR_HTTP2_INVALID_PSEUDOHEADER, - ERR_HTTP2_INVALID_SETTING_VALUE, - ERR_INVALID_ARG_TYPE -} = require('internal/errors').codes; + codes: { + ERR_HTTP2_HEADER_SINGLE_VALUE, + ERR_HTTP2_INVALID_CONNECTION_HEADERS, + ERR_HTTP2_INVALID_PSEUDOHEADER, + ERR_HTTP2_INVALID_SETTING_VALUE, + ERR_INVALID_ARG_TYPE + }, + addCodeToName, + hideStackFrames +} = require('internal/errors'); const kSocket = Symbol('socket'); const kProxySocket = Symbol('proxySocket'); @@ -404,27 +408,21 @@ function isIllegalConnectionSpecificHeader(name, value) { } } -function assertValidPseudoHeader(key) { +const assertValidPseudoHeader = hideStackFrames((key) => { if (!kValidPseudoHeaders.has(key)) { - const err = new ERR_HTTP2_INVALID_PSEUDOHEADER(key); - Error.captureStackTrace(err, assertValidPseudoHeader); - throw err; + throw new ERR_HTTP2_INVALID_PSEUDOHEADER(key); } -} +}); -function assertValidPseudoHeaderResponse(key) { +const assertValidPseudoHeaderResponse = hideStackFrames((key) => { if (key !== ':status') { - const err = new ERR_HTTP2_INVALID_PSEUDOHEADER(key); - Error.captureStackTrace(err, assertValidPseudoHeaderResponse); - throw err; + throw new ERR_HTTP2_INVALID_PSEUDOHEADER(key); } -} +}); -function assertValidPseudoHeaderTrailer(key) { - const err = new ERR_HTTP2_INVALID_PSEUDOHEADER(key); - Error.captureStackTrace(err, assertValidPseudoHeaderTrailer); - throw err; -} +const assertValidPseudoHeaderTrailer = hideStackFrames((key) => { + throw new ERR_HTTP2_INVALID_PSEUDOHEADER(key); +}); function mapToHeaders(map, assertValuePseudoHeader = assertValidPseudoHeader) { @@ -496,10 +494,8 @@ class NghttpError extends Error { constructor(ret) { super(binding.nghttp2ErrorString(ret)); this.code = 'ERR_HTTP2_ERROR'; - this.name = 'Error [ERR_HTTP2_ERROR]'; this.errno = ret; - this.stack; - delete this.name; + addCodeToName(this, super.name, 'ERR_HTTP2_ERROR'); } toString() { @@ -507,26 +503,24 @@ class NghttpError extends Error { } } -function assertIsObject(value, name, types) { +const assertIsObject = hideStackFrames((value, name, types) => { if (value !== undefined && (value === null || typeof value !== 'object' || Array.isArray(value))) { - const err = new ERR_INVALID_ARG_TYPE(name, types || 'Object', value); - Error.captureStackTrace(err, assertIsObject); - throw err; + throw new ERR_INVALID_ARG_TYPE(name, types || 'Object', value); } -} +}); -function assertWithinRange(name, value, min = 0, max = Infinity) { - if (value !== undefined && +const assertWithinRange = hideStackFrames( + (name, value, min = 0, max = Infinity) => { + if (value !== undefined && (typeof value !== 'number' || value < min || value > max)) { - const err = new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError( - name, value, min, max); - Error.captureStackTrace(err, assertWithinRange); - throw err; + throw new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError( + name, value, min, max); + } } -} +); function toHeaderObject(headers) { const obj = Object.create(null); diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index ca78425a52a332..dc0a1dcb279ff7 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -993,6 +993,7 @@ function formatPrimitive(fn, value, ctx) { } function formatError(value) { + // TODO(BridgeAR): Always show the error code if present. return value.stack || errorToString(value); } diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 6de46349c00fa8..a80917ee7edde4 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -1,10 +1,13 @@ 'use strict'; const { - ERR_INVALID_ARG_TYPE, - ERR_INVALID_ARG_VALUE, - ERR_OUT_OF_RANGE -} = require('internal/errors').codes; + hideStackFrames, + codes: { + ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, + ERR_OUT_OF_RANGE + } +} = require('internal/errors'); function isInt32(value) { return value === (value | 0); @@ -52,66 +55,52 @@ function validateMode(value, name, def) { throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc); } -function validateInteger(value, name) { - let err; - +const validateInteger = hideStackFrames((value, name) => { if (typeof value !== 'number') - err = new ERR_INVALID_ARG_TYPE(name, 'number', value); - else if (!Number.isSafeInteger(value)) - err = new ERR_OUT_OF_RANGE(name, 'an integer', value); - - if (err) { - Error.captureStackTrace(err, validateInteger); - throw err; - } - + throw new ERR_INVALID_ARG_TYPE(name, 'number', value); + if (!Number.isSafeInteger(value)) + throw new ERR_OUT_OF_RANGE(name, 'an integer', value); return value; -} - -function validateInt32(value, name, min = -2147483648, max = 2147483647) { - // The defaults for min and max correspond to the limits of 32-bit integers. - if (!isInt32(value)) { - let err; - if (typeof value !== 'number') { - err = new ERR_INVALID_ARG_TYPE(name, 'number', value); - } else if (!Number.isInteger(value)) { - err = new ERR_OUT_OF_RANGE(name, 'an integer', value); - } else { - err = new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value); +}); + +const validateInt32 = hideStackFrames( + (value, name, min = -2147483648, max = 2147483647) => { + // The defaults for min and max correspond to the limits of 32-bit integers. + if (!isInt32(value)) { + if (typeof value !== 'number') { + throw new ERR_INVALID_ARG_TYPE(name, 'number', value); + } + if (!Number.isInteger(value)) { + throw new ERR_OUT_OF_RANGE(name, 'an integer', value); + } + throw new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value); } - Error.captureStackTrace(err, validateInt32); - throw err; - } else if (value < min || value > max) { - const err = new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value); - Error.captureStackTrace(err, validateInt32); - throw err; + if (value < min || value > max) { + throw new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value); + } + return value; } +); - return value; -} - -function validateUint32(value, name, positive) { +const validateUint32 = hideStackFrames((value, name, positive) => { if (!isUint32(value)) { - let err; if (typeof value !== 'number') { - err = new ERR_INVALID_ARG_TYPE(name, 'number', value); - } else if (!Number.isInteger(value)) { - err = new ERR_OUT_OF_RANGE(name, 'an integer', value); - } else { - const min = positive ? 1 : 0; - // 2 ** 32 === 4294967296 - err = new ERR_OUT_OF_RANGE(name, `>= ${min} && < 4294967296`, value); + throw new ERR_INVALID_ARG_TYPE(name, 'number', value); } - Error.captureStackTrace(err, validateUint32); - throw err; - } else if (positive && value === 0) { - const err = new ERR_OUT_OF_RANGE(name, '>= 1 && < 4294967296', value); - Error.captureStackTrace(err, validateUint32); - throw err; + if (!Number.isInteger(value)) { + throw new ERR_OUT_OF_RANGE(name, 'an integer', value); + } + const min = positive ? 1 : 0; + // 2 ** 32 === 4294967296 + throw new ERR_OUT_OF_RANGE(name, `>= ${min} && < 4294967296`, value); } - + if (positive && value === 0) { + throw new ERR_OUT_OF_RANGE(name, '>= 1 && < 4294967296', value); + } + // TODO(BridgeAR): Remove return values from validation functions and + // especially reduce side effects caused by validation functions. return value; -} +}); function validateString(value, name) { if (typeof value !== 'string') diff --git a/lib/os.js b/lib/os.js index d6ecd29f57a7e1..af97f40e57e9ba 100644 --- a/lib/os.js +++ b/lib/os.js @@ -26,7 +26,12 @@ const constants = internalBinding('constants').os; const { deprecate } = require('internal/util'); const isWindows = process.platform === 'win32'; -const { codes: { ERR_SYSTEM_ERROR } } = require('internal/errors'); +const { + codes: { + ERR_SYSTEM_ERROR + }, + hideStackFrames +} = require('internal/errors'); const { validateInt32 } = require('internal/validators'); const { @@ -47,16 +52,14 @@ const { } = internalBinding('os'); function getCheckedFunction(fn) { - return function checkError(...args) { + return hideStackFrames(function checkError(...args) { const ctx = {}; const ret = fn(...args, ctx); if (ret === undefined) { - const err = new ERR_SYSTEM_ERROR(ctx); - Error.captureStackTrace(err, checkError); - throw err; + throw new ERR_SYSTEM_ERROR(ctx); } return ret; - }; + }); } const getHomeDirectory = getCheckedFunction(_getHomeDirectory); diff --git a/lib/util.js b/lib/util.js index d087c740b04d8b..f6f99f82b45722 100644 --- a/lib/util.js +++ b/lib/util.js @@ -21,18 +21,22 @@ 'use strict'; -const errors = require('internal/errors'); +const { + codes: { + ERR_FALSY_VALUE_REJECTION, + ERR_INVALID_ARG_TYPE, + ERR_OUT_OF_RANGE + }, + errnoException, + exceptionWithHostPort, + hideStackFrames +} = require('internal/errors'); const { format, formatWithOptions, inspect } = require('internal/util/inspect'); const { debuglog } = require('internal/util/debuglog'); -const { - ERR_FALSY_VALUE_REJECTION, - ERR_INVALID_ARG_TYPE, - ERR_OUT_OF_RANGE -} = errors.codes; const { validateNumber } = require('internal/validators'); const { TextDecoder, TextEncoder } = require('internal/encoding'); const { isBuffer } = require('buffer').Buffer; @@ -158,19 +162,16 @@ function _extend(target, source) { return target; } -function callbackifyOnRejected(reason, cb) { +const callbackifyOnRejected = hideStackFrames((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 // occurred", we error-wrap so the callback consumer can distinguish between // "the promise rejected with null" or "the promise fulfilled with undefined". if (!reason) { - const newReason = new ERR_FALSY_VALUE_REJECTION(); - newReason.reason = reason; - reason = newReason; - Error.captureStackTrace(reason, callbackifyOnRejected); + reason = new ERR_FALSY_VALUE_REJECTION(reason); } return cb(reason); -} +}); function callbackify(original) { if (typeof original !== 'function') { @@ -209,8 +210,8 @@ function getSystemErrorName(err) { // Keep the `exports =` so that various functions can still be monkeypatched module.exports = exports = { - _errnoException: errors.errnoException, - _exceptionWithHostPort: errors.exceptionWithHostPort, + _errnoException: errnoException, + _exceptionWithHostPort: exceptionWithHostPort, _extend, callbackify, debuglog, diff --git a/lib/zlib.js b/lib/zlib.js index ec08db9f7c56b0..9220b11b0f5971 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -22,12 +22,15 @@ 'use strict'; const { - ERR_BROTLI_INVALID_PARAM, - ERR_BUFFER_TOO_LARGE, - ERR_INVALID_ARG_TYPE, - ERR_OUT_OF_RANGE, - ERR_ZLIB_INITIALIZATION_FAILED, -} = require('internal/errors').codes; + codes: { + ERR_BROTLI_INVALID_PARAM, + ERR_BUFFER_TOO_LARGE, + ERR_INVALID_ARG_TYPE, + ERR_OUT_OF_RANGE, + ERR_ZLIB_INITIALIZATION_FAILED, + }, + hideStackFrames +} = require('internal/errors'); const Transform = require('_stream_transform'); const { deprecate, @@ -170,7 +173,7 @@ function zlibOnError(message, errno, code) { // 2. Returns true for finite numbers // 3. Throws ERR_INVALID_ARG_TYPE for non-numbers // 4. Throws ERR_OUT_OF_RANGE for infinite numbers -function checkFiniteNumber(number, name) { +const checkFiniteNumber = hideStackFrames((number, name) => { // Common case if (number === undefined) { return false; @@ -186,33 +189,29 @@ function checkFiniteNumber(number, name) { // Other non-numbers if (typeof number !== 'number') { - const err = new ERR_INVALID_ARG_TYPE(name, 'number', number); - Error.captureStackTrace(err, checkFiniteNumber); - throw err; + throw new ERR_INVALID_ARG_TYPE(name, 'number', number); } // Infinite numbers - const err = new ERR_OUT_OF_RANGE(name, 'a finite number', number); - Error.captureStackTrace(err, checkFiniteNumber); - throw err; -} + throw new ERR_OUT_OF_RANGE(name, 'a finite number', number); +}); // 1. Returns def for number when it's undefined or NaN // 2. Returns number for finite numbers >= lower and <= upper // 3. Throws ERR_INVALID_ARG_TYPE for non-numbers // 4. Throws ERR_OUT_OF_RANGE for infinite numbers or numbers > upper or < lower -function checkRangesOrGetDefault(number, name, lower, upper, def) { - if (!checkFiniteNumber(number, name)) { - return def; - } - if (number < lower || number > upper) { - const err = new ERR_OUT_OF_RANGE(name, - `>= ${lower} and <= ${upper}`, number); - Error.captureStackTrace(err, checkRangesOrGetDefault); - throw err; +const checkRangesOrGetDefault = hideStackFrames( + (number, name, lower, upper, def) => { + if (!checkFiniteNumber(number, name)) { + return def; + } + if (number < lower || number > upper) { + throw new ERR_OUT_OF_RANGE(name, + `>= ${lower} and <= ${upper}`, number); + } + return number; } - return number; -} +); // The base class for all Zlib-style streams. function ZlibBase(opts, mode, handle, { flush, finishFlush, fullFlush }) { diff --git a/test/common/wpt.js b/test/common/wpt.js index f9e48c5f977beb..5592ddfd4aed16 100644 --- a/test/common/wpt.js +++ b/test/common/wpt.js @@ -287,11 +287,6 @@ class WPTRunner { // TODO(joyeecheung): work with the upstream to port more tests in .html // to .js. runJsTests() { - // TODO(joyeecheung): it's still under discussion whether we should leave - // err.name alone. See https://github.com/nodejs/node/issues/20253 - const internalErrors = require('internal/errors'); - internalErrors.useOriginalName = true; - let queue = []; // If the tests are run as `node test/wpt/test-something.js subset.any.js`,