From da8ff5586281f8b2da48c1b0020043ea354d848b Mon Sep 17 00:00:00 2001 From: uzlopak Date: Thu, 5 Oct 2023 14:00:02 +0200 Subject: [PATCH] errors: improve hideStackFrames --- benchmark/error/hidestackframes-noerr.js | 62 ++++++ benchmark/error/hidestackframes-throw.js | 87 ++++++++ benchmark/error/hidestackframes.js | 45 ---- lib/_http_outgoing.js | 6 +- lib/buffer.js | 20 +- lib/internal/crypto/hkdf.js | 10 +- lib/internal/crypto/util.js | 4 +- lib/internal/errors.js | 103 ++++----- lib/internal/fs/utils.js | 99 ++++----- lib/internal/http2/compat.js | 6 +- lib/internal/http2/core.js | 8 +- lib/internal/http2/util.js | 16 +- lib/internal/util.js | 16 +- lib/internal/validators.js | 54 ++--- lib/os.js | 2 +- lib/util.js | 9 +- lib/zlib.js | 10 +- .../errors/error_aggregateTwoErrors.snapshot | 3 +- .../parallel/test-errors-hide-stack-frames.js | 208 ++++++++++++++++++ test/parallel/test-util-callbackify.js | 18 ++ 20 files changed, 547 insertions(+), 239 deletions(-) create mode 100644 benchmark/error/hidestackframes-noerr.js create mode 100644 benchmark/error/hidestackframes-throw.js delete mode 100644 benchmark/error/hidestackframes.js create mode 100644 test/parallel/test-errors-hide-stack-frames.js diff --git a/benchmark/error/hidestackframes-noerr.js b/benchmark/error/hidestackframes-noerr.js new file mode 100644 index 00000000000000..9e662f6a620f13 --- /dev/null +++ b/benchmark/error/hidestackframes-noerr.js @@ -0,0 +1,62 @@ +'use strict'; + +const common = require('../common.js'); +const assert = require('assert'); + +const bench = common.createBenchmark(main, { + type: [ + 'hide-stackframes', + 'direct-call', + ], + n: [1e7], +}, { + flags: ['--expose-internals'], +}); + +function main({ n, type }) { + const { + hideStackFrames, + codes: { + ERR_INVALID_ARG_TYPE, + }, + } = require('internal/errors'); + + const testfn = (value) => { + if (typeof value !== 'number') { + throw new ERR_INVALID_ARG_TYPE('Benchmark', 'number', value); + } + }; + + const hideStackFramesTestfn = hideStackFrames((value) => { + if (typeof value !== 'number') { + throw new ERR_INVALID_ARG_TYPE.HideStackFrameError('Benchmark', 'number', value); + } + }); + + const fn = type === 'hide-stackframe' ? hideStackFramesTestfn : testfn; + + const value = 42; + + const length = 1024; + const array = []; + const errCase = false; + + for (let i = 0; i < length; ++i) { + array.push(fn(value)); + } + + bench.start(); + + for (let i = 0; i < n; i++) { + const index = i % length; + array[index] = fn(value); + } + + bench.end(n); + + // Verify the entries to prevent dead code elimination from making + // the benchmark invalid. + for (let i = 0; i < length; ++i) { + assert.strictEqual(typeof array[i], errCase ? 'object' : 'undefined'); + } +} diff --git a/benchmark/error/hidestackframes-throw.js b/benchmark/error/hidestackframes-throw.js new file mode 100644 index 00000000000000..c7147e39ba57a3 --- /dev/null +++ b/benchmark/error/hidestackframes-throw.js @@ -0,0 +1,87 @@ +'use strict'; + +const common = require('../common.js'); +const assert = require('assert'); + +const bench = common.createBenchmark(main, { + type: [ + 'hide-stackframes', + 'direct-call', + ], + double: ['true', 'false'], + n: [1e5], +}, { + flags: ['--expose-internals'], +}); + +function main({ n, type, double }) { + const { + hideStackFrames, + codes: { + ERR_INVALID_ARG_TYPE, + }, + } = require('internal/errors'); + + const value = 'err'; + + const testfn = (value) => { + if (typeof value !== 'number') { + throw new ERR_INVALID_ARG_TYPE('Benchmark', 'number', value); + } + }; + + const hideStackFramesTestfn = hideStackFrames((value) => { + if (typeof value !== 'number') { + throw new ERR_INVALID_ARG_TYPE.HideStackFrameError('Benchmark', 'number', value); + } + }); + + function doubleTestfn(value) { + testfn(value); + } + + const doubleHideStackFramesTestfn = hideStackFrames((value) => { + hideStackFramesTestfn.withoutStackTrace(value); + }); + + const fn = type === 'hide-stackframe' ? + double === 'true' ? + doubleHideStackFramesTestfn : + hideStackFramesTestfn : + double === 'true' ? + doubleTestfn : + testfn; + + const length = 1024; + const array = []; + let errCase = false; + + // Warm up. + for (let i = 0; i < length; ++i) { + try { + fn(value); + } catch (e) { + errCase = true; + array.push(e); + } + } + + bench.start(); + + for (let i = 0; i < n; i++) { + const index = i % length; + try { + fn(value); + } catch (e) { + array[index] = e; + } + } + + bench.end(n); + + // Verify the entries to prevent dead code elimination from making + // the benchmark invalid. + for (let i = 0; i < length; ++i) { + assert.strictEqual(typeof array[i], errCase ? 'object' : 'undefined'); + } +} diff --git a/benchmark/error/hidestackframes.js b/benchmark/error/hidestackframes.js deleted file mode 100644 index b28be725a30969..00000000000000 --- a/benchmark/error/hidestackframes.js +++ /dev/null @@ -1,45 +0,0 @@ -'use strict'; - -const common = require('../common.js'); - -const bench = common.createBenchmark(main, { - type: ['hide-stackframes-throw', 'direct-call-throw', - 'hide-stackframes-noerr', 'direct-call-noerr'], - n: [10e4], -}, { - flags: ['--expose-internals'], -}); - -function main({ n, type }) { - const { - hideStackFrames, - codes: { - ERR_INVALID_ARG_TYPE, - }, - } = require('internal/errors'); - - const testfn = (value) => { - if (typeof value !== 'number') { - throw new ERR_INVALID_ARG_TYPE('Benchmark', 'number', value); - } - }; - - let fn = testfn; - if (type.startsWith('hide-stackframe')) - fn = hideStackFrames(testfn); - let value = 42; - if (type.endsWith('-throw')) - value = 'err'; - - bench.start(); - - for (let i = 0; i < n; i++) { - try { - fn(value); - } catch { - // No-op - } - } - - bench.end(n); -} diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 178a3418dace0a..3830b2a5c7b91a 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -619,17 +619,17 @@ function matchHeader(self, state, field, value) { const validateHeaderName = hideStackFrames((name, label) => { if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) { - throw new ERR_INVALID_HTTP_TOKEN(label || 'Header name', name); + throw new ERR_INVALID_HTTP_TOKEN.HideStackFramesError(label || 'Header name', name); } }); const validateHeaderValue = hideStackFrames((name, value) => { if (value === undefined) { - throw new ERR_HTTP_INVALID_HEADER_VALUE(value, name); + throw new ERR_HTTP_INVALID_HEADER_VALUE.HideStackFramesError(value, name); } if (checkInvalidHeaderChar(value)) { debug('Header "%s" contains invalid characters', name); - throw new ERR_INVALID_CHAR('header content', name); + throw new ERR_INVALID_CHAR.HideStackFramesError('header content', name); } }); diff --git a/lib/buffer.js b/lib/buffer.js index 0ff7c1920adbba..cfc6d5b21febc3 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -108,7 +108,6 @@ const { ERR_UNKNOWN_ENCODING, }, genericNodeError, - hideStackFrames, } = require('internal/errors'); const { validateArray, @@ -386,19 +385,12 @@ Buffer.of = of; ObjectSetPrototypeOf(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. -const assertSize = hideStackFrames((size) => { - validateNumber(size, 'size', 0, kMaxLength); -}); - /** * Creates a new filled Buffer instance. * alloc(size[, fill[, encoding]]) */ Buffer.alloc = function alloc(size, fill, encoding) { - assertSize(size); + validateNumber(size, 'size', 0, kMaxLength); if (fill !== undefined && fill !== 0 && size > 0) { const buf = createUnsafeBuffer(size); return _fill(buf, fill, 0, buf.length, encoding); @@ -411,7 +403,7 @@ Buffer.alloc = function alloc(size, fill, encoding) { * instance. If `--zero-fill-buffers` is set, will zero-fill the buffer. */ Buffer.allocUnsafe = function allocUnsafe(size) { - assertSize(size); + validateNumber(size, 'size', 0, kMaxLength); return allocate(size); }; @@ -421,15 +413,15 @@ Buffer.allocUnsafe = function allocUnsafe(size) { * If `--zero-fill-buffers` is set, will zero-fill the buffer. */ Buffer.allocUnsafeSlow = function allocUnsafeSlow(size) { - assertSize(size); + validateNumber(size, 'size', 0, kMaxLength); return createUnsafeBuffer(size); }; // If --zero-fill-buffers command line argument is set, a zero-filled // buffer is returned. -function SlowBuffer(length) { - assertSize(length); - return createUnsafeBuffer(length); +function SlowBuffer(size) { + validateNumber(size, 'size', 0, kMaxLength); + return createUnsafeBuffer(size); } ObjectSetPrototypeOf(SlowBuffer.prototype, Uint8ArrayPrototype); diff --git a/lib/internal/crypto/hkdf.js b/lib/internal/crypto/hkdf.js index cf3c39e8d9da5a..757a2391a0167f 100644 --- a/lib/internal/crypto/hkdf.js +++ b/lib/internal/crypto/hkdf.js @@ -49,15 +49,15 @@ const { } = require('internal/errors'); const validateParameters = hideStackFrames((hash, key, salt, info, length) => { - validateString(hash, 'digest'); + validateString.withoutStackTrace(hash, 'digest'); key = prepareKey(key); - salt = validateByteSource(salt, 'salt'); - info = validateByteSource(info, 'info'); + salt = validateByteSource.withoutStackTrace(salt, 'salt'); + info = validateByteSource.withoutStackTrace(info, 'info'); - validateInteger(length, 'length', 0, kMaxLength); + validateInteger.withoutStackTrace(length, 'length', 0, kMaxLength); if (info.byteLength > 1024) { - throw new ERR_OUT_OF_RANGE( + throw new ERR_OUT_OF_RANGE.HideStackFramesError( 'info', 'must not contain more than 1024 bytes', info.byteLength); diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js index 51ca3f4c056fb9..45a236a1130249 100644 --- a/lib/internal/crypto/util.js +++ b/lib/internal/crypto/util.js @@ -116,7 +116,7 @@ const getArrayBufferOrView = hideStackFrames((buffer, name, encoding) => { return Buffer.from(buffer, encoding); } if (!isArrayBufferView(buffer)) { - throw new ERR_INVALID_ARG_TYPE( + throw new ERR_INVALID_ARG_TYPE.HideStackFramesError( name, [ 'string', @@ -403,7 +403,7 @@ const validateByteSource = hideStackFrames((val, name) => { if (isAnyArrayBuffer(val) || isArrayBufferView(val)) return val; - throw new ERR_INVALID_ARG_TYPE( + throw new ERR_INVALID_ARG_TYPE.HideStackFramesError( name, [ 'string', diff --git a/lib/internal/errors.js b/lib/internal/errors.js index ea8138d8dca7eb..bf30c43a4e35ac 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -86,8 +86,7 @@ const kTypes = [ const MainContextError = Error; const overrideStackTrace = new SafeWeakMap(); const kNoOverride = Symbol('kNoOverride'); -let userStackTraceLimit; -const nodeInternalPrefix = '__node_internal_'; + const prepareStackTrace = (globalThis, error, trace) => { // API for node internals to override error stack formatting // without interfering with userland code. @@ -97,21 +96,6 @@ const prepareStackTrace = (globalThis, error, trace) => { return f(error, trace); } - const firstFrame = trace[0]?.getFunctionName(); - if (firstFrame && StringPrototypeStartsWith(firstFrame, nodeInternalPrefix)) { - for (let l = trace.length - 1; l >= 0; l--) { - const fn = trace[l]?.getFunctionName(); - if (fn && StringPrototypeStartsWith(fn, nodeInternalPrefix)) { - ArrayPrototypeSplice(trace, 0, l + 1); - break; - } - } - // `userStackTraceLimit` is the user value for `Error.stackTraceLimit`, - // it is updated at every new exception in `captureLargerStackTrace`. - if (trace.length > userStackTraceLimit) - ArrayPrototypeSplice(trace, userStackTraceLimit); - } - const globalOverride = maybeOverridePrepareStackTrace(globalThis, error, trace); if (globalOverride !== kNoOverride) return globalOverride; @@ -242,11 +226,7 @@ function inspectWithNoCustomRetry(obj, options) { // and may have .path and .dest. class SystemError extends Error { constructor(key, context) { - const limit = Error.stackTraceLimit; - if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0; super(); - // Reset the limit and setting the name property. - if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit; const prefix = getMessage(key, [], this); let message = `${prefix}: ${context.syscall} returned ` + `${context.code} (${context.message})`; @@ -256,8 +236,6 @@ class SystemError extends Error { if (context.dest !== undefined) message += ` => ${context.dest}`; - captureLargerStackTrace(this); - this.code = key; ObjectDefineProperties(this, { @@ -372,6 +350,29 @@ function makeSystemErrorWithCode(key) { }; } +function makeNodeErrorForHideStackFrame(Base, clazz) { + class HideStackFramesError extends Base { + constructor(...args) { + if (isErrorStackTraceLimitWritable()) { + const limit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + super(...args); + Error.stackTraceLimit = limit; + } else { + super(...args); + } + } + + // This is a workaround for wpt tests that expect that the error + // constructor has a `name` property of the base class. + get ['constructor']() { + return clazz; + } + } + + return HideStackFramesError; +} + function makeNodeErrorWithCode(Base, key) { const msg = messages.get(key); const expectedLength = typeof msg !== 'string' ? -1 : getExpectedArgumentLength(msg); @@ -479,11 +480,16 @@ function makeNodeErrorWithCode(Base, key) { * @returns {T} */ function hideStackFrames(fn) { - // We rename the functions that will be hidden to cut off the stacktrace - // at the outermost one - const hidden = nodeInternalPrefix + fn.name; - ObjectDefineProperty(fn, 'name', { __proto__: null, value: hidden }); - return fn; + function wrappedFn(...args) { + try { + return ReflectApply(fn, fn, args); + } catch (error) { + Error.stackTraceLimit && ErrorCaptureStackTrace(error, wrappedFn); + throw error; + } + }; + wrappedFn.withoutStackTrace = fn; + return wrappedFn; } // Utility function for registering the error codes. Only used here. Exported @@ -492,18 +498,20 @@ function E(sym, val, def, ...otherClasses) { // Special case for SystemError that formats the error message differently // The SystemErrors only have SystemError as their base classes. messages.set(sym, val); - if (def === SystemError) { - def = makeSystemErrorWithCode(sym); - } else { - def = makeNodeErrorWithCode(def, sym); - } + + const ErrClass = def === SystemError ? + makeSystemErrorWithCode(sym) : + makeNodeErrorWithCode(def, sym); if (otherClasses.length !== 0) { otherClasses.forEach((clazz) => { - def[clazz.name] = makeNodeErrorWithCode(clazz, sym); + ErrClass[clazz.name] = makeNodeErrorWithCode(clazz, sym); }); } - codes[sym] = def; + + ErrClass.HideStackFramesError = makeNodeErrorForHideStackFrame(ErrClass, def); + + codes[sym] = ErrClass; } function getExpectedArgumentLength(msg) { @@ -553,20 +561,6 @@ function uvErrmapGet(name) { return MapPrototypeGet(uvBinding.errmap, name); } -const captureLargerStackTrace = hideStackFrames( - function captureLargerStackTrace(err) { - const stackTraceLimitIsWritable = isErrorStackTraceLimitWritable(); - if (stackTraceLimitIsWritable) { - userStackTraceLimit = Error.stackTraceLimit; - Error.stackTraceLimit = Infinity; - } - ErrorCaptureStackTrace(err); - // Reset the limit - if (stackTraceLimitIsWritable) Error.stackTraceLimit = userStackTraceLimit; - - return err; - }); - /** * This creates an error compatible with errors produced in the C++ * function UVException using a context object with data assembled in C++. @@ -616,7 +610,7 @@ const uvException = hideStackFrames(function uvException(ctx) { err.dest = dest; } - return captureLargerStackTrace(err); + return err; }); /** @@ -657,7 +651,7 @@ const uvExceptionWithHostPort = hideStackFrames( ex.port = port; } - return captureLargerStackTrace(ex); + return ex; }); /** @@ -687,7 +681,7 @@ const errnoException = hideStackFrames( ex.code = code; ex.syscall = syscall; - return captureLargerStackTrace(ex); + return ex; }); /** @@ -735,7 +729,7 @@ const exceptionWithHostPort = hideStackFrames( ex.port = port; } - return captureLargerStackTrace(ex); + return ex; }); /** @@ -779,7 +773,7 @@ const dnsException = hideStackFrames(function(code, syscall, hostname) { ex.hostname = hostname; } - return captureLargerStackTrace(ex); + return ex; }); function connResetException(msg) { @@ -1012,7 +1006,6 @@ module.exports = { AbortError, aggregateTwoErrors, aggregateErrors, - captureLargerStackTrace, codes, connResetException, dnsException, diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index f84133296e86fb..24c20b5ca1aeca 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -360,30 +360,6 @@ function handleErrorFromBinding(ctx) { } } -// Check if the path contains null types if it is a string nor Uint8Array, -// otherwise return silently. -const nullCheck = hideStackFrames((path, propName, throwError = true) => { - const pathIsString = typeof path === 'string'; - const pathIsUint8Array = isUint8Array(path); - - // We can only perform meaningful checks on strings and Uint8Arrays. - if ((!pathIsString && !pathIsUint8Array) || - (pathIsString && !StringPrototypeIncludes(path, '\u0000')) || - (pathIsUint8Array && !TypedArrayPrototypeIncludes(path, 0))) { - return; - } - - const err = new ERR_INVALID_ARG_VALUE( - propName, - path, - 'must be a string, Uint8Array, or URL without null bytes', - ); - if (throwError) { - throw err; - } - return err; -}); - function preprocessSymlinkDestination(path, type, linkPath) { if (!isWindows) { // No preprocessing is needed on Unix. @@ -663,14 +639,14 @@ function toUnixTimestamp(time, name = 'time') { const validateOffsetLengthRead = hideStackFrames( (offset, length, bufferLength) => { if (offset < 0) { - throw new ERR_OUT_OF_RANGE('offset', '>= 0', offset); + throw new ERR_OUT_OF_RANGE.HideStackFramesError('offset', '>= 0', offset); } if (length < 0) { - throw new ERR_OUT_OF_RANGE('length', '>= 0', length); + throw new ERR_OUT_OF_RANGE.HideStackFramesError('length', '>= 0', length); } if (offset + length > bufferLength) { - throw new ERR_OUT_OF_RANGE('length', - `<= ${bufferLength - offset}`, length); + throw new ERR_OUT_OF_RANGE.HideStackFramesError('length', + `<= ${bufferLength - offset}`, length); } }, ); @@ -678,31 +654,41 @@ const validateOffsetLengthRead = hideStackFrames( const validateOffsetLengthWrite = hideStackFrames( (offset, length, byteLength) => { if (offset > byteLength) { - throw new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset); + throw new ERR_OUT_OF_RANGE.HideStackFramesError('offset', `<= ${byteLength}`, offset); } if (length > byteLength - offset) { - throw new ERR_OUT_OF_RANGE('length', `<= ${byteLength - offset}`, length); + throw new ERR_OUT_OF_RANGE.HideStackFramesError('length', `<= ${byteLength - offset}`, length); } if (length < 0) { - throw new ERR_OUT_OF_RANGE('length', '>= 0', length); + throw new ERR_OUT_OF_RANGE.HideStackFramesError('length', '>= 0', length); } - validateInt32(length, 'length', 0); + validateInt32.withoutStackTrace(length, 'length', 0); }, ); const validatePath = hideStackFrames((path, propName = 'path') => { if (typeof path !== 'string' && !isUint8Array(path)) { - throw new ERR_INVALID_ARG_TYPE(propName, ['string', 'Buffer', 'URL'], path); + throw new ERR_INVALID_ARG_TYPE.HideStackFramesError(propName, ['string', 'Buffer', 'URL'], path); } - const err = nullCheck(path, propName, false); + const pathIsString = typeof path === 'string'; + const pathIsUint8Array = isUint8Array(path); - if (err !== undefined) { - throw err; + // We can only perform meaningful checks on strings and Uint8Arrays. + if ((!pathIsString && !pathIsUint8Array) || + (pathIsString && !StringPrototypeIncludes(path, '\u0000')) || + (pathIsUint8Array && !TypedArrayPrototypeIncludes(path, 0))) { + return; } + + throw new ERR_INVALID_ARG_VALUE.HideStackFramesError( + propName, + path, + 'must be a string, Uint8Array, or URL without null bytes', + ); }); // TODO(rafaelgss): implement the path.resolve on C++ side @@ -737,11 +723,11 @@ const getValidatedFd = hideStackFrames((fd, propName = 'fd') => { const validateBufferArray = hideStackFrames((buffers, propName = 'buffers') => { if (!ArrayIsArray(buffers)) - throw new ERR_INVALID_ARG_TYPE(propName, 'ArrayBufferView[]', buffers); + throw new ERR_INVALID_ARG_TYPE.HideStackFramesError(propName, 'ArrayBufferView[]', buffers); for (let i = 0; i < buffers.length; i++) { if (!isArrayBufferView(buffers[i])) - throw new ERR_INVALID_ARG_TYPE(propName, 'ArrayBufferView[]', buffers); + throw new ERR_INVALID_ARG_TYPE.HideStackFramesError(propName, 'ArrayBufferView[]', buffers); } return buffers; @@ -797,7 +783,7 @@ const validateCpOptions = hideStackFrames((options) => { validateBoolean(options.verbatimSymlinks, 'options.verbatimSymlinks'); options.mode = getValidMode(options.mode, 'copyFile'); if (options.dereference === true && options.verbatimSymlinks === true) { - throw new ERR_INCOMPATIBLE_OPTION_PAIR('dereference', 'verbatimSymlinks'); + throw new ERR_INCOMPATIBLE_OPTION_PAIR.HideStackFramesError('dereference', 'verbatimSymlinks'); } if (options.filter !== undefined) { validateFunction(options.filter, 'options.filter'); @@ -822,21 +808,23 @@ const validateRmOptions = hideStackFrames((path, options, expectDir, cb) => { } if (stats.isDirectory() && !options.recursive) { - return cb(new ERR_FS_EISDIR({ + const err = new ERR_FS_EISDIR.HideStackFramesError({ code: 'EISDIR', message: 'is a directory', path, syscall: 'rm', errno: EISDIR, - })); + }); + + return cb(err); } return cb(null, options); }); }); const validateRmOptionsSync = hideStackFrames((path, options, expectDir) => { - options = validateRmdirOptions(options, defaultRmOptions); - validateBoolean(options.force, 'options.force'); + options = validateRmdirOptions.withoutStackTrace(options, defaultRmOptions); + validateBoolean.withoutStackTrace(options.force, 'options.force'); if (!options.force || expectDir || !options.recursive) { const isDirectory = lazyLoadFs() @@ -847,7 +835,7 @@ const validateRmOptionsSync = hideStackFrames((path, options, expectDir) => { } if (isDirectory && !options.recursive) { - throw new ERR_FS_EISDIR({ + throw new ERR_FS_EISDIR.HideStackFramesError({ code: 'EISDIR', message: 'is a directory', path, @@ -877,13 +865,13 @@ const validateRmdirOptions = hideStackFrames( (options, defaults = defaultRmdirOptions) => { if (options === undefined) return defaults; - validateObject(options, 'options'); + validateObject.withoutStackTrace(options, 'options'); options = { ...defaults, ...options }; - validateBoolean(options.recursive, 'options.recursive'); - validateInt32(options.retryDelay, 'options.retryDelay', 0); - validateUint32(options.maxRetries, 'options.maxRetries'); + validateBoolean.withoutStackTrace(options.recursive, 'options.recursive'); + validateInt32.withoutStackTrace(options.retryDelay, 'options.retryDelay', 0); + validateUint32.withoutStackTrace(options.maxRetries, 'options.maxRetries'); return options; }); @@ -902,13 +890,13 @@ const getValidMode = hideStackFrames((mode, type) => { if (mode == null) { return def; } - validateInteger(mode, 'mode', min, max); + validateInteger.withoutStackTrace(mode, 'mode', min, max); return mode; }); const validateStringAfterArrayBufferView = hideStackFrames((buffer, name) => { if (typeof buffer !== 'string') { - throw new ERR_INVALID_ARG_TYPE( + throw new ERR_INVALID_ARG_TYPE.HideStackFramesError( name, ['string', 'Buffer', 'TypedArray', 'DataView'], buffer, @@ -918,16 +906,16 @@ const validateStringAfterArrayBufferView = hideStackFrames((buffer, name) => { const validatePosition = hideStackFrames((position, name, length) => { if (typeof position === 'number') { - validateInteger(position, name, -1); + validateInteger.withoutStackTrace(position, name, -1); } else if (typeof position === 'bigint') { const maxPosition = 2n ** 63n - 1n - BigInt(length); if (!(position >= -1n && position <= maxPosition)) { - throw new ERR_OUT_OF_RANGE(name, - `>= -1 && <= ${maxPosition}`, - position); + throw new ERR_OUT_OF_RANGE.HideStackFramesError(name, + `>= -1 && <= ${maxPosition}`, + position); } } else { - throw new ERR_INVALID_ARG_TYPE(name, ['integer', 'bigint'], position); + throw new ERR_INVALID_ARG_TYPE.HideStackFramesError(name, ['integer', 'bigint'], position); } }); @@ -951,7 +939,6 @@ module.exports = { getValidatedPath, getValidMode, handleErrorFromBinding, - nullCheck, possiblyTransformPath, preprocessSymlinkDestination, realpathCacheKey: Symbol('realpathCacheKey'), diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 2ab9c70ccd7402..7bf079900c652f 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -89,13 +89,13 @@ const assertValidHeader = hideStackFrames((name, value) => { if (name === '' || typeof name !== 'string' || StringPrototypeIncludes(name, ' ')) { - throw new ERR_INVALID_HTTP_TOKEN('Header name', name); + throw new ERR_INVALID_HTTP_TOKEN.HideStackFramesError('Header name', name); } if (isPseudoHeader(name)) { - throw new ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED(); + throw new ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED.HideStackFramesError(); } if (value === undefined || value === null) { - throw new ERR_HTTP2_INVALID_HEADER_VALUE(value, name); + throw new ERR_HTTP2_INVALID_HEADER_VALUE.HideStackFramesError(value, name); } if (!isConnectionHeaderAllowed(name, value)) { connectionHeaderMessageWarn(); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 1a689859aff752..c1b7bc09d0566a 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -766,25 +766,25 @@ const setAndValidatePriorityOptions = hideStackFrames((options) => { if (options.weight === undefined) { options.weight = NGHTTP2_DEFAULT_WEIGHT; } else { - validateNumber(options.weight, 'options.weight'); + validateNumber.withoutStackTrace(options.weight, 'options.weight'); } if (options.parent === undefined) { options.parent = 0; } else { - validateNumber(options.parent, 'options.parent', 0); + validateNumber.withoutStackTrace(options.parent, 'options.parent', 0); } if (options.exclusive === undefined) { options.exclusive = false; } else { - validateBoolean(options.exclusive, 'options.exclusive'); + validateBoolean.withoutStackTrace(options.exclusive, 'options.exclusive'); } if (options.silent === undefined) { options.silent = false; } else { - validateBoolean(options.silent, 'options.silent'); + validateBoolean.withoutStackTrace(options.silent, 'options.silent'); } }); diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 6d4a7f94b3d11a..ee00c3c7ad8e5a 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -8,7 +8,6 @@ const { Error, MathMax, Number, - ObjectDefineProperty, ObjectKeys, SafeSet, String, @@ -23,12 +22,11 @@ const { codes: { ERR_HTTP2_HEADER_SINGLE_VALUE, ERR_HTTP2_INVALID_CONNECTION_HEADERS, - ERR_HTTP2_INVALID_PSEUDOHEADER, + ERR_HTTP2_INVALID_PSEUDOHEADER: { HideStackFramesError: ERR_HTTP2_INVALID_PSEUDOHEADER }, ERR_HTTP2_INVALID_SETTING_VALUE, ERR_INVALID_ARG_TYPE, ERR_INVALID_HTTP_TOKEN, }, - captureLargerStackTrace, getMessage, hideStackFrames, kIsNodeError, @@ -552,14 +550,10 @@ class NghttpError extends Error { binding.nghttp2ErrorString(integerCode)); this.code = customErrorCode || 'ERR_HTTP2_ERROR'; this.errno = integerCode; - captureLargerStackTrace(this); - ObjectDefineProperty(this, kIsNodeError, { - __proto__: null, - value: true, - enumerable: false, - writable: false, - configurable: true, - }); + } + + get [kIsNodeError]() { + return true; } toString() { diff --git a/lib/internal/util.js b/lib/internal/util.js index 558a5da69773bb..4149840b244adc 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -8,6 +8,7 @@ const { ArrayPrototypeSlice, ArrayPrototypeSort, Error, + ErrorCaptureStackTrace, FunctionPrototypeCall, ObjectDefineProperties, ObjectDefineProperty, @@ -44,11 +45,11 @@ const { } = primordials; const { - hideStackFrames, codes: { ERR_NO_CRYPTO, ERR_UNKNOWN_SIGNAL, }, + isErrorStackTraceLimitWritable, uvErrmapGet, overrideStackTrace, } = require('internal/errors'); @@ -693,10 +694,19 @@ const lazyDOMExceptionClass = () => { return _DOMException; }; -const lazyDOMException = hideStackFrames((message, name) => { +const lazyDOMException = (message, name) => { _DOMException ??= internalBinding('messaging').DOMException; + if (isErrorStackTraceLimitWritable()) { + const limit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + const ex = new _DOMException(message, name); + Error.stackTraceLimit = limit; + ErrorCaptureStackTrace(ex, lazyDOMException); + return ex; + } return new _DOMException(message, name); -}); + +}; const kEnumerableProperty = { __proto__: null }; kEnumerableProperty.enumerable = true; diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 088a0a668ba8b2..c93dcad11849bb 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -22,11 +22,11 @@ const { const { hideStackFrames, codes: { - ERR_SOCKET_BAD_PORT, - ERR_INVALID_ARG_TYPE, - ERR_INVALID_ARG_VALUE, - ERR_OUT_OF_RANGE, - ERR_UNKNOWN_SIGNAL, + ERR_SOCKET_BAD_PORT: { HideStackFramesError: ERR_SOCKET_BAD_PORT }, + ERR_INVALID_ARG_TYPE: { HideStackFramesError: ERR_INVALID_ARG_TYPE }, + ERR_INVALID_ARG_VALUE: { HideStackFramesError: ERR_INVALID_ARG_VALUE }, + ERR_OUT_OF_RANGE: { HideStackFramesError: ERR_OUT_OF_RANGE }, + ERR_UNKNOWN_SIGNAL: { HideStackFramesError: ERR_UNKNOWN_SIGNAL }, }, } = require('internal/errors'); const { normalizeEncoding } = require('internal/util'); @@ -157,10 +157,10 @@ const validateUint32 = hideStackFrames((value, name, positive = false) => { */ /** @type {validateString} */ -function validateString(value, name) { +const validateString = hideStackFrames((value, name) => { if (typeof value !== 'string') throw new ERR_INVALID_ARG_TYPE(name, 'string', value); -} +}); /** * @callback validateNumber @@ -172,7 +172,7 @@ function validateString(value, name) { */ /** @type {validateNumber} */ -function validateNumber(value, name, min = undefined, max) { +const validateNumber = hideStackFrames((value, name, min = undefined, max) => { if (typeof value !== 'number') throw new ERR_INVALID_ARG_TYPE(name, 'number', value); @@ -183,7 +183,7 @@ function validateNumber(value, name, min = undefined, max) { `${min != null ? `>= ${min}` : ''}${min != null && max != null ? ' && ' : ''}${max != null ? `<= ${max}` : ''}`, value); } -} +}); /** * @callback validateOneOf @@ -213,10 +213,10 @@ const validateOneOf = hideStackFrames((value, name, oneOf) => { */ /** @type {validateBoolean} */ -function validateBoolean(value, name) { +const validateBoolean = hideStackFrames((value, name) => { if (typeof value !== 'boolean') throw new ERR_INVALID_ARG_TYPE(name, 'boolean', value); -} +}); const kValidateObjectNone = 0; const kValidateObjectAllowNullable = 1 << 0; @@ -309,12 +309,12 @@ const validateArray = hideStackFrames((value, name, minLength = 0) => { */ /** @type {validateStringArray} */ -function validateStringArray(value, name) { +const validateStringArray = hideStackFrames((value, name) => { validateArray(value, name); for (let i = 0; i < value.length; i++) { validateString(value[i], `${name}[${i}]`); } -} +}); /** * @callback validateBooleanArray @@ -324,12 +324,12 @@ function validateStringArray(value, name) { */ /** @type {validateBooleanArray} */ -function validateBooleanArray(value, name) { +const validateBooleanArray = hideStackFrames((value, name) => { validateArray(value, name); for (let i = 0; i < value.length; i++) { validateBoolean(value[i], `${name}[${i}]`); } -} +}); /** * @callback validateAbortSignalArray @@ -356,7 +356,7 @@ function validateAbortSignalArray(value, name) { * @param {string} [name='signal'] * @returns {asserts signal is keyof signals} */ -function validateSignalName(signal, name = 'signal') { +const validateSignalName = hideStackFrames((signal, name = 'signal') => { validateString(signal, name); if (signals[signal] === undefined) { @@ -367,7 +367,7 @@ function validateSignalName(signal, name = 'signal') { throw new ERR_UNKNOWN_SIGNAL(signal); } -} +}); /** * @callback validateBuffer @@ -389,7 +389,7 @@ const validateBuffer = hideStackFrames((buffer, name = 'buffer') => { * @param {string} data * @param {string} encoding */ -function validateEncoding(data, encoding) { +const validateEncoding = hideStackFrames((data, encoding) => { const normalizedEncoding = normalizeEncoding(encoding); const length = data.length; @@ -397,7 +397,7 @@ function validateEncoding(data, encoding) { throw new ERR_INVALID_ARG_VALUE('encoding', encoding, `is invalid for data of length ${length}`); } -} +}); /** * Check that the port number is not NaN when coerced to a number, @@ -407,7 +407,7 @@ function validateEncoding(data, encoding) { * @param {boolean} [allowZero=true] * @returns {number} */ -function validatePort(port, name = 'Port', allowZero = true) { +const validatePort = hideStackFrames((port, name = 'Port', allowZero = true) => { if ((typeof port !== 'number' && typeof port !== 'string') || (typeof port === 'string' && StringPrototypeTrim(port).length === 0) || +port !== (+port >>> 0) || @@ -416,7 +416,7 @@ function validatePort(port, name = 'Port', allowZero = true) { throw new ERR_SOCKET_BAD_PORT(name, port, allowZero); } return port | 0; -} +}); /** * @callback validateAbortSignal @@ -499,7 +499,7 @@ const linkValueRegExp = /^(?:<[^>]*>)(?:\s*;\s*[^;"\s]+(?:=(")?[^;"\s]*\1)?)*$/; * @param {any} value * @param {string} name */ -function validateLinkHeaderFormat(value, name) { +const validateLinkHeaderFormat = hideStackFrames((value, name) => { if ( typeof value === 'undefined' || !RegExpPrototypeExec(linkValueRegExp, value) @@ -510,7 +510,7 @@ function validateLinkHeaderFormat(value, name) { 'must be an array or string of format "; rel=preload; as=style"', ); } -} +}); const validateInternalField = hideStackFrames((object, fieldKey, className) => { if (typeof object !== 'object' || object === null || !ObjectPrototypeHasOwnProperty(object, fieldKey)) { @@ -522,9 +522,9 @@ const validateInternalField = hideStackFrames((object, fieldKey, className) => { * @param {any} hints * @return {string} */ -function validateLinkHeaderValue(hints) { +const validateLinkHeaderValue = hideStackFrames((hints) => { if (typeof hints === 'string') { - validateLinkHeaderFormat(hints, 'hints'); + validateLinkHeaderFormat.withoutStackTrace(hints, 'hints'); return hints; } else if (ArrayIsArray(hints)) { const hintsLength = hints.length; @@ -536,7 +536,7 @@ function validateLinkHeaderValue(hints) { for (let i = 0; i < hintsLength; i++) { const link = hints[i]; - validateLinkHeaderFormat(link, 'hints'); + validateLinkHeaderFormat.withoutStackTrace(link, 'hints'); result += link; if (i !== hintsLength - 1) { @@ -552,7 +552,7 @@ function validateLinkHeaderValue(hints) { hints, 'must be an array or string of format "; rel=preload; as=style"', ); -} +}); module.exports = { isInt32, diff --git a/lib/os.js b/lib/os.js index 34391697b5891c..e77b7f35b8dda8 100644 --- a/lib/os.js +++ b/lib/os.js @@ -65,7 +65,7 @@ function getCheckedFunction(fn) { const ctx = {}; const ret = fn(...args, ctx); if (ret === undefined) { - throw new ERR_SYSTEM_ERROR(ctx); + throw new ERR_SYSTEM_ERROR.HideStackFramesError(ctx); } return ret; }); diff --git a/lib/util.js b/lib/util.js index 7fb7994e6536ba..26a95b2a3daeff 100644 --- a/lib/util.js +++ b/lib/util.js @@ -32,6 +32,7 @@ const { DatePrototypeGetMonth, DatePrototypeGetSeconds, Error, + ErrorCaptureStackTrace, FunctionPrototypeBind, NumberIsSafeInteger, ObjectDefineProperties, @@ -53,7 +54,6 @@ const { }, errnoException, exceptionWithHostPort, - hideStackFrames, } = require('internal/errors'); const { format, @@ -278,16 +278,17 @@ function _extend(target, source) { return target; } -const callbackifyOnRejected = hideStackFrames((reason, cb) => { +const 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 // occurred", we error-wrap so the callback consumer can distinguish between // "the promise rejected with null" or "the promise fulfilled with undefined". if (!reason) { - reason = new ERR_FALSY_VALUE_REJECTION(reason); + reason = new ERR_FALSY_VALUE_REJECTION.HideStackFramesError(reason); + ErrorCaptureStackTrace(reason, callbackifyOnRejected); } return cb(reason); -}); +}; /** * @template {(...args: any[]) => Promise} T diff --git a/lib/zlib.js b/lib/zlib.js index 2b90c6f91fed76..3766938f6bc7bb 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -211,10 +211,10 @@ const checkFiniteNumber = hideStackFrames((number, name) => { return false; } - validateNumber(number, name); + validateNumber.withoutStackTrace(number, name); // Infinite numbers - throw new ERR_OUT_OF_RANGE(name, 'a finite number', number); + throw new ERR_OUT_OF_RANGE.HideStackFramesError(name, 'a finite number', number); }); // 1. Returns def for number when it's undefined or NaN @@ -223,12 +223,12 @@ const checkFiniteNumber = hideStackFrames((number, name) => { // 4. Throws ERR_OUT_OF_RANGE for infinite numbers or numbers > upper or < lower const checkRangesOrGetDefault = hideStackFrames( (number, name, lower, upper, def) => { - if (!checkFiniteNumber(number, name)) { + if (!checkFiniteNumber.withoutStackTrace(number, name)) { return def; } if (number < lower || number > upper) { - throw new ERR_OUT_OF_RANGE(name, - `>= ${lower} and <= ${upper}`, number); + throw new ERR_OUT_OF_RANGE.HideStackFramesError(name, + `>= ${lower} and <= ${upper}`, number); } return number; }, diff --git a/test/fixtures/errors/error_aggregateTwoErrors.snapshot b/test/fixtures/errors/error_aggregateTwoErrors.snapshot index a8d13c461b2cb0..a84ae702bb1e2e 100644 --- a/test/fixtures/errors/error_aggregateTwoErrors.snapshot +++ b/test/fixtures/errors/error_aggregateTwoErrors.snapshot @@ -2,7 +2,8 @@ throw aggregateTwoErrors(err, originalError); ^ -[AggregateError: original] { +AggregateError: original + at Function. (node:internal*errors:*:*) { code: 'ERR0', [errors]: [ Error: original diff --git a/test/parallel/test-errors-hide-stack-frames.js b/test/parallel/test-errors-hide-stack-frames.js new file mode 100644 index 00000000000000..3c4b5eafe1da30 --- /dev/null +++ b/test/parallel/test-errors-hide-stack-frames.js @@ -0,0 +1,208 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); +const { hideStackFrames, codes } = require('internal/errors'); +const { validateInteger } = require('internal/validators'); +const assert = require('assert'); + +{ + // Test that the a built-in error has the correct name and message. + function a() { + b(); + } + + function b() { + c(); + } + + const c = hideStackFrames(function() { + throw new Error('test'); + }); + + try { + a(); + } catch (e) { + assert.strictEqual(e.name, 'Error'); + assert.strictEqual(e.message, 'test'); + } +} + +{ + // Test that validator errors have the correct name and message. + try { + validateInteger('2', 'test'); + } catch (e) { + assert.strictEqual(e.name, 'TypeError'); + assert.strictEqual(e.message, 'The "test" argument must be of type number. Received type string (\'2\')'); + } +} + +{ + // Test that validator fn is not in the stack trace. + + function a(value) { + validateInteger(value, 'test'); + } + try { + a('2'); + } catch (e) { + assert.strictEqual(Error.stackTraceLimit, 10); + const stack = e.stack.split('\n'); + assert.doesNotMatch(stack[1], /validateInteger/); + assert.match(stack[1], /at a/); + } +} + +{ + // Test that the stack trace is hidden for normal unnamed functions. + function a() { + b(); + } + + function b() { + c(); + } + + const c = hideStackFrames(function() { + throw new Error('test'); + }); + + try { + a(); + } catch (e) { + assert.strictEqual(Error.stackTraceLimit, 10); + const stack = e.stack.split('\n'); + assert.doesNotMatch(stack[1], /at c/); + assert.match(stack[1], /at b/); + assert.strictEqual(Error.stackTraceLimit, 10); + } +} + +{ + // Test that the stack trace is hidden for normal functions. + function a() { + b(); + } + + function b() { + c(); + } + + const c = hideStackFrames(function c() { + throw new Error('test'); + }); + + try { + a(); + } catch (e) { + assert.strictEqual(Error.stackTraceLimit, 10); + const stack = e.stack.split('\n'); + assert.doesNotMatch(stack[1], /at c/); + assert.match(stack[1], /at b/); + assert.strictEqual(Error.stackTraceLimit, 10); + } +} + +{ + // Test that the stack trace is hidden for arrow functions. + function a() { + b(); + } + + function b() { + c(); + } + + const c = hideStackFrames(() => { + throw new Error('test'); + }); + + try { + a(); + } catch (e) { + assert.strictEqual(Error.stackTraceLimit, 10); + const stack = e.stack.split('\n'); + assert.doesNotMatch(stack[1], /at c/); + assert.match(stack[1], /at b/); + assert.strictEqual(Error.stackTraceLimit, 10); + } +} + +{ + // Creating a new Error object without stack trace, then throwing it + // should get a stack trace by hideStackFrames. + function a() { + b(); + } + + function b() { + c(); + } + + const c = hideStackFrames(function() { + throw new Error('test'); + }); + + try { + a(); + } catch (e) { + assert.strictEqual(Error.stackTraceLimit, 10); + const stack = e.stack.split('\n'); + assert.doesNotMatch(stack[1], /at c/); + assert.match(stack[1], /at b/); + assert.strictEqual(Error.stackTraceLimit, 10); + } +} + +{ + const ERR_ACCESS_DENIED = codes.ERR_ACCESS_DENIED; + // Creating a new Error object without stack trace, then throwing it + // should get a stack trace by hideStackFrames. + function a() { + b(); + } + + function b() { + c(); + } + + const c = hideStackFrames(function() { + throw new ERR_ACCESS_DENIED.NoStackError('test'); + }); + + try { + a(); + } catch (e) { + assert.strictEqual(Error.stackTraceLimit, 10); + const stack = e.stack.split('\n'); + assert.doesNotMatch(stack[1], /at c/); + assert.match(stack[1], /at b/); + assert.strictEqual(Error.stackTraceLimit, 10); + } +} + +{ + // Creating a new Error object with stack trace, then throwing it + // should get a stack trace by hideStackFrames. + function a() { + b(); + } + + const b = hideStackFrames(function b() { + c(); + }); + + const c = hideStackFrames(function() { + throw new Error('test'); + }); + + try { + a(); + } catch (e) { + assert.strictEqual(Error.stackTraceLimit, 10); + const stack = e.stack.split('\n'); + assert.match(stack[1], /at a/); + assert.strictEqual(Error.stackTraceLimit, 10); + } +} diff --git a/test/parallel/test-util-callbackify.js b/test/parallel/test-util-callbackify.js index 444e10b16bf85a..f287c91e946217 100644 --- a/test/parallel/test-util-callbackify.js +++ b/test/parallel/test-util-callbackify.js @@ -225,6 +225,7 @@ const values = [ const errLines = stderr.trim().split(/[\r\n]+/); const errLine = errLines.find((l) => /^Error/.exec(l)); assert.strictEqual(errLine, `Error: ${fixture}`); + assert.strictEqual(errLines.length, 7); }) ); } @@ -279,3 +280,20 @@ const values = [ }); }); } + +{ + // Test Promise factory + function promiseFn(value) { + return Promise.reject(value); + } + + const cbPromiseFn = callbackify(promiseFn); + + cbPromiseFn(null, (err) => { + assert.strictEqual(err.message, 'Promise was rejected with falsy value'); + assert.strictEqual(err.code, 'ERR_FALSY_VALUE_REJECTION'); + assert.strictEqual(err.reason, null); + const stack = err.stack.split(/[\r\n]+/); + assert.match(stack[1], /at process\.processTicksAndRejections/); + }); +}