Skip to content

Commit

Permalink
errors: slights improvements for hideStackFrames
Browse files Browse the repository at this point in the history
As suggested by @puzpuzpuz and @BridgeAR
* Do not run the full loop for user-generated exceptions
* Reduce the prefix
* Add function names to the functions in errors.js
so that they keep their names in the debugger/profiler

Refs: nodejs#35644
  • Loading branch information
mmomtchev committed Oct 15, 2020
1 parent c947273 commit c205a80
Showing 1 changed file with 49 additions and 43 deletions.
92 changes: 49 additions & 43 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const MainContextError = Error;
const overrideStackTrace = new WeakMap();
const kNoOverride = Symbol('kNoOverride');
let userStackTraceLimit = Error.stackTraceLimit;
const nodeInternalPrefix = '__node_internal_';
const prepareStackTrace = (globalThis, error, trace) => {
// API for node internals to override error stack formatting
// without interfering with userland code.
Expand All @@ -65,15 +66,18 @@ const prepareStackTrace = (globalThis, error, trace) => {
return f(error, trace);
}

for (let l = trace.length - 1; l >= 0; l--) {
const fn = trace[l].getFunctionName();
if (fn != null && fn.startsWith('__node_internal_hidden_')) {
trace.splice(0, l + 1);
break;
const firstFrame = trace[0]?.getFunctionName();
if (firstFrame && firstFrame.startsWith(nodeInternalPrefix)) {
for (let l = trace.length - 1; l >= 0; l--) {
const fn = trace[l].getFunctionName();
if (fn != null && fn.startsWith(nodeInternalPrefix)) {
trace.splice(0, l + 1);
break;
}
}
if (trace.length > userStackTraceLimit)
trace.splice(userStackTraceLimit);
}
if (trace.length > userStackTraceLimit)
trace.splice(userStackTraceLimit);

const globalOverride =
maybeOverridePrepareStackTrace(globalThis, error, trace);
Expand Down Expand Up @@ -136,7 +140,7 @@ function lazyBuffer() {
return buffer;
}

const addCodeToName = hideStackFrames(function(err, name, code) {
const addCodeToName = hideStackFrames(function addCodeToName(err, name, code) {
// Set the stack
err = captureLargerStackTrace(err);
// Add the error code to the name to include it in the stack trace.
Expand Down Expand Up @@ -309,7 +313,7 @@ function makeNodeErrorWithCode(Base, key) {
function hideStackFrames(fn) {
// We rename the functions that will be hidden to cut off the stacktrace
// at the outermost one
const hidden = '__node_internal_hidden_' + fn.name;
const hidden = nodeInternalPrefix + fn.name;
ObjectDefineProperty(fn, 'name', { value: hidden });
return fn;
}
Expand Down Expand Up @@ -380,16 +384,17 @@ function uvErrmapGet(name) {
return uvBinding.errmap.get(name);
}

function captureLargerStackTrace(err) {
userStackTraceLimit = Error.stackTraceLimit;
Error.stackTraceLimit = Infinity;
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(err);
// Reset the limit and setting the name property.
Error.stackTraceLimit = userStackTraceLimit;
const captureLargerStackTrace = hideStackFrames(
function captureLargerStackTrace(err) {
userStackTraceLimit = Error.stackTraceLimit;
Error.stackTraceLimit = Infinity;
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(err);
// Reset the limit
Error.stackTraceLimit = userStackTraceLimit;

return err;
}
return err;
});

/**
* This creates an error compatible with errors produced in the C++
Expand All @@ -400,7 +405,7 @@ function captureLargerStackTrace(err) {
* @param {Object} ctx
* @returns {Error}
*/
const uvException = hideStackFrames(function(ctx) {
const uvException = hideStackFrames(function uvException(ctx) {
const [code, uvmsg] = uvErrmapGet(ctx.errno) || uvUnmappedError;
let message = `${code}: ${ctx.message || uvmsg}, ${ctx.syscall}`;

Expand Down Expand Up @@ -455,8 +460,8 @@ const uvException = hideStackFrames(function(ctx) {
* @param {number} [port]
* @returns {Error}
*/
const uvExceptionWithHostPort =
hideStackFrames(function(err, syscall, address, port) {
const uvExceptionWithHostPort = hideStackFrames(
function uvExceptionWithHostPort(err, syscall, address, port) {
const [code, uvmsg] = uvErrmapGet(err) || uvUnmappedError;
const message = `${syscall} ${code}: ${uvmsg}`;
let details = '';
Expand Down Expand Up @@ -495,28 +500,29 @@ const uvExceptionWithHostPort =
* @param {string} [original]
* @returns {Error}
*/
const errnoException = hideStackFrames(function(err, syscall, original) {
// TODO(joyeecheung): We have to use the type-checked
// getSystemErrorName(err) to guard against invalid arguments from users.
// This can be replaced with [ code ] = errmap.get(err) when this method
// is no longer exposed to user land.
if (util === undefined) util = require('util');
const code = util.getSystemErrorName(err);
const message = original ?
`${syscall} ${code} ${original}` : `${syscall} ${code}`;
const errnoException = hideStackFrames(
function errnoException(err, syscall, original) {
// TODO(joyeecheung): We have to use the type-checked
// getSystemErrorName(err) to guard against invalid arguments from users.
// This can be replaced with [ code ] = errmap.get(err) when this method
// is no longer exposed to user land.
if (util === undefined) util = require('util');
const code = util.getSystemErrorName(err);
const message = original ?
`${syscall} ${code} ${original}` : `${syscall} ${code}`;

const tmpLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
// eslint-disable-next-line no-restricted-syntax
const ex = new Error(message);
Error.stackTraceLimit = tmpLimit;
ex.errno = err;
ex.code = code;
ex.syscall = syscall;
const tmpLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
// eslint-disable-next-line no-restricted-syntax
const ex = new Error(message);
Error.stackTraceLimit = tmpLimit;
ex.errno = err;
ex.code = code;
ex.syscall = syscall;

captureLargerStackTrace(ex);
return ex;
});
captureLargerStackTrace(ex);
return ex;
});

/**
* Deprecated, new function is `uvExceptionWithHostPort()`
Expand All @@ -529,8 +535,8 @@ const errnoException = hideStackFrames(function(err, syscall, original) {
* @param {string} [additional]
* @returns {Error}
*/
const exceptionWithHostPort =
hideStackFrames(function(err, syscall, address, port, additional) {
const exceptionWithHostPort = hideStackFrames(
function exceptionWithHostPort(err, syscall, address, port, additional) {
// TODO(joyeecheung): We have to use the type-checked
// getSystemErrorName(err) to guard against invalid arguments from users.
// This can be replaced with [ code ] = errmap.get(err) when this method
Expand Down

0 comments on commit c205a80

Please sign in to comment.