lib: validate Error.captureStackTrace() calls

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>
nodejs · Mar 18, 2019 · 3fe1e80896d69b2125e3a264d0707fdbc6f37740 · 3fe1e80
1 parent bfbce28
commit 3fe1e80896d69b2125e3a264d0707fdbc6f37740
diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml
@@ -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

diff --git a/lib/assert.js b/lib/assert.js
@@ -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;

diff --git a/lib/events.js b/lib/events.js
@@ -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),

diff --git a/lib/fs.js b/lib/fs.js
@@ -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;
  }

diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js
@@ -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;

diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js
@@ -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);
  }

diff --git a/lib/internal/console/constructor.js b/lib/internal/console/constructor.js
@@ -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);
  },

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -354,6 +354,7 @@ function uvException(ctx) {
    err.dest = dest;
  }
+  
+  // eslint-disable-next-line no-restricted-syntax
  Error.captureStackTrace(err, excludedStackFn || uvException);
  return err;
 }
@@ -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;
 }
@@ -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;
 }
@@ -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;
 }
@@ -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;
 }

diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js
@@ -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);