lib: avoid mutating Error.stackTraceLimit when it is not writable

PR-URL: #38215 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nodejs · Apr 14, 2021 · 09c9e5dea42fd11606f0d8988462e291cbfafd32 · 09c9e5d
1 parent 98c2067
commit 09c9e5dea42fd11606f0d8988462e291cbfafd32
diff --git a/lib/assert.js b/lib/assert.js
@@ -56,6 +56,7 @@ const {
     ERR_INVALID_RETURN_VALUE,
     ERR_MISSING_ARGS,
   },
+  isErrorStackTraceLimitWritable,
   overrideStackTrace,
 } = require('internal/errors');
 const AssertionError = require('internal/assert/assertion_error');
@@ -282,14 +283,15 @@ function parseCode(code, offset) {
 
 function getErrMessage(message, fn) {
   const tmpLimit = Error.stackTraceLimit;
+  const errorStackTraceLimitIsWritable = isErrorStackTraceLimitWritable();
   // Make sure the limit is set to 1. Otherwise it could fail (<= 0) or it
   // does to much work.
-  Error.stackTraceLimit = 1;
+  if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = 1;
   // We only need the stack trace. To minimize the overhead use an object
   // instead of an error.
   const err = {};
   ErrorCaptureStackTrace(err, fn);
-  Error.stackTraceLimit = tmpLimit;
+  if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = tmpLimit;
 
   overrideStackTrace.set(err, (_, stack) => stack);
   const call = err.stack[0];
@@ -326,7 +328,7 @@ function getErrMessage(message, fn) {
   try {
     // Set the stack trace limit to zero. This makes sure unexpected token
     // errors are handled faster.
-    Error.stackTraceLimit = 0;
+    if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = 0;
 
     if (filename) {
       if (decoder === undefined) {
@@ -371,7 +373,7 @@ function getErrMessage(message, fn) {
     errorCache.set(identifier, undefined);
   } finally {
     // Reset limit.
-    Error.stackTraceLimit = tmpLimit;
+    if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = tmpLimit;
     if (fd !== undefined)
       closeSync(fd);
   }

diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js
@@ -24,6 +24,7 @@ const {
 const {
   validateObject,
 } = require('internal/validators');
+const { isErrorStackTraceLimitWritable } = require('internal/errors');
 
 let blue = '';
 let green = '';
@@ -341,7 +342,7 @@ class AssertionError extends Error {
     } = options;
 
     const limit = Error.stackTraceLimit;
-    Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
 
     if (message != null) {
       super(String(message));
@@ -436,7 +437,7 @@ class AssertionError extends Error {
       }
     }
 
-    Error.stackTraceLimit = limit;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
 
     this.generatedMessage = !message;
     ObjectDefineProperty(this, 'name', {

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -33,7 +33,10 @@ const {
   Number,
   NumberIsInteger,
   ObjectDefineProperty,
+  ObjectIsExtensible,
+  ObjectGetOwnPropertyDescriptor,
   ObjectKeys,
+  ObjectPrototypeHasOwnProperty,
   RangeError,
   ReflectApply,
   RegExpPrototypeTest,
@@ -204,6 +207,17 @@ const addCodeToName = hideStackFrames(function addCodeToName(err, name, code) {
   }
 });
 
+function isErrorStackTraceLimitWritable() {
+  const desc = ObjectGetOwnPropertyDescriptor(Error, 'stackTraceLimit');
+  if (desc === undefined) {
+    return ObjectIsExtensible(Error);
+  }
+
+  return ObjectPrototypeHasOwnProperty(desc, 'writable') ?
+    desc.writable :
+    desc.set !== undefined;
+}
+
 // A specialized Error that includes an additional info property with
 // additional information about the error condition.
 // It has the properties present in a UVException but with a custom error
@@ -215,10 +229,10 @@ const addCodeToName = hideStackFrames(function addCodeToName(err, name, code) {
 class SystemError extends Error {
   constructor(key, context) {
     const limit = Error.stackTraceLimit;
-    Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
     super();
     // Reset the limit and setting the name property.
-    Error.stackTraceLimit = limit;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
     const prefix = getMessage(key, [], this);
     let message = `${prefix}: ${context.syscall} returned ` +
                   `${context.code} (${context.message})`;
@@ -327,10 +341,10 @@ function makeSystemErrorWithCode(key) {
 function makeNodeErrorWithCode(Base, key) {
   return function NodeError(...args) {
     const limit = Error.stackTraceLimit;
-    Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
     const error = new Base();
     // Reset the limit and setting the name property.
-    Error.stackTraceLimit = limit;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
     const message = getMessage(key, args, error);
     ObjectDefineProperty(error, 'message', {
       value: message,
@@ -434,11 +448,14 @@ function uvErrmapGet(name) {
 
 const captureLargerStackTrace = hideStackFrames(
   function captureLargerStackTrace(err) {
-    userStackTraceLimit = Error.stackTraceLimit;
-    Error.stackTraceLimit = Infinity;
+    const stackTraceLimitIsWritable = isErrorStackTraceLimitWritable();
+    if (stackTraceLimitIsWritable) {
+      userStackTraceLimit = Error.stackTraceLimit;
+      Error.stackTraceLimit = Infinity;
+    }
     ErrorCaptureStackTrace(err);
     // Reset the limit
-    Error.stackTraceLimit = userStackTraceLimit;
+    if (stackTraceLimitIsWritable) Error.stackTraceLimit = userStackTraceLimit;
 
     return err;
   });
@@ -471,12 +488,12 @@ const uvException = hideStackFrames(function uvException(ctx) {
   // the stack frames due to the `captureStackTrace()` function that is called
   // later.
   const tmpLimit = Error.stackTraceLimit;
-  Error.stackTraceLimit = 0;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
   // Pass the message to the constructor instead of setting it on the object
   // to make sure it is the same as the one created in C++
   // eslint-disable-next-line no-restricted-syntax
   const err = new Error(message);
-  Error.stackTraceLimit = tmpLimit;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
 
   for (const prop of ObjectKeys(ctx)) {
     if (prop === 'message' || prop === 'path' || prop === 'dest') {
@@ -523,10 +540,10 @@ const uvExceptionWithHostPort = hideStackFrames(
     // lose the stack frames due to the `captureStackTrace()` function that
     // is called later.
     const tmpLimit = Error.stackTraceLimit;
-    Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
     // eslint-disable-next-line no-restricted-syntax
     const ex = new Error(`${message}${details}`);
-    Error.stackTraceLimit = tmpLimit;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
     ex.code = code;
     ex.errno = err;
     ex.syscall = syscall;
@@ -558,10 +575,10 @@ const errnoException = hideStackFrames(
       `${syscall} ${code} ${original}` : `${syscall} ${code}`;
 
     const tmpLimit = Error.stackTraceLimit;
-    Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
     // eslint-disable-next-line no-restricted-syntax
     const ex = new Error(message);
-    Error.stackTraceLimit = tmpLimit;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
     ex.errno = err;
     ex.code = code;
     ex.syscall = syscall;
@@ -602,10 +619,10 @@ const exceptionWithHostPort = hideStackFrames(
     // lose the stack frames due to the `captureStackTrace()` function that
     // is called later.
     const tmpLimit = Error.stackTraceLimit;
-    Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
     // eslint-disable-next-line no-restricted-syntax
     const ex = new Error(`${syscall} ${code}${details}`);
-    Error.stackTraceLimit = tmpLimit;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
     ex.errno = err;
     ex.code = code;
     ex.syscall = syscall;
@@ -647,10 +664,10 @@ const dnsException = hideStackFrames(function(code, syscall, hostname) {
   // the stack frames due to the `captureStackTrace()` function that is called
   // later.
   const tmpLimit = Error.stackTraceLimit;
-  Error.stackTraceLimit = 0;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
   // eslint-disable-next-line no-restricted-syntax
   const ex = new Error(message);
-  Error.stackTraceLimit = tmpLimit;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
   ex.errno = errno;
   ex.code = code;
   ex.syscall = syscall;
@@ -783,6 +800,7 @@ module.exports = {
   exceptionWithHostPort,
   getMessage,
   hideStackFrames,
+  isErrorStackTraceLimitWritable,
   isStackOverflowError,
   connResetException,
   uvErrmapGet,

diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js
@@ -29,6 +29,7 @@ const {
   popAsyncContext,
 } = require('internal/async_hooks');
 const async_hooks = require('async_hooks');
+const { isErrorStackTraceLimitWritable } = require('internal/errors');
 
 // *Must* match Environment::TickInfo::Fields in src/env.h.
 const kHasRejectionToWarn = 1;
@@ -264,10 +265,10 @@ function processPromiseRejections() {
 function getErrorWithoutStack(name, message) {
   // Reset the stack to prevent any overhead.
   const tmp = Error.stackTraceLimit;
-  Error.stackTraceLimit = 0;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
   // eslint-disable-next-line no-restricted-syntax
   const err = new Error(message);
-  Error.stackTraceLimit = tmp;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmp;
   ObjectDefineProperty(err, 'name', {
     value: name,
     enumerable: false,

diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js
@@ -9,7 +9,12 @@ const {
 } = primordials;
 
 const assert = require('internal/assert');
-const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
+const {
+  codes: {
+    ERR_INVALID_ARG_TYPE,
+  },
+  isErrorStackTraceLimitWritable,
+} = require('internal/errors');
 const { validateString } = require('internal/validators');
 
 // Lazily loaded
@@ -157,10 +162,10 @@ function createWarningObject(warning, type, code, ctor, detail) {
   // Improve error creation performance by skipping the error frames.
   // They are added in the `captureStackTrace()` function below.
   const tmpStackLimit = Error.stackTraceLimit;
-  Error.stackTraceLimit = 0;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
   // eslint-disable-next-line no-restricted-syntax
   warning = new Error(warning);
-  Error.stackTraceLimit = tmpStackLimit;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpStackLimit;
   warning.name = String(type || 'Warning');
   if (code !== undefined) warning.code = code;
   if (detail !== undefined) warning.detail = detail;

diff --git a/lib/internal/util.js b/lib/internal/util.js
@@ -385,7 +385,7 @@ function isInsideNodeModules() {
     // side-effect-free. Since this is currently only used for a deprecated API,
     // the perf implications should be okay.
     getStructuredStack = runInNewContext(`(function() {
-      Error.stackTraceLimit = Infinity;
+      try { Error.stackTraceLimit = Infinity; } catch {}
       return function structuredStack() {
         const e = new Error();
         overrideStackTrace.set(e, (err, trace) => trace);

diff --git a/lib/repl.js b/lib/repl.js
@@ -140,6 +140,7 @@ const {
     ERR_INVALID_REPL_INPUT,
     ERR_SCRIPT_EXECUTION_INTERRUPTED,
   },
+  isErrorStackTraceLimitWritable,
   overrideStackTrace,
 } = require('internal/errors');
 const { sendInspectorCommand } = require('internal/util/inspector');
@@ -554,9 +555,9 @@ function REPLServer(prompt,
           const interrupt = new Promise((resolve, reject) => {
             sigintListener = () => {
               const tmp = Error.stackTraceLimit;
-              Error.stackTraceLimit = 0;
+              if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
               const err = new ERR_SCRIPT_EXECUTION_INTERRUPTED();
-              Error.stackTraceLimit = tmp;
+              if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmp;
               reject(err);
             };
             prioritizedSigintQueue.add(sigintListener);

diff --git a/test/parallel/test-errors-systemerror-frozen-intrinsics.js b/test/parallel/test-errors-systemerror-frozen-intrinsics.js
@@ -0,0 +1,24 @@
+// Flags: --expose-internals --frozen-intrinsics
+'use strict';
+require('../common');
+const assert = require('assert');
+const { E, SystemError, codes } = require('internal/errors');
+
+E('ERR_TEST', 'custom message', SystemError);
+const { ERR_TEST } = codes;
+
+const ctx = {
+  code: 'ETEST',
+  message: 'code message',
+  syscall: 'syscall_test',
+  path: '/str',
+  dest: '/str2'
+};
+assert.throws(
+  () => { throw new ERR_TEST(ctx); },
+  {
+    code: 'ERR_TEST',
+    name: 'SystemError',
+    info: ctx,
+  }
+);
diff --git a/test/parallel/test-errors-systemerror-stackTraceLimit-custom-setter.js b/test/parallel/test-errors-systemerror-stackTraceLimit-custom-setter.js
@@ -0,0 +1,30 @@
+// Flags: --expose-internals
+'use strict';
+require('../common');
+const assert = require('assert');
+const { E, SystemError, codes } = require('internal/errors');
+
+let stackTraceLimit;
+Reflect.defineProperty(Error, 'stackTraceLimit', {
+  get() { return stackTraceLimit; },
+  set(value) { stackTraceLimit = value; },
+});
+
+E('ERR_TEST', 'custom message', SystemError);
+const { ERR_TEST } = codes;
+
+const ctx = {
+  code: 'ETEST',
+  message: 'code message',
+  syscall: 'syscall_test',
+  path: '/str',
+  dest: '/str2'
+};
+assert.throws(
+  () => { throw new ERR_TEST(ctx); },
+  {
+    code: 'ERR_TEST',
+    name: 'SystemError',
+    info: ctx,
+  }
+);
diff --git a/test/parallel/test-errors-systemerror-stackTraceLimit-deleted-and-Error-sealed.js b/test/parallel/test-errors-systemerror-stackTraceLimit-deleted-and-Error-sealed.js
@@ -0,0 +1,27 @@
+// Flags: --expose-internals
+'use strict';
+require('../common');
+const assert = require('assert');
+const { E, SystemError, codes } = require('internal/errors');
+
+delete Error.stackTraceLimit;
+Object.seal(Error);
+
+E('ERR_TEST', 'custom message', SystemError);
+const { ERR_TEST } = codes;
+
+const ctx = {
+  code: 'ETEST',
+  message: 'code message',
+  syscall: 'syscall_test',
+  path: '/str',
+  dest: '/str2'
+};
+assert.throws(
+  () => { throw new ERR_TEST(ctx); },
+  {
+    code: 'ERR_TEST',
+    name: 'SystemError',
+    info: ctx,
+  }
+);