Skip to content

Commit

Permalink
util: fix util.getCallSites plurality
Browse files Browse the repository at this point in the history
`util.getCallSite` returns an array of call site objects. Rename the
function to reflect that it returns a given count of frames captured
as an array of call site object.

Renames the first parameter `frames` to be `frameCount` to indicate
that it specifies the count of returned call sites.

PR-URL: nodejs#55626
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
  • Loading branch information
legendecas authored and louwers committed Nov 2, 2024
1 parent aec35a6 commit eb16391
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 61 deletions.
30 changes: 15 additions & 15 deletions benchmark/util/get-callsite.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
'use strict';

const common = require('../common');
const { getCallSite } = require('node:util');
const { getCallSites } = require('node:util');
const assert = require('node:assert');

const bench = common.createBenchmark(main, {
n: [1e6],
method: ['ErrorCallSite', 'ErrorCallSiteSerialized', 'CPP'],
method: ['ErrorCallSites', 'ErrorCallSitesSerialized', 'CPP'],
});

function ErrorGetCallSite() {
function ErrorGetCallSites() {
const originalStackFormatter = Error.prepareStackTrace;
Error.prepareStackTrace = (_err, stack) => {
if (stack && stack.length > 1) {
Expand All @@ -25,15 +25,15 @@ function ErrorGetCallSite() {
return err.stack;
}

function ErrorCallSiteSerialized() {
const callsite = ErrorGetCallSite();
function ErrorCallSitesSerialized() {
const callSites = ErrorGetCallSites();
const serialized = [];
for (let i = 0; i < callsite.length; ++i) {
for (let i = 0; i < callSites.length; ++i) {
serialized.push({
functionName: callsite[i].getFunctionName(),
scriptName: callsite[i].getFileName(),
lineNumber: callsite[i].getLineNumber(),
column: callsite[i].getColumnNumber(),
functionName: callSites[i].getFunctionName(),
scriptName: callSites[i].getFileName(),
lineNumber: callSites[i].getLineNumber(),
column: callSites[i].getColumnNumber(),
});
}
return serialized;
Expand All @@ -42,14 +42,14 @@ function ErrorCallSiteSerialized() {
function main({ n, method }) {
let fn;
switch (method) {
case 'ErrorCallSite':
fn = ErrorGetCallSite;
case 'ErrorCallSites':
fn = ErrorGetCallSites;
break;
case 'ErrorCallSiteSerialized':
fn = ErrorCallSiteSerialized;
case 'ErrorCallSitesSerialized':
fn = ErrorCallSitesSerialized;
break;
case 'CPP':
fn = getCallSite;
fn = getCallSites;
break;
}
let lastStack = {};
Expand Down
14 changes: 14 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -3761,6 +3761,19 @@ Instantiating classes without the `new` qualifier exported by the `node:repl` mo
It is recommended to use the `new` qualifier instead. This applies to all REPL classes, including
`REPLServer` and `Recoverable`.

### DEP0186: `util.getCallSite`

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/55626
description: Runtime deprecation.
-->

Type: Runtime

The `util.getCallSite` API has been removed. Please use [`util.getCallSites()`][] instead.

[NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
[RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
[RFC 8247 Section 2.4]: https://www.rfc-editor.org/rfc/rfc8247#section-2.4
Expand Down Expand Up @@ -3886,6 +3899,7 @@ It is recommended to use the `new` qualifier instead. This applies to all REPL c
[`url.parse()`]: url.md#urlparseurlstring-parsequerystring-slashesdenotehost
[`url.resolve()`]: url.md#urlresolvefrom-to
[`util._extend()`]: util.md#util_extendtarget-source
[`util.getCallSites()`]: util.md#utilgetcallsitesframecount
[`util.getSystemErrorName()`]: util.md#utilgetsystemerrornameerr
[`util.inspect()`]: util.md#utilinspectobject-options
[`util.inspect.custom`]: util.md#utilinspectcustom
Expand Down
14 changes: 7 additions & 7 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -364,31 +364,31 @@ util.formatWithOptions({ colors: true }, 'See object %O', { foo: 42 });
// when printed to a terminal.
```

## `util.getCallSite(frames)`
## `util.getCallSites(frameCount)`

> Stability: 1.1 - Active development
<!-- YAML
added: v22.9.0
-->

* `frames` {number} Number of frames returned in the stacktrace.
* `frameCount` {number} Number of frames to capture as call site objects.
**Default:** `10`. Allowable range is between 1 and 200.
* Returns: {Object\[]} An array of stacktrace objects
* `functionName` {string} Returns the name of the function associated with this stack frame.
* Returns: {Object\[]} An array of call site objects
* `functionName` {string} Returns the name of the function associated with this call site.
* `scriptName` {string} Returns the name of the resource that contains the script for the
function for this StackFrame.
function for this call site.
* `lineNumber` {number} Returns the number, 1-based, of the line for the associate function call.
* `column` {number} Returns the 1-based column offset on the line for the associated function call.

Returns an array of stacktrace objects containing the stack of
Returns an array of call site objects containing the stack of
the caller function.

```js
const util = require('node:util');

function exampleFunction() {
const callSites = util.getCallSite();
const callSites = util.getCallSites();

console.log('Call Sites:');
callSites.forEach((callSite, index) => {
Expand Down
15 changes: 10 additions & 5 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,13 +330,13 @@ function parseEnv(content) {

/**
* Returns the callSite
* @param {number} frames
* @param {number} frameCount
* @returns {object}
*/
function getCallSite(frames = 10) {
function getCallSites(frameCount = 10) {
// Using kDefaultMaxCallStackSizeToCapture as reference
validateNumber(frames, 'frames', 1, 200);
return binding.getCallSite(frames);
validateNumber(frameCount, 'frameCount', 1, 200);
return binding.getCallSites(frameCount);
};

// Keep the `exports =` so that various functions can still be monkeypatched
Expand All @@ -353,7 +353,12 @@ module.exports = {
format,
styleText,
formatWithOptions,
getCallSite,
// Deprecated getCallSite.
// This API can be removed in next semver-minor release.
getCallSite: deprecate(getCallSites,
'The `util.getCallSite` API is deprecated. Please use `util.getCallSites()` instead.',
'DEP0186'),
getCallSites,
getSystemErrorMap,
getSystemErrorName,
getSystemErrorMessage,
Expand Down
6 changes: 3 additions & 3 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ static void ParseEnv(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(dotenv.ToObject(env));
}

static void GetCallSite(const FunctionCallbackInfo<Value>& args) {
static void GetCallSites(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();

Expand Down Expand Up @@ -345,7 +345,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(GetProxyDetails);
registry->Register(GetCallerLocation);
registry->Register(PreviewEntries);
registry->Register(GetCallSite);
registry->Register(GetCallSites);
registry->Register(GetOwnNonIndexProperties);
registry->Register(GetConstructorName);
registry->Register(GetExternalValue);
Expand Down Expand Up @@ -451,7 +451,7 @@ void Initialize(Local<Object> target,
SetMethodNoSideEffect(
context, target, "getConstructorName", GetConstructorName);
SetMethodNoSideEffect(context, target, "getExternalValue", GetExternalValue);
SetMethodNoSideEffect(context, target, "getCallSite", GetCallSite);
SetMethodNoSideEffect(context, target, "getCallSites", GetCallSites);
SetMethod(context, target, "sleep", Sleep);
SetMethod(context, target, "parseEnv", ParseEnv);

Expand Down
4 changes: 2 additions & 2 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const net = require('net');
// Do not require 'os' until needed so that test-os-checked-function can
// monkey patch it. If 'os' is required here, that test will fail.
const path = require('path');
const { inspect, getCallSite } = require('util');
const { inspect, getCallSites } = require('util');
const { isMainThread } = require('worker_threads');
const { isModuleNamespaceObject } = require('util/types');

Expand Down Expand Up @@ -550,7 +550,7 @@ function canCreateSymLink() {
}

function mustNotCall(msg) {
const callSite = getCallSite()[1];
const callSite = getCallSites()[1];
return function mustNotCall(...args) {
const argsInfo = args.length > 0 ?
`\ncalled with arguments: ${args.map((arg) => inspect(arg)).join(', ')}` : '';
Expand Down
4 changes: 0 additions & 4 deletions test/fixtures/get-call-site.js

This file was deleted.

4 changes: 4 additions & 0 deletions test/fixtures/get-call-sites.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const util = require('node:util');
const assert = require('node:assert');
assert.ok(util.getCallSites().length > 1);
process.stdout.write(util.getCallSites()[0].scriptName);
9 changes: 9 additions & 0 deletions test/parallel/test-util-getcallsite.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict';

require('../common');
const { getCallSite } = require('node:util');
const { expectWarning } = require('../common');

const warning = 'The `util.getCallSite` API is deprecated. Please use `util.getCallSites()` instead.';
expectWarning('DeprecationWarning', warning, 'DEP0186');
getCallSite();
Original file line number Diff line number Diff line change
Expand Up @@ -3,68 +3,68 @@
const common = require('../common');

const fixtures = require('../common/fixtures');
const file = fixtures.path('get-call-site.js');
const file = fixtures.path('get-call-sites.js');

const { getCallSite } = require('node:util');
const { getCallSites } = require('node:util');
const { spawnSync } = require('node:child_process');
const assert = require('node:assert');

{
const callsite = getCallSite();
assert.ok(callsite.length > 1);
const callSites = getCallSites();
assert.ok(callSites.length > 1);
assert.match(
callsite[0].scriptName,
/test-util-getCallSite/,
callSites[0].scriptName,
/test-util-getcallsites/,
'node:util should be ignored',
);
}

{
const callsite = getCallSite(3);
assert.strictEqual(callsite.length, 3);
const callSites = getCallSites(3);
assert.strictEqual(callSites.length, 3);
assert.match(
callsite[0].scriptName,
/test-util-getCallSite/,
callSites[0].scriptName,
/test-util-getcallsites/,
'node:util should be ignored',
);
}

// Guarantee dot-left numbers are ignored
{
const callsite = getCallSite(3.6);
assert.strictEqual(callsite.length, 3);
const callSites = getCallSites(3.6);
assert.strictEqual(callSites.length, 3);
}

{
const callsite = getCallSite(3.4);
assert.strictEqual(callsite.length, 3);
const callSites = getCallSites(3.4);
assert.strictEqual(callSites.length, 3);
}

{
assert.throws(() => {
// Max than kDefaultMaxCallStackSizeToCapture
getCallSite(201);
getCallSites(201);
}, common.expectsError({
code: 'ERR_OUT_OF_RANGE'
}));
assert.throws(() => {
getCallSite(-1);
getCallSites(-1);
}, common.expectsError({
code: 'ERR_OUT_OF_RANGE'
}));
assert.throws(() => {
getCallSite({});
getCallSites({});
}, common.expectsError({
code: 'ERR_INVALID_ARG_TYPE'
}));
}

{
const callsite = getCallSite(1);
assert.strictEqual(callsite.length, 1);
const callSites = getCallSites(1);
assert.strictEqual(callSites.length, 1);
assert.match(
callsite[0].scriptName,
/test-util-getCallSite/,
callSites[0].scriptName,
/test-util-getcallsites/,
'node:util should be ignored',
);
}
Expand All @@ -77,8 +77,8 @@ const assert = require('node:assert');
'-e',
`const util = require('util');
const assert = require('assert');
assert.ok(util.getCallSite().length > 1);
process.stdout.write(util.getCallSite()[0].scriptName);
assert.ok(util.getCallSites().length > 1);
process.stdout.write(util.getCallSites()[0].scriptName);
`,
],
);
Expand All @@ -100,7 +100,7 @@ const assert = require('node:assert');
{
const originalStackTraceLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
const callsite = getCallSite();
assert.notStrictEqual(callsite.length, 0);
const callSites = getCallSites();
assert.notStrictEqual(callSites.length, 0);
Error.stackTraceLimit = originalStackTraceLimit;
}

0 comments on commit eb16391

Please sign in to comment.