From 2a85e0ac006f08fc17d0f813fe7b10ada0c06e6b Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 8 Feb 2022 17:56:15 -0800 Subject: [PATCH] lib: add internal genericNodeError() function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are a few places in lib where `new Error()` is called and then additional properties are attached in various ways. This creates a utility function to generate the errors. PR-URL: https://github.com/nodejs/node/pull/41879 Reviewed-By: James M Snell Reviewed-By: Tobias Nießen --- lib/buffer.js | 12 ++--- lib/child_process.js | 19 ++++---- lib/events.js | 14 +++--- lib/internal/errors.js | 45 +++++++++++++------ lib/net.js | 11 ++--- lib/zlib.js | 7 ++- .../sequential/test-child-process-execsync.js | 2 +- 7 files changed, 60 insertions(+), 50 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 482c2b6cab17a6..57d6cddbaa2e6b 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -25,7 +25,6 @@ const { Array, ArrayIsArray, ArrayPrototypeForEach, - Error, MathFloor, MathMin, MathTrunc, @@ -99,7 +98,8 @@ const { ERR_MISSING_ARGS, ERR_UNKNOWN_ENCODING }, - hideStackFrames + genericNodeError, + hideStackFrames, } = require('internal/errors'); const { validateArray, @@ -1207,10 +1207,10 @@ if (internalBinding('config').hasIntl) { return result; const code = icuErrName(result); - // eslint-disable-next-line no-restricted-syntax - const err = new Error(`Unable to transcode Buffer [${code}]`); - err.code = code; - err.errno = result; + const err = genericNodeError( + `Unable to transcode Buffer [${code}]`, + { code: code, errno: result } + ); throw err; }; } diff --git a/lib/child_process.js b/lib/child_process.js index a7ef8ba1e4af1a..415010241cdaba 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -32,7 +32,6 @@ const { ArrayPrototypeSort, ArrayPrototypeSplice, ArrayPrototypeUnshift, - Error, NumberIsInteger, ObjectAssign, ObjectDefineProperty, @@ -62,6 +61,7 @@ const { Pipe, constants: PipeConstants } = internalBinding('pipe_wrap'); const { AbortError, codes: errorCodes, + genericNodeError, } = require('internal/errors'); const { ERR_INVALID_ARG_VALUE, @@ -395,11 +395,11 @@ function execFile(file, args = [], options, callback) { cmd += ` ${ArrayPrototypeJoin(args, ' ')}`; if (!ex) { - // eslint-disable-next-line no-restricted-syntax - ex = new Error('Command failed: ' + cmd + '\n' + stderr); - ex.killed = child.killed || killed; - ex.code = code < 0 ? getSystemErrorName(code) : code; - ex.signal = signal; + ex = genericNodeError(`Command failed: ${cmd}\n${stderr}`, { + code: code < 0 ? getSystemErrorName(code) : code, + killed: child.killed || killed, + signal: signal + }); } ex.cmd = cmd; @@ -819,16 +819,13 @@ function checkExecSyncError(ret, args, cmd) { let err; if (ret.error) { err = ret.error; + ObjectAssign(err, ret); } else if (ret.status !== 0) { let msg = 'Command failed: '; msg += cmd || ArrayPrototypeJoin(args, ' '); if (ret.stderr && ret.stderr.length > 0) msg += `\n${ret.stderr.toString()}`; - // eslint-disable-next-line no-restricted-syntax - err = new Error(msg); - } - if (err) { - ObjectAssign(err, ret); + err = genericNodeError(msg, ret); } return err; } diff --git a/lib/events.js b/lib/events.js index e140c1a5f3d6bd..9162e8ca8b273c 100644 --- a/lib/events.js +++ b/lib/events.js @@ -65,6 +65,7 @@ const { ERR_OUT_OF_RANGE, ERR_UNHANDLED_ERROR }, + genericNodeError, } = require('internal/errors'); const { @@ -597,15 +598,10 @@ function _addListener(target, type, listener, prepend) { if (m > 0 && existing.length > m && !existing.warned) { existing.warned = true; // No error code for this since it is a Warning - // eslint-disable-next-line no-restricted-syntax - const w = new Error('Possible EventEmitter memory leak detected. ' + - `${existing.length} ${String(type)} listeners ` + - `added to ${inspect(target, { depth: -1 })}. Use ` + - 'emitter.setMaxListeners() to increase limit'); - w.name = 'MaxListenersExceededWarning'; - w.emitter = target; - w.type = type; - w.count = existing.length; + const w = genericNodeError( + `Possible EventEmitter memory leak detected. ${existing.length} ${String(type)} listeners ` + + `added to ${inspect(target, { depth: -1 })}. Use emitter.setMaxListeners() to increase limit`, + { name: 'MaxListenersExceededWarning', emitter: target, type: type, count: existing.length }); process.emitWarning(w); } } diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 0ef58432391f39..2cbac1735e2b29 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -33,6 +33,7 @@ const { MathMax, Number, NumberIsInteger, + ObjectAssign, ObjectDefineProperty, ObjectDefineProperties, ObjectIsExtensible, @@ -827,34 +828,50 @@ class AbortError extends Error { this.name = 'AbortError'; } } + +/** + * This creates a generic Node.js error. + * + * @param {string} message The error message. + * @param {object} errorProperties Object with additional properties to be added to the error. + * @returns {Error} + */ +const genericNodeError = hideStackFrames(function genericNodeError(message, errorProperties) { + // eslint-disable-next-line no-restricted-syntax + const err = new Error(message); + ObjectAssign(err, errorProperties); + return err; +}); + module.exports = { + AbortError, aggregateTwoErrors, + captureLargerStackTrace, codes, + connResetException, dnsException, + // This is exported only to facilitate testing. + E, errnoException, exceptionWithHostPort, + fatalExceptionStackEnhancers, + genericNodeError, getMessage, - hideStackFrames, hideInternalStackFrames, + hideStackFrames, isErrorStackTraceLimitWritable, isStackOverflowError, + kEnhanceStackBeforeInspector, + kIsNodeError, + kNoOverride, + maybeOverridePrepareStackTrace, + overrideStackTrace, + prepareStackTrace, setArrowMessage, - connResetException, + SystemError, uvErrmapGet, uvException, uvExceptionWithHostPort, - SystemError, - AbortError, - // This is exported only to facilitate testing. - E, - kNoOverride, - prepareStackTrace, - maybeOverridePrepareStackTrace, - overrideStackTrace, - kEnhanceStackBeforeInspector, - fatalExceptionStackEnhancers, - kIsNodeError, - captureLargerStackTrace, }; // To declare an error message, use the E(sym, val, def) function above. The sym diff --git a/lib/net.js b/lib/net.js index 9681616df62799..c5d46988c4dde3 100644 --- a/lib/net.js +++ b/lib/net.js @@ -25,7 +25,6 @@ const { ArrayIsArray, ArrayPrototypeIndexOf, Boolean, - Error, Number, NumberIsNaN, NumberParseInt, @@ -97,7 +96,8 @@ const { }, errnoException, exceptionWithHostPort, - uvExceptionWithHostPort + genericNodeError, + uvExceptionWithHostPort, } = require('internal/errors'); const { isUint8Array } = require('internal/util/types'); const { @@ -484,9 +484,10 @@ function writeAfterFIN(chunk, encoding, cb) { encoding = null; } - // eslint-disable-next-line no-restricted-syntax - const er = new Error('This socket has been ended by the other party'); - er.code = 'EPIPE'; + const er = genericNodeError( + 'This socket has been ended by the other party', + { code: 'EPIPE' } + ); if (typeof cb === 'function') { defaultTriggerAsyncIdScope(this[async_id_symbol], process.nextTick, cb, er); } diff --git a/lib/zlib.js b/lib/zlib.js index 35e16534fb3d52..718f72af67650e 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -26,7 +26,6 @@ const { ArrayPrototypeForEach, ArrayPrototypeMap, ArrayPrototypePush, - Error, FunctionPrototypeBind, MathMaxApply, NumberIsFinite, @@ -51,7 +50,8 @@ const { ERR_OUT_OF_RANGE, ERR_ZLIB_INITIALIZATION_FAILED, }, - hideStackFrames + genericNodeError, + hideStackFrames, } = require('internal/errors'); const { Transform, finished } = require('stream'); const { @@ -186,8 +186,7 @@ function zlibOnError(message, errno, code) { // There is no way to cleanly recover. // Continuing only obscures problems. - // eslint-disable-next-line no-restricted-syntax - const error = new Error(message); + const error = genericNodeError(message, { errno, code }); error.errno = errno; error.code = code; self.destroy(error); diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js index bc589efb8b5b64..5512eaeed7af35 100644 --- a/test/sequential/test-child-process-execsync.js +++ b/test/sequential/test-child-process-execsync.js @@ -60,7 +60,7 @@ try { assert.ok(caught, 'execSync should throw'); const end = Date.now() - start; assert(end < SLEEP); - assert(err.status > 128 || err.signal); + assert(err.status > 128 || err.signal, `status: ${err.status}, signal: ${err.signal}`); } assert.throws(function() {