Skip to content

Commit

Permalink
lib: validate Error.captureStackTrace() calls
Browse files Browse the repository at this point in the history
This adds a custom eslint rule to verify that
`Error.captureStackTrace()` is only called if necessary. In most
cases the helper function should be used instead.

PR-URL: #26738
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
BridgeAR committed Mar 23, 2019
1 parent bfbce28 commit 3fe1e80
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ rules:
# Config specific to lib
- selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError)$/])"
message: "Use an error exported by the internal/errors module."
- selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']"
message: "Please use `require('internal/errors').hideStackFrames()` instead."
# Custom rules in tools/eslint-rules
node-core/require-globals: error
node-core/no-let-in-for-declaration: error
Expand Down
1 change: 1 addition & 0 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ function getErrMessage(message, fn) {
// We only need the stack trace. To minimize the overhead use an object
// instead of an error.
const err = {};
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(err, fn);
Error.stackTraceLimit = tmpLimit;

Expand Down
1 change: 1 addition & 0 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
try {
const { kExpandStackSymbol } = require('internal/util');
const capture = {};
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(capture, EventEmitter.prototype.emit);
Object.defineProperty(er, kExpandStackSymbol, {
value: enhanceStackTrace.bind(null, er, capture),
Expand Down
2 changes: 2 additions & 0 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,15 @@ function showTruncateDeprecation() {
function handleErrorFromBinding(ctx) {
if (ctx.errno !== undefined) { // libuv error numbers
const err = uvException(ctx);
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(err, handleErrorFromBinding);
throw err;
}
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.
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(ctx.error, handleErrorFromBinding);
throw ctx.error;
}
Expand Down
1 change: 1 addition & 0 deletions lib/internal/assert/assertion_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ class AssertionError extends Error {
this.actual = actual;
this.expected = expected;
this.operator = operator;
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(this, stackStartFn);
// Create error message including the error code in the name.
this.stack;
Expand Down
1 change: 1 addition & 0 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ function fatalError(e) {
process._rawDebug(e.stack);
} else {
const o = { message: e };
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(o, fatalError);
process._rawDebug(o.stack);
}
Expand Down
1 change: 1 addition & 0 deletions lib/internal/console/constructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ const consoleMethods = {
name: 'Trace',
message: this[kFormatForStderr](args)
};
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(err, this.trace);
this.error(err.stack);
},
Expand Down
5 changes: 5 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ function uvException(ctx) {
err.dest = dest;
}

// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(err, excludedStackFn || uvException);
return err;
}
Expand Down Expand Up @@ -396,6 +397,7 @@ function uvExceptionWithHostPort(err, syscall, address, port) {
ex.port = port;
}

// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(ex, excludedStackFn || uvExceptionWithHostPort);
return ex;
}
Expand Down Expand Up @@ -424,6 +426,7 @@ function errnoException(err, syscall, original) {
ex.code = ex.errno = code;
ex.syscall = syscall;

// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(ex, excludedStackFn || errnoException);
return ex;
}
Expand Down Expand Up @@ -472,6 +475,7 @@ function exceptionWithHostPort(err, syscall, address, port, additional) {
ex.port = port;
}

// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(ex, excludedStackFn || exceptionWithHostPort);
return ex;
}
Expand Down Expand Up @@ -512,6 +516,7 @@ function dnsException(code, syscall, hostname) {
ex.hostname = hostname;
}

// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(ex, excludedStackFn || dnsException);
return ex;
}
Expand Down
1 change: 1 addition & 0 deletions lib/internal/process/warning.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ function emitWarning(warning, type, code, ctor, now) {
warning.name = String(type || 'Warning');
if (code !== undefined) warning.code = code;
if (detail !== undefined) warning.detail = detail;
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(warning, ctor || process.emitWarning);
} else if (!(warning instanceof Error)) {
throw new ERR_INVALID_ARG_TYPE('warning', ['Error', 'string'], warning);
Expand Down

0 comments on commit 3fe1e80

Please sign in to comment.