Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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