Skip to content

Commit

Permalink
errors: improve hideStackFrames
Browse files Browse the repository at this point in the history
  • Loading branch information
Uzlopak committed Oct 1, 2023
1 parent 092fb9f commit a12adde
Show file tree
Hide file tree
Showing 16 changed files with 397 additions and 166 deletions.
62 changes: 62 additions & 0 deletions benchmark/error/hidestackframes-noerr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
'use strict';

const common = require('../common.js');
const assert = require('assert');

const bench = common.createBenchmark(main, {
type: [
'hide-stackframes',
'direct-call',
],
n: [1e7],
}, {
flags: ['--expose-internals'],
});

function main({ n, type }) {
const {
hideStackFrames,
codes: {
ERR_INVALID_ARG_TYPE,
},
} = require('internal/errors');

const testfn = (value) => {
if (typeof value !== 'number') {
throw new ERR_INVALID_ARG_TYPE('Benchmark', 'number', value);
}
};

const hideStackFramesTestfn = hideStackFrames((value) => {
if (typeof value !== 'number') {
throw new ERR_INVALID_ARG_TYPE.HideStackFrameError('Benchmark', 'number', value);
}
});

const fn = type === 'hide-stackframe' ? hideStackFramesTestfn : testfn;

const value = 42;

const length = 1024;
const array = [];
const errCase = false;

for (let i = 0; i < length; ++i) {
array.push(fn(value));
}

bench.start();

for (let i = 0; i < n; i++) {
const index = i % length;
array[index] = fn(value);
}

bench.end(n);

// Verify the entries to prevent dead code elimination from making
// the benchmark invalid.
for (let i = 0; i < length; ++i) {
assert.strictEqual(typeof array[i], errCase ? 'object' : 'undefined');
}
}
72 changes: 72 additions & 0 deletions benchmark/error/hidestackframes-throw.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
'use strict';

const common = require('../common.js');
const assert = require('assert');

const bench = common.createBenchmark(main, {
type: [
'hide-stackframes',
'direct-call',
],
n: [1e5],
}, {
flags: ['--expose-internals'],
});

function main({ n, type }) {
const {
hideStackFrames,
codes: {
ERR_INVALID_ARG_TYPE,
},
} = require('internal/errors');

const value = 'err';

const testfn = (value) => {
if (typeof value !== 'number') {
throw new ERR_INVALID_ARG_TYPE('Benchmark', 'number', value);
}
};

const hideStackFramesTestfn = hideStackFrames((value) => {
if (typeof value !== 'number') {
throw new ERR_INVALID_ARG_TYPE.HideStackFrameError('Benchmark', 'number', value);
}
});

const fn = type === 'hide-stackframe' ? hideStackFramesTestfn : testfn;

const length = 1024;
const array = [];
let errCase = false;

// Warm up.
for (let i = 0; i < length; ++i) {
try {
fn(value);
} catch (e) {
errCase = true;
array.push(e);
}
}

bench.start();

for (let i = 0; i < n; i++) {
const index = i % length;
try {
fn(value);
} catch (e) {
array[index] = e;
}
}

bench.end(n);

// Verify the entries to prevent dead code elimination from making
// the benchmark invalid.
for (let i = 0; i < length; ++i) {
assert.strictEqual(typeof array[i], errCase ? 'object' : 'undefined');
}
}
45 changes: 0 additions & 45 deletions benchmark/error/hidestackframes.js

This file was deleted.

6 changes: 3 additions & 3 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,17 +619,17 @@ function matchHeader(self, state, field, value) {

const validateHeaderName = hideStackFrames((name, label) => {
if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) {
throw new ERR_INVALID_HTTP_TOKEN(label || 'Header name', name);
throw new ERR_INVALID_HTTP_TOKEN.HideStackFramesError(label || 'Header name', name);
}
});

const validateHeaderValue = hideStackFrames((name, value) => {
if (value === undefined) {
throw new ERR_HTTP_INVALID_HEADER_VALUE(value, name);
throw new ERR_HTTP_INVALID_HEADER_VALUE.HideStackFramesError(value, name);
}
if (checkInvalidHeaderChar(value)) {
debug('Header "%s" contains invalid characters', name);
throw new ERR_INVALID_CHAR('header content', name);
throw new ERR_INVALID_CHAR.HideStackFramesError('header content', name);
}
});

