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

process: make getActive{Handles,Requests} official #21102

Closed
wants to merge 1 commit into from
Closed
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
9 changes: 9 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,15 @@ accepted by the legacy `url.parse()` API. The mentioned APIs now use the WHATWG
URL parser that requires strictly valid URLs. Passing an invalid URL is
deprecated and support will be removed in the future.

<a id="DEP0110"></a>
### DEP0110: _getActiveHandles and _getActiveRequests

Type: Runtime

As `process.getActiveHandles()` and `process.getActiveRequests()` are
officially supported and documented, their previous names are now deprecated,
and will be removed in the future.

[`--pending-deprecation`]: cli.html#cli_pending_deprecation
[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
[`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
Expand Down
29 changes: 29 additions & 0 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,35 @@ a code.
Specifying a code to [`process.exit(code)`][`process.exit()`] will override any
previous setting of `process.exitCode`.

## process.getActiveHandles()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to take the opportunity now to move these to the async_hooks module or util module and pull them off the process object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I'd prefer not to expand the API surface area of process if we can reasonably avoid it.

Copy link
Contributor

@mscdex mscdex Jun 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already there though and people use it. I don't see how it makes more sense to place it on the async_hooks or util modules when the data returned by these functions have to strictly do with what's going on in the process.

async_hooks has to do with hooking into asynchronous events, which is not the same thing as inspecting what is currently happening at any given moment (without hooks).

util is essentially a box full of helper functions that work with whatever input they're given, so that doesn't really fit either.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could discuss that in a future PR? I mean, I'm not qualified to answer about this, but I would agree that it's easier to keep the functions where they are.
On the other hand, they were not documented, and could be moved without any further notice

<!-- YAML
added: REPLACEME
-->

* Returns: {Array}

Returns an array containing all handles that are preventing the Node.js process
from exiting. Be aware that the returned handles may not always correspond to
the user-created handles, but instead to an internal one.

```js
console.log(`Active handles: ${util.inspect(process.getActiveHandles())}`);
```

## process.getActiveRequests()
<!-- YAML
added: REPLACEME
-->

* Returns: {Array}

Returns an array containing all requests that are preventing the Node.js process
from exiting.

```js
console.log(`Active requests: ${util.inspect(process.getActiveRequests())}`);
```

## process.getegid()
<!-- YAML
added: v2.0.0
Expand Down
14 changes: 14 additions & 0 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,20 @@
function setupProcessObject() {
_setupProcessObject(pushValueToArray);

const util = NativeModule.require('util');
process._getActiveHandles = util.deprecate(
process.getActiveHandles,
"'process._getActiveHandles' is deprecated, " +
"use 'process.getActiveHandles' instead",
'DEP0110'
);
process._getActiveRequests = util.deprecate(
process.getActiveRequests,
"'process._getActiveRequests' is deprecated, " +
"use 'process.getActiveRequests' instead",
'DEP0110'
);

function pushValueToArray() {
for (var i = 0; i < arguments.length; i++)
this.push(arguments[i]);
Expand Down
4 changes: 2 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2248,8 +2248,8 @@ void SetupProcessObject(Environment* env,
env->SetMethod(process, "chdir", Chdir);
env->SetMethod(process, "umask", Umask);
}
env->SetMethod(process, "_getActiveRequests", GetActiveRequests);
env->SetMethod(process, "_getActiveHandles", GetActiveHandles);
env->SetMethod(process, "getActiveRequests", GetActiveRequests);
env->SetMethod(process, "getActiveHandles", GetActiveHandles);
env->SetMethod(process, "_kill", Kill);

env->SetMethod(process, "cwd", Cwd);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-process-getactivehandles.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function clientConnected(client) {


function checkAll() {
const handles = process._getActiveHandles();
const handles = process.getActiveHandles();

clients.forEach(function(item) {
assert.ok(handles.includes(item));
Expand Down
2 changes: 1 addition & 1 deletion test/pseudo-tty/ref_keeps_node_running.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ handle.readStart();
handle.onread = () => {};

function isHandleActive(handle) {
return process._getActiveHandles().some((active) => active === handle);
return process.getActiveHandles().some((active) => active === handle);
}

strictEqual(isHandleActive(handle), true, 'TTY handle not initially active');
Expand Down
4 changes: 2 additions & 2 deletions test/pummel/test-net-connect-econnrefused.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ function pummel() {

function check() {
setTimeout(function() {
assert.strictEqual(process._getActiveRequests().length, 0);
assert.strictEqual(process._getActiveHandles().length, 1); // the timer
assert.strictEqual(process.getActiveRequests().length, 0);
assert.strictEqual(process.getActiveHandles().length, 1); // the timer
check_called = true;
}, 0);
}
Expand Down