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

errors: improve hideStackFrames #49990

Merged
merged 1 commit into from
Nov 11, 2023
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
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));
}
Uzlopak marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -1213,7 +1213,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 @@ -1719,9 +1719,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