Expand Down
20 changes: 6 additions & 14 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ const {
ERR_UNKNOWN_ENCODING,
},
genericNodeError,
hideStackFrames,
} = require('internal/errors');
const {
validateArray,
Expand Down Expand Up @@ -386,19 +385,12 @@ Buffer.of = of;

ObjectSetPrototypeOf(Buffer, Uint8Array);

// The 'assertSize' method will remove itself from the callstack when an error
// occurs. This is done simply to keep the internal details of the
// implementation from bleeding out to users.
const assertSize = hideStackFrames((size) => {
validateNumber(size, 'size', 0, kMaxLength);
});

/**
* Creates a new filled Buffer instance.
* alloc(size[, fill[, encoding]])
*/
Buffer.alloc = function alloc(size, fill, encoding) {
assertSize(size);
validateNumber(size, 'size', 0, kMaxLength);
if (fill !== undefined && fill !== 0 && size > 0) {
const buf = createUnsafeBuffer(size);
return _fill(buf, fill, 0, buf.length, encoding);
Expand All @@ -411,7 +403,7 @@ Buffer.alloc = function alloc(size, fill, encoding) {
* instance. If `--zero-fill-buffers` is set, will zero-fill the buffer.
*/
Buffer.allocUnsafe = function allocUnsafe(size) {
assertSize(size);
validateNumber(size, 'size', 0, kMaxLength);
return allocate(size);
};

Expand All @@ -421,15 +413,15 @@ Buffer.allocUnsafe = function allocUnsafe(size) {
* If `--zero-fill-buffers` is set, will zero-fill the buffer.
*/
Buffer.allocUnsafeSlow = function allocUnsafeSlow(size) {
assertSize(size);
validateNumber(size, 'size', 0, kMaxLength);
return createUnsafeBuffer(size);
};

// If --zero-fill-buffers command line argument is set, a zero-filled
// buffer is returned.
function SlowBuffer(length) {
assertSize(length);
return createUnsafeBuffer(length);
function SlowBuffer(size) {
validateNumber(size, 'size', 0, kMaxLength);
return createUnsafeBuffer(size);
}

ObjectSetPrototypeOf(SlowBuffer.prototype, Uint8ArrayPrototype);
Expand Down
68 changes: 40 additions & 28 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ const MainContextError = Error;
const overrideStackTrace = new SafeWeakMap();
const kNoOverride = Symbol('kNoOverride');
let userStackTraceLimit;
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 @@ -97,21 +96,6 @@ const prepareStackTrace = (globalThis, error, trace) => {
return f(error, trace);
}

const firstFrame = trace[0]?.getFunctionName();
if (firstFrame && StringPrototypeStartsWith(firstFrame, nodeInternalPrefix)) {
for (let l = trace.length - 1; l >= 0; l--) {
const fn = trace[l]?.getFunctionName();
if (fn && StringPrototypeStartsWith(fn, nodeInternalPrefix)) {
ArrayPrototypeSplice(trace, 0, l + 1);
break;
}
}
// `userStackTraceLimit` is the user value for `Error.stackTraceLimit`,
// it is updated at every new exception in `captureLargerStackTrace`.
if (trace.length > userStackTraceLimit)
ArrayPrototypeSplice(trace, userStackTraceLimit);
}

const globalOverride =
maybeOverridePrepareStackTrace(globalThis, error, trace);
if (globalOverride !== kNoOverride) return globalOverride;
Expand Down Expand Up @@ -372,6 +356,29 @@ function makeSystemErrorWithCode(key) {
};
}

function makeNodeErrorForHideStackFrame(Base, clazz) {
class HideStackFramesError extends Base {
constructor(...args) {
if (isErrorStackTraceLimitWritable()) {
const limit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
super(...args);
Error.stackTraceLimit = limit;
} else {
super(...args);
}
}

// This is a workaround for wpt tests that expect that the error
// constructor has a `name` property of the base class.
get ['constructor']() {
return clazz;
}
}

return HideStackFramesError;
}

function makeNodeErrorWithCode(Base, key) {
const msg = messages.get(key);
const expectedLength = typeof msg !== 'string' ? -1 : getExpectedArgumentLength(msg);
Expand Down Expand Up @@ -479,11 +486,14 @@ function makeNodeErrorWithCode(Base, key) {
* @returns {T}
*/
function hideStackFrames(fn) {
// We rename the functions that will be hidden to cut off the stacktrace
// at the outermost one
const hidden = nodeInternalPrefix + fn.name;
ObjectDefineProperty(fn, 'name', { __proto__: null, value: hidden });
return fn;
return function wrappedFn() {
try {
return ReflectApply(fn, fn, arguments);
} catch (error) {
Error.stackTraceLimit && ErrorCaptureStackTrace(error, wrappedFn);
throw error;
}
};
}

// Utility function for registering the error codes. Only used here. Exported
Expand All @@ -492,18 +502,20 @@ function E(sym, val, def, ...otherClasses) {
// Special case for SystemError that formats the error message differently
// The SystemErrors only have SystemError as their base classes.
messages.set(sym, val);
if (def === SystemError) {
def = makeSystemErrorWithCode(sym);
} else {
def = makeNodeErrorWithCode(def, sym);
}

const ErrClass = def === SystemError ?
makeSystemErrorWithCode(sym) :
makeNodeErrorWithCode(def, sym);

if (otherClasses.length !== 0) {
otherClasses.forEach((clazz) => {
def[clazz.name] = makeNodeErrorWithCode(clazz, sym);
ErrClass[clazz.name] = makeNodeErrorWithCode(clazz, sym);
});
}
codes[sym] = def;

ErrClass.HideStackFramesError = makeNodeErrorForHideStackFrame(ErrClass, def);

codes[sym] = ErrClass;
}

function getExpectedArgumentLength(msg) {
Expand Down
Loading

0 comments on commit a12adde

Please sign in to comment.