Skip to content

Commit

Permalink
lib: avoid mutating Error.stackTraceLimit when it is not writable
Browse files Browse the repository at this point in the history
PR-URL: #38215
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
aduh95 committed Apr 14, 2021
1 parent 98c2067 commit 09c9e5d
Show file tree
Hide file tree
Showing 13 changed files with 221 additions and 31 deletions.
10 changes: 6 additions & 4 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const {
ERR_INVALID_RETURN_VALUE,
ERR_MISSING_ARGS,
},
isErrorStackTraceLimitWritable,
overrideStackTrace,
} = require('internal/errors');
const AssertionError = require('internal/assert/assertion_error');
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/assert/assertion_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const {
const {
validateObject,
} = require('internal/validators');
const { isErrorStackTraceLimitWritable } = require('internal/errors');

let blue = '';
let green = '';
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -436,7 +437,7 @@ class AssertionError extends Error {
}
}

Error.stackTraceLimit = limit;
if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;

this.generatedMessage = !message;
ObjectDefineProperty(this, 'name', {
Expand Down
52 changes: 35 additions & 17 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ const {
Number,
NumberIsInteger,
ObjectDefineProperty,
ObjectIsExtensible,
ObjectGetOwnPropertyDescriptor,
ObjectKeys,
ObjectPrototypeHasOwnProperty,
RangeError,
ReflectApply,
RegExpPrototypeTest,
Expand Down Expand Up @@ -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
Expand All @@ -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})`;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
});
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -783,6 +800,7 @@ module.exports = {
exceptionWithHostPort,
getMessage,
hideStackFrames,
isErrorStackTraceLimitWritable,
isStackOverflowError,
connResetException,
uvErrmapGet,
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/process/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 8 additions & 3 deletions lib/internal/process/warning.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ const {
ERR_INVALID_REPL_INPUT,
ERR_SCRIPT_EXECUTION_INTERRUPTED,
},
isErrorStackTraceLimitWritable,
overrideStackTrace,
} = require('internal/errors');
const { sendInspectorCommand } = require('internal/util/inspector');
Expand Down Expand Up @@ -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);
Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-errors-systemerror-frozen-intrinsics.js
Original file line number Diff line number Diff line change
@@ -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,
}
);
Original file line number Diff line number Diff line change
@@ -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,
}
);
Original file line number Diff line number Diff line change
@@ -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,
}
);
Loading

0 comments on commit 09c9e5d

Please sign in to comment.