Skip to content

async_hooks: expose list of types #13610

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

Closed
wants to merge 7 commits 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
17 changes: 17 additions & 0 deletions doc/api/async_hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,16 @@ provided by AsyncHooks itself. The logging should then be skipped when
it was the logging itself that caused AsyncHooks callback to call. By
doing this the otherwise infinite recursion is broken.

#### `async_hooks.getTypes()`
Copy link
Member

Choose a reason for hiding this comment

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

Alphabetical order.


Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary whitespace before the yaml comment

<!-- YAML
added: REPLACEME
-->

* Returns: `{Array}` list of async resource type names
Copy link
Member

Choose a reason for hiding this comment

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

The "list of async resource type names" is redundant considering it is documented two lines below.

Copy link
Member

Choose a reason for hiding this comment

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

The "list of async resource type names" is redundant considering it is documented two lines below.

Copy link
Member

Choose a reason for hiding this comment

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

Some note about why these are useful would be helpful here.


Return the list of async resource type names. Names are simple strings.

#### `asyncHook.enable()`

* Returns {AsyncHook} A reference to `asyncHook`.
Expand Down Expand Up @@ -428,6 +438,13 @@ const server = net.createServer(function onConnection(conn) {
});
```

#### `async_hooks.registerTypeName(type)`
Copy link
Member

Choose a reason for hiding this comment

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

added YAML annotation needed.


* `type` {string} a new type of async resource
* Returns {string} the type name to use
Copy link
Member

@TimothyGu TimothyGu Jun 13, 2017

Choose a reason for hiding this comment

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

Why not just return undefined? The way it is documented can be interpreted to imply some processing has been done on type, while it is actually returned verbatim.

Copy link
Contributor

Choose a reason for hiding this comment

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

I find it convenient to do the same as is also already in this PR:

const kTickObjectType = async_hooks.registerTypeName('TickObject');


Registers the type name for a new async resource.

#### `async_hooks.triggerId()`

* Returns {number} the ID of the resource responsible for calling the callback
Expand Down
15 changes: 15 additions & 0 deletions lib/async_hooks.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

const errors = require('internal/errors');
const async_wrap = process.binding('async_wrap');
/* Both these arrays are used to communicate between JS and C++ with as little
* overhead as possible.
Expand Down Expand Up @@ -461,6 +462,18 @@ function init(asyncId, type, triggerId, resource) {
processing_hook = false;
}

const JSProviders = new Set();
function registerTypeName(type) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see a type check here, ensuring type is a string.

if (JSProviders.has(type))
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should make sure type isn't already contained in async_wrap.Providers either?

throw new errors.TypeError('ERR_ASYNC_PROVIDER_NAME', type);
JSProviders.add(type);
return type;
}

function getTypes() {
return Object.keys(async_wrap.Providers)
.concat(...JSProviders.keys());
Copy link
Member

Choose a reason for hiding this comment

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

No need for .keys(). Set's iteration protocol returns its contents by default.

Copy link
Member

Choose a reason for hiding this comment

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

This is certainly not the fastest method, but given that this shouldn't be a hot code path that should be ok

}

// Placing all exports down here because the exported classes won't export
// otherwise.
Expand All @@ -469,6 +482,8 @@ module.exports = {
createHook,
currentId,
triggerId,
getTypes,
registerTypeName,
// Embedder API
AsyncResource,
runInAsyncIdScope,
Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ module.exports = exports = {
// Note: Please try to keep these in alphabetical order
E('ERR_ARG_NOT_ITERABLE', '%s must be iterable');
E('ERR_ASSERTION', (msg) => msg);
E('ERR_ASYNC_PROVIDER_NAME', '"%s" type name already registered');
E('ERR_CONSOLE_WRITABLE_STREAM',
(name) => `Console expects a writable stream instance for ${name}`);
E('ERR_CPU_USAGE', (errMsg) => `Unable to obtain cpu usage ${errMsg}`);
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/process/next_tick.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ function setupNextTick() {
const { kInit, kBefore, kAfter, kDestroy, kAsyncUidCntr, kInitTriggerId } =
async_wrap.constants;
const { async_id_symbol, trigger_id_symbol } = async_wrap;
const kTickObjectType = async_hooks.registerTypeName('TickObject');
var nextTickQueue = [];
var microtasksScheduled = false;

Expand Down Expand Up @@ -223,7 +224,7 @@ function setupNextTick() {
tickObject[trigger_id_symbol] = triggerId || initTriggerId();
if (async_hook_fields[kInit] > 0) {
emitInit(tickObject[async_id_symbol],
'TickObject',
kTickObjectType,
tickObject[trigger_id_symbol],
tickObject);
}
Expand Down
9 changes: 6 additions & 3 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const { kInit, kBefore, kAfter, kDestroy, kAsyncUidCntr } =
// Symbols for storing async id state.
const async_id_symbol = Symbol('asyncId');
const trigger_id_symbol = Symbol('triggerId');
const kTimeoutType = async_hooks.registerTypeName('Timeout');
const kImmediateType = async_hooks.registerTypeName('Immediate');

// Timeout values > TIMEOUT_MAX are set to 1.
const TIMEOUT_MAX = 2147483647; // 2^31-1
Expand Down Expand Up @@ -190,7 +192,8 @@ function insert(item, unrefed) {
item[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr];
item[trigger_id_symbol] = initTriggerId();
if (async_hook_fields[kInit] > 0)
emitInit(item[async_id_symbol], 'Timeout', item[trigger_id_symbol], item);
emitInit(item[async_id_symbol], kTimeoutType, item[trigger_id_symbol],
item);
}

L.append(list, item);
Expand Down Expand Up @@ -589,7 +592,7 @@ function Timeout(after, callback, args) {
this[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr];
this[trigger_id_symbol] = initTriggerId();
if (async_hook_fields[kInit] > 0)
emitInit(this[async_id_symbol], 'Timeout', this[trigger_id_symbol], this);
emitInit(this[async_id_symbol], kTimeoutType, this[trigger_id_symbol], this);
}


Expand Down Expand Up @@ -824,7 +827,7 @@ function Immediate() {
this[async_id_symbol] = ++async_uid_fields[kAsyncUidCntr];
this[trigger_id_symbol] = initTriggerId();
if (async_hook_fields[kInit] > 0)
emitInit(this[async_id_symbol], 'Immediate', this[trigger_id_symbol], this);
emitInit(this[async_id_symbol], kImmediateType, this[trigger_id_symbol], this);
}

function setImmediate(callback, arg1, arg2, arg3) {
Expand Down
34 changes: 34 additions & 0 deletions test/parallel/test-async-hooks-type-registry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const async_hooks = require('async_hooks');

const types1 = async_hooks.getTypes();
types1.forEach((v, k) => assert.strictEqual(
typeof v,
'string',
`${v} from types[${k}] should be a string`)
);

const sillyName = 'gaga';
const newType = async_hooks.registerTypeName(sillyName);
assert.strictEqual(
typeof newType,
'string',
`${newType} should be a string`
);
assert.strictEqual(newType, sillyName);
assert.throws(
() => async_hooks.registerTypeName(sillyName),
common.expectsError(
{
code: 'ERR_ASYNC_PROVIDER_NAME',
type: TypeError,
message: `"${sillyName}" type name already registered`
}
)
);

const types2 = async_hooks.getTypes();
assert.strictEqual(types2.length, types1.length + 1);
assert.strictEqual(types2.includes(newType), true);