Skip to content

Commit

Permalink
errors: improve hideStackFrames
Browse files Browse the repository at this point in the history
PR-URL: #49990
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Uzlopak authored and richardlau committed Mar 25, 2024
1 parent ade6614 commit db7459b
Show file tree
Hide file tree
Showing 41 changed files with 978 additions and 564 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');
}
}
87 changes: 87 additions & 0 deletions benchmark/error/hidestackframes-throw.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
'use strict';

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

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

function main({ n, type, double }) {
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);
}
});

function doubleTestfn(value) {
testfn(value);
}

const doubleHideStackFramesTestfn = hideStackFrames((value) => {
hideStackFramesTestfn.withoutStackTrace(value);
});

const fn = type === 'hide-stackframe' ?
double === 'true' ?
doubleHideStackFramesTestfn :
hideStackFramesTestfn :
double === 'true' ?
doubleTestfn :
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.

2 changes: 1 addition & 1 deletion lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ rules:
- selector: ThrowStatement > CallExpression[callee.name=/Error$/]
message: Use new keyword when throwing an Error.
# Config specific to lib
- selector: NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError)$/])
- selector: NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError|NodeAggregateError)$/])
message: Use an error exported by the internal/errors module.
- selector: CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']
message: Please use `require('internal/errors').hideStackFrames()` instead.
Expand Down
10 changes: 5 additions & 5 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const {
traceEnd,
getNextTraceEventId,
} = require('internal/http');
const { connResetException, codes } = require('internal/errors');
const { ConnResetException, codes } = require('internal/errors');
const {
ERR_HTTP_HEADERS_SENT,
ERR_INVALID_ARG_TYPE,
Expand Down Expand Up @@ -452,7 +452,7 @@ function socketCloseListener() {
if (res) {
// Socket closed before we emitted 'end' below.
if (!res.complete) {
res.destroy(connResetException('aborted'));
res.destroy(new ConnResetException('aborted'));
}
req._closed = true;
req.emit('close');
Expand All @@ -465,7 +465,7 @@ function socketCloseListener() {
// receive a response. The error needs to
// fire on the request.
req.socket._hadError = true;
req.emit('error', connResetException('socket hang up'));
req.emit('error', new ConnResetException('socket hang up'));
}
req._closed = true;
req.emit('close');
Expand Down Expand Up @@ -516,7 +516,7 @@ function socketOnEnd() {
// If we don't have a response then we know that the socket
// ended prematurely and we need to emit an error on the request.
req.socket._hadError = true;
req.emit('error', connResetException('socket hang up'));
req.emit('error', new ConnResetException('socket hang up'));
}
if (parser) {
parser.finish();
Expand Down Expand Up @@ -869,7 +869,7 @@ function onSocketNT(req, socket, err) {

function _destroy(req, err) {
if (!req.aborted && !err) {
err = connResetException('socket hang up');
err = new ConnResetException('socket hang up');
}
if (err) {
req.emit('error', err);
Expand Down
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
4 changes: 2 additions & 2 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const {
} = require('internal/async_hooks');
const { IncomingMessage } = require('_http_incoming');
const {
connResetException,
ConnResetException,
codes,
} = require('internal/errors');
const {
Expand Down Expand Up @@ -790,7 +790,7 @@ function socketOnClose(socket, state) {
function abortIncoming(incoming) {
while (incoming.length) {
const req = incoming.shift();
req.destroy(connResetException('aborted'));
req.destroy(new ConnResetException('aborted'));
}
// Abort socket._httpMessage ?
}
Expand Down
10 changes: 5 additions & 5 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const { Pipe, constants: PipeConstants } = internalBinding('pipe_wrap');
const { owner_symbol } = require('internal/async_hooks').symbols;
const { isArrayBufferView } = require('internal/util/types');
const { SecureContext: NativeSecureContext } = internalBinding('crypto');
const { connResetException, codes } = require('internal/errors');
const { ConnResetException, codes } = require('internal/errors');
const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
Expand Down Expand Up @@ -1218,7 +1218,7 @@ function onSocketClose(err) {
// Emit ECONNRESET
if (!this._controlReleased && !this[kErrorEmitted]) {
this[kErrorEmitted] = true;
const connReset = connResetException('socket hang up');
const connReset = new ConnResetException('socket hang up');
this._tlsOptions.server.emit('tlsClientError', connReset, this);
}
}
Expand Down Expand Up @@ -1724,9 +1724,9 @@ function onConnectEnd() {
if (!this._hadError) {
const options = this[kConnectOptions];
this._hadError = true;
const error = connResetException('Client network socket disconnected ' +
'before secure TLS connection was ' +
'established');
const error = new ConnResetException('Client network socket disconnected ' +
'before secure TLS connection was ' +
'established');
error.path = options.path;
error.host = options.host;
error.port = options.port;
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
Loading

0 comments on commit db7459b

Please sign in to comment.