docs: documenting async hooks features #12953

thlorenz · 2017-05-10T21:17:36Z

Continuation of ongoing PR to add documentation for the async hooks feature which was merged recently.

The existing commits in the previous PR have been cherry-picked onto latest master.
I suggest to work out all nits and finally squash this into one proper commit.

Checklist

documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

async hooks
docs

jasnell · 2017-05-11T00:30:45Z

doc/api/async_hooks.md

+
+### Overview
+
+Here is a simple overview of the public API. All of this API is explained in


s/Here/Following

jasnell · 2017-05-11T00:31:28Z

doc/api/async_hooks.md

+```
+
+#### `async_hooks.createHook(callbacks)`
+


args listing? e.g.

* `callbacks` ...

Also, yaml block?

Including args listing for consistency. The entire block below elaborates on what the callbacks are.
Not sure what you mean by yaml block could you give an example?

Not sure what you mean by yaml block could you give an example?



you can add those directly below headings :) (oh, and edit: the REPLACEME is literal, it will be replaced during releasing)

OK added, thanks

jasnell · 2017-05-11T00:32:07Z

doc/api/async_hooks.md

+emulate the lifetime of handles and requests that do not fit this model. For
+example, `HTTPParser` instances are recycled to improve performance. Therefore the
+`destroy()` callback is called manually after a connection is done using
+it, just before it's placed back into the unused resource pool.


s/it's/it is

jasnell · 2017-05-11T00:32:52Z

doc/api/async_hooks.md

+specifics of all functions that can be passed to `callbacks` is in the section
+`Hook Callbacks`.
+
+**Error Handling**: If any `AsyncHook` callbacks throw, the application will


This would be better as a heading... e.g.

##### Error Handling

jasnell · 2017-05-11T00:33:48Z

doc/api/async_hooks.md

+Some examples include `TCP`, `GetAddrInfo` and `HTTPParser`. Users will be able
+to define their own `type` when using the public embedder API.
+
+**Note:** It is possible to have type name collisions. Embedders are recommended


s/**Note:**/*Note*:

jasnell · 2017-05-11T00:37:25Z

doc/api/async_hooks.md

+
+**Note:** Some resources depend on GC for cleanup. So if a reference is made to
+the `resource` object passed to `init()` it's possible that `destroy()` is
+never called. Causing a memory leak in the application. Of course if you know


Please avoid the use of you

jasnell · 2017-05-11T00:37:38Z

doc/api/async_hooks.md

+placed in an unused resource pool to be used later. For these `destroy()` will
+be called just before the resource is placed on the unused resource pool.
+
+**Note:** Some resources depend on GC for cleanup. So if a reference is made to


s/**Note:**/*Note*:

jasnell · 2017-05-11T00:38:40Z

doc/api/async_hooks.md

@@ -0,0 +1,670 @@
+# Async Hooks
+
+


No need for the extra blank lines in here

jasnell · 2017-05-11T00:39:10Z

doc/api/async_hooks.md

+
+The `async-hooks` module provides an API to register callbacks tracking the
+lifetime of asynchronous resources created inside a Node.js application.
+


Please include something like:

It can be accessed using `require('async-hooks')`

jasnell · 2017-05-11T00:39:41Z

doc/api/async_hooks.md

+
+## Terminology
+
+An async resource represents either a "handle" or a "request".


rather than the quotes, make these italic. e.g. handle or a request

AndreasMadsen · 2017-05-11T06:28:20Z

repeating my comment from #12892 (comment):

@thlorenz ... after reviewing the code I would like to cut out the low-level JS Embedder API (high-level JS Embedder API stays). And change the high-level Embedder API to be domain.run() like instead of domain.enter() + domain.exit(). My reasons are that I think we would otherwise repeat some domain mistakes. I think the changes are very minor and it won't prevent us from adding the EPS API later if necessary.

What effect does that have for async_hooks right now?

The Standalone JS API section is removed. It is only needed for performance reasons. We need the API ourself, but the high-level JS Embedder API does the same and should be enough for userland.
asyncEvent.emitBefore() and asyncEvent.emitAfter() is combined to asyncEvent.makeCallback(). The following code change is needed to make that happen:

--- a/lib/async_hooks.js
+++ b/lib/async_hooks.js
@@ -222,6 +222,13 @@ class AsyncEvent {
     processing_hook = false;
   }

+  makeCallback(cb, ...args) {
+    emitBeforeS(this[async_id_symbol], this[trigger_id_symbol]);
+    cb(...args);
+    emitAfterS(this[async_id_symbol]);
+    return this;
+  }
+

In response to @Fishrock123 (#12892 (comment)) ...

To be clear: It was designed to be low-level to avoid domains-like problems so I am awfully confused.

What made the domain API to high-level was not the Embedder API but the other API (~~I don't know what to call it~~ try/catch). In async_hooks we are replacing the other API with the Hooks API. If only including the high-level Embedder API turns out to be a problem, it is a semver-minor change to introduce the remaining Embedder API.

In #11883 (comment) I explained the problems with the domain API:

Can you elaborate on the domains problem you allude to?

For native modules and in other cases like queues, one have to wrap the code with the domain embedder API. There are two method of doing this: (Actually more domain.bind and domain.intercept but they are similar to domain.run):
nativeAsync(function () {
  domain.run(cb);
});
nativeAsync(function (...args) {
  domain.enter();
  cb(...args);
  domain.exit();
});
The fist method is fine, this is what I want for async_hooks. The latter method caused some issues (if I remember correctly) because it was abused or used incorrectly. An example of abuse is to call domain.exit(); before domain.enter() in order to hack the domain state. An example of incorrect use, is to forget to call domain.exit() or domain.enter() because the third-party logic is too complex. The above script is short and thus easy to do correctly, but if there are multiple exit or entrance points for nativeAsync one could forget either domain.exit() or domain.enter(). Yes it can sound very silly, but it is an easy mistake to make for those who don't know better and the consequences of doing it wrong is that the context is lost which can be quite severe.

BridgeAR · 2017-05-11T07:35:12Z

doc/api/async_hooks.md

+
+// Return the id of the handle responsible for triggering the callback of the
+// current execution scope to fire.
+const tid = aysnc_hooks.triggerId();


s/aysnc/async

BridgeAR · 2017-05-11T07:36:11Z

doc/api/async_hooks.md

+```js
+const async_hooks = require('async_hooks');
+
+asyns_hooks.createHook({


s/asyns/async

BridgeAR · 2017-05-11T07:37:06Z

doc/api/async_hooks.md

+TTYWRAP(6) -> Timeout(4) -> TIMERWRAP(5) -> TickObject(3) -> root(1)
+```
+
+The `TCPWRAP` isn't part of this graph; evne though it was the reason for


s/evne/even

BridgeAR · 2017-05-11T07:44:03Z

doc/api/async_hooks.md

+
+### `class AsyncEvent()`
+
+The class `AsyncEvent` was designed to be extended from for embedder's async


Maybe replace from for by by the?

BridgeAR · 2017-05-11T07:44:38Z

doc/api/async_hooks.md

+#### `AsyncEvent(type[, triggerId])`
+
+* arguments
+  * `type` {String} the type of ascycn event


s/ascycn/async

BridgeAR · 2017-05-11T07:45:03Z

doc/api/async_hooks.md

+
+```js
+class DBQuery extends AsyncEvent {
+  construtor(db) {


s/construtor/constructor

BridgeAR · 2017-05-11T07:46:35Z

doc/api/async_hooks.md

+* `id` {number} Generated by `newId()`
+* Returns {Undefined}
+
+Notify hooks that a resource is being destroyed (or being moved to the free'd


s/free'd/freed

BridgeAR · 2017-05-11T07:57:25Z

doc/api/async_hooks.md

+```
+
+It is important to note that the id returned fom `currentId()` is related to
+execution timing.  Not causality (which is covered by `triggerId()`). For


Nit: s/ /

BridgeAR · 2017-05-11T07:57:28Z

doc/api/async_hooks.md

+example:
+
+```js
+const server = net.createServer(function onconnection(conn) {


Nit: s/onconnection/onConnection

BridgeAR · 2017-05-11T07:57:31Z

doc/api/async_hooks.md

+  // MakeCallback().
+  async_hooks.currentId();
+
+}).listen(port, function onlistening() {


Nit: s/onlistening/onListening

BridgeAR · 2017-05-11T08:07:47Z

doc/api/async_hooks.md

+
+* `triggerId` {number}
+* `callback` {Function}
+* Returns {Undefined}


All types should be lower cased. I saw Undefined, String, Object and Function

All types? Getting mixed info here. I was told elsewhere only primitive types.
For now I lowercased Undefined and String as requested above.

Going to wait for clarification here until I lower case more.

mcollina

A nit. This also needs a summary at the top, as it's a long document.

mcollina · 2017-05-11T12:45:53Z

doc/api/async_hooks.md

+                      `${type}(${id}): trigger: ${triggerId} scope: ${cId}`);
+  },
+  before (id) {
+    process._rawDebug(' '.repeat(ws) + 'before: ', id);


it's not explained why we need to use _rawDebug here.

It's also not explained how to get the data out of the app process in general. If I want to send it across network or push to a database, all I can do is disable hooks and enable them back, but that has a potential of dropping a lot of information in meantime, so _rawDebug seems the only option to get any information out of async hooks.

Could this be addressed head-on in the docs?
If not, we should document practical use cases for this. All synchronous APIs can be used, but there's not a lot of them left.

There is the option of not allowing any async hooks to execute while one of its own hooks is running. That should be the most straight forward approach, but we'd also loose information about what's happening during hook execution. If that's a fine trade then I can whip up the patch.

FYI: Reason process._rawDebug() needs to be used is because it writes directly to stderr. A similar effect would be to do fs.writeSync(2, 'foo');. Otherwise init will be called and cause a circular call to the hooks. Causing the application to crash.

We all know why rawDebug is required, but the doc should mention that.

I'm not sure if disabling hooks while one is running is a good idea because you probably can't guarantee other parts of the app won't get executed between async calls of hook callback.

But without that we need to educate everyone they can only use sync writes (and make their hooks slow down other stuff).

Would it be possible to expose an API to push items to a queue that would then be offloaded somewhere on a background thread? I expect people will want to send hook results out live as means of debugging their code.

Pushing custom trave events from hooks might also be a thing.

Oh, unless you mean not running hooks if a hook callback is one of the parents in callstack, which seems like a good solution to all the issues with sending out data.

Hooks code can be diagnosed with profiling and flamegraphs still, so that's not much of an issue.

@trevnorris would that just be in the current tick or for the full context?

async_hooks.createHooks({ init() { setImmediate(() => setImmediate(() => /* does this fire hooks */)); } })

What @AndreasMadsen asked ^^

It looks ok, but we'd need to be clear about what happens in the setImmediate() case.

I think an fs.writeSync(1, ...) can be used in place of _rawDebug(). People will want to play around with hooks and print things while gainining an understanding, so since console.log() messes this up, perhaps an intro section is needed on this topic, describing the problem and a solution. It could show a safe "print" function using fs.writeSync as an example, and then that function could be used consistently throughout the other examples in the docs

Sorry for the delay.
@AndreasMadsen @jasnell

would that just be in the current tick or for the full context?

That puts a swift end to my suggestion. I lapsed back to the previous implementation where all resources were tracked individually so each resource could omit firing its hooks.

I might suggest we add an API like async_hooks.log(). Telling the user to fs.writeSync(1) seems like something that enough people will need to do that we might as well codify it in the API.

That puts a swift end to my suggestion. I lapsed back to the previous implementation where all resources were tracked individually so each resource could omit firing its hooks.

I had my suspicions :D

Something I have been thinking about is if two separate AsyncHook instances both call an async operation. Sure they can know about their own and prevent an internal recursion, but I don't think they can prevent a recursion jumping between each other.

addaleax · 2017-05-11T12:54:48Z

doc/api/async_hooks.md

+##### `init(id, type, triggerId, resource)`
+
+* `id` {number} a unique id for the async resource
+* `type` {String} the type of the async resource


String, Undefined should also be lowercase ;)

thlorenz · 2017-05-17T15:18:43Z

Thanks for all the feedback .. tied up mostly this week, but will get to it next week and try to fix all problems that were pointed out.

trevnorris · 2017-05-17T18:34:31Z

@AndreasMadsen

An example of abuse is to call domain.exit(); before domain.enter() in order to hack the domain state.

The state is checked when unwinding, and the application will crash if it isn't called in the correct order. It will also crash if the user forgets to run emitAfter() as the stack is unwinding. So abusing the API, or even being negligent, should be impossible.

change the high-level Embedder API to be domain.run() like instead of domain.enter() + domain.exit().

The Standalone JS API section is removed. It is only needed for performance reasons. We need the API ourself, but the high-level JS Embedder API does the same and should be enough for userland.

If only including the high-level Embedder API turns out to be a problem, it is a semver-minor change to introduce the remaining Embedder API.

To clarify, do you mean emitBefore() and emitAfter() in AsyncEvent, the "Sensitive Embedder API", or both?

In regards to AsyncEvent I don't agree that only having a `makeCallback() type command is enough for users, as you said it is also easier to add than remove features. Though this API is experimental at the moment and I think it'd be good for users to be able to try out both. If for nothing else to work out kinks with the existing API.

As for the "Sensitive Embedder API", I'm not too attached at keeping it around but it will make it much easier for users to incorporate into existing code; rather than needing to inherit from AsyncEvent. As I said above, it should be impossible for users to hack around this API. There are a plethora of checks in state management to make sure nothing of this sort happens. Which is why emitAfter() requires the asyncId to be passed. Adding a makeCallback() type function for this would be helpful as well.

In short, if the "Sensitive Embedder API" is left intact then I would be alright removing emitBefore()/emitAfter() from AsyncEvent and replacing it with a sort of makeCallback() (though I don't love the name; possibly it should just be run() or something similar). I am alright adding a makeCallback() call to the embedder API.

AndreasMadsen · 2017-05-17T20:16:49Z

The state is checked when unwinding, and the application will crash if it isn't called in the correct order. It will also crash if the user forgets to run emitAfter() as the stack is unwinding. So abusing the API, or even being negligent, should be impossible.

This makes sense. I am a little worried about if we got this right, but assuming so, replacing emitBefore and emitAfter with makeCallback() is not super important to me. But I would still like to know what the use case is.

Though this API is experimental at the moment and I think it'd be good for users to be able to try out both. If for nothing else to work out kinks with the existing API.

I think there are plenty of cases where an experimental API couldn't be changed later. But more importantly, I think we will get much better feedback if we are lacking a feature, than if we have too many features. Nobody complains about too many features.

For example, let's say that nobody uses the "Standalone JS API" (I think this is quite likely), then we will get no feedback and thus assume it is perfect. When we mark async_hooks as stable and it later turns out that it did have issues, then we are in trouble.

I would much rather have a clear indication that every part of the async_hooks API is used.

To clarify, do you mean emitBefore() and emitAfter() in AsyncEvent, the "Sensitive Embedder API", or both?

I don't know what "Sensitive Embedder API" is, can we use the terminology from the EPS? There are two APIs:

high-level called AsyncEvent
low-level called "Standalone JS API".

I want to keep the "Standalone JS API" private for now. I don't think the "Standalone JS API" makes things easier, using:

resource = new AsyncEvent();
resource.emitBefore();

is at least as easy as:

id = async_hooks.newId();
async_hooks.emitInit(id);
async_hooks.emitBefore(id);

thlorenz · 2017-05-18T16:48:56Z

Addressed all obvious nits, asked for clarification on others.
For the more complex issues I'm waiting for the discussions to come to a place where we clearly know what exact changes we need to make to the docs.

If anyone has a fix for either of those (i.e. @trevnorris) please just provide me with a commit and I'll cherry-pick it onto here. Alternatively anyone who wants to help LMK and I'll add them as a collab to the repo with this branch.

addaleax

Mostly looks good to me, with a few grammar nits and suggestions.

addaleax · 2017-05-21T15:38:04Z

doc/api/async_hooks.md

+
+The `async-hooks` module provides an API to register callbacks tracking the
+lifetime of asynchronous resources created inside a Node.js application.
+It can be accessed using `require('async-hooks')`.


These should be underscores (i.e. `async_hooks`)

Also, please move the require statement to it's own line for consistency with other docs. e.g.

It can be accessed using: ```js const async_hooks = require('async_hooks');

addaleax · 2017-05-21T15:40:54Z

doc/api/async_hooks.md

+Handles are a reference to a system resource. Some resources are a simple
+identifier. For example, file system handles are represented by a file
+descriptor. Other handles are represented by libuv as a platform abstracted
+struct, e.g. `uv_tcp_t`. Each handle can be continually reused to access and


Is anything described in the Some resources … uv_tcp_t bit actually visible to users? I don’t think so, so I’d just drop it.

addaleax · 2017-05-21T15:41:16Z

doc/api/async_hooks.md

+operate on the referenced resource.
+
+Requests are short lived data structures created to accomplish one task. The
+callback for a request should always and only ever fire one time. Which is when


one time. Which is → one time, which is

addaleax · 2017-05-21T15:42:50Z

doc/api/async_hooks.md

+Registers functions to be called for different lifetime events of each async
+operation.
+The callbacks `init()`/`before()`/`after()`/`destroy()` are registered via an
+`AsyncHooks` instance and fire during the respective asynchronous events in the


AsyncHooks → AsyncHook

fire -> "are called"?

addaleax · 2017-05-21T15:43:35Z

doc/api/async_hooks.md

+-->
+
+* `callbacks` {Object} the callbacks to register
+


Can you add * Returns: {AsyncHook}?

addaleax · 2017-05-21T15:56:42Z

doc/api/async_hooks.md

+
+*Note:* Some resources depend on GC for cleanup. So if a reference is made to
+the `resource` object passed to `init()` it's possible that `destroy()` is
+never called. Causing a memory leak in the application. Of course if 


never called. Causing a memory leak → never called, causing a memory leak

addaleax · 2017-05-21T15:58:13Z

doc/api/async_hooks.md

+```
+
+It is important to note that the id returned fom `currentId()` is related to
+execution timing. Not causality (which is covered by `triggerId()`). For


execution timing. Not causality → execution timing, not causality

addaleax · 2017-05-21T15:58:31Z

doc/api/async_hooks.md

+
+```js
+const server = net.createServer(function onConnection(conn) {
+  // Returns the id of the server, not of the new connection. Because the


the new connection. Because the → the new connection, because the

addaleax · 2017-05-21T15:59:05Z

doc/api/async_hooks.md

+```js
+const server = net.createServer(function onConnection(conn) {
+  // Returns the id of the server, not of the new connection. Because the
+  // on connection callback runs in the execution scope of the server's


I’d drop on, or spell it as onConnection to match the code

addaleax · 2017-05-21T16:01:10Z

doc/api/async_hooks.md

+If the user's callback throws an exception then `emitAfter()` will
+automatically be called for all `id`'s on the stack if the error is handled by
+a domain or `'uncaughtException'` handler. So there is no need to guard against
+this.


I’d drop this last sentence fragment, but if you keep it, please join it with a comma

If this addresses my question about lack of try/catch above, it probably needs a bit more text, not less, to describe what will happen.

jasnell · 2017-05-22T15:05:01Z

doc/api/async_hooks.md

+
+## Terminology
+
+An async resource represents either a _handle_ or a _request_.


s/_handle_/*handle*
s/_request_/*request*

jasnell · 2017-05-22T15:06:04Z

doc/api/async_hooks.md

+the assigned task has either completed or encountered an error. Requests are
+used by handles to perform tasks such as accepting a new connection or
+writing data to disk.
+


Something should be described around here about how the async_hooks API specifically relates to handles and requests

It doesn't relate, in fact, it abstracts the two cases into the resource concept.

jasnell · 2017-05-22T15:06:21Z

doc/api/async_hooks.md

+### Overview
+
+Following is a simple overview of the public API. All of this API is explained in
+more detail further down.


Just strike the second sentence here.

jasnell · 2017-05-22T15:14:00Z

doc/api/async_hooks.md

+destroy: 5
+```
+
+First notice that `scope` and the value returned by `currentId()` are always


*Note*: As illustrated in the example, `currentId()` and `scope` each specify the value of the current execution context; which is delineated by calls to `before()` and `after()`.

jasnell · 2017-05-22T15:14:23Z

doc/api/async_hooks.md

+the same. That's because `currentId()` simply returns the value of the
+current execution context; which is defined by `before()` and `after()` calls.
+
+Now only using `scope` to graph resource allocation results in the following:


Remove the word Now

trevnorris · 2017-05-23T07:25:29Z

@AndreasMadsen

I want to keep the "Standalone JS API" private for now.

Can we keep it exposed but undocumented? Could even attach the functions to process.binding('async_wrap') (though that may be a bit of a hack). I would like users like yourself that really dig deep and experiment to have access and be able to try it out. And TBH I'm using it right now, but guess that shouldn't be a reason to keep it. :P

AndreasMadsen · 2017-05-23T07:50:12Z

Can we keep it exposed but undocumented?

Yes, that is all I want. I have quite a lot of concerns about abuse, recently I got some new about setting the triggerId for foreign resources. I'm sorry that I don't more time to put it into a coherent writing right now.

sam-github

Lots of great info here on a very large and subtle API, good work.

sam-github · 2017-05-23T18:34:39Z

doc/api/async_hooks.md

+```js
+const async_hooks = require('async_hooks');
+
+// Return the id of the current execution context.


sam-github · 2017-05-23T18:37:17Z

doc/api/async_hooks.md

+function init(id, type, triggerId, resource) { }
+
+// before() is called just before the resource's callback is called. It can be
+// called 0-N times for handles (e.g. TCPWrap), and should be called exactly 1


"should" -> "will", should implies a goal that may not always be achieved, I don't think that is the case here.

Reason I said "should be" was to communicate at the time that since it's an experimental API there's a possibility that it may not be called, and if it isn't then there's a bug.

sam-github · 2017-05-23T18:38:05Z

doc/api/async_hooks.md

+function before(id) { }
+
+// after() is called just after the resource's callback has finished.
+// This hook will not be called if an uncaught exception occurred.


occured where? in the async_hook callback? In the user's callback function?

Also: Is this actually true? I have tried to reproduce this, but it’s all a bit tricky to see through. If it is true that after won’t be called, I’d call that a bug? How would the hooks know that the execution context is exited in that case? @nodejs/diagnostics

@addaleax that is left over documentation from a previous implementation. The after callbacks should always be called.

sam-github · 2017-05-23T18:38:43Z

doc/api/async_hooks.md

+
+// destroy() is called when an AsyncWrap instance is destroyed. In cases like
+// HTTPParser where the resource is reused, or timers where the handle is only
+// a JS object, destroy() will be triggered manually soon after after() has


JS->Javascript

sam-github · 2017-05-23T18:39:51Z

doc/api/async_hooks.md

+
+// destroy() is called when an AsyncWrap instance is destroyed. In cases like
+// HTTPParser where the resource is reused, or timers where the handle is only
+// a JS object, destroy() will be triggered manually soon after after() has


"manually", this sounds like an imp detail, that you are saying that the core C++ won't call destroy(), but it will be arranged to be called by node's javascript code, but is that relevant to the user?

I think with HTTPParser, the handle might be gone before destroy() is actually invoked, so accessing the handle would cause problems. (So as to not call JS during a GC operation.)

sam-github · 2017-05-23T19:41:06Z

doc/api/async_hooks.md

+If the user's callback throws an exception then `emitAfter()` will
+automatically be called for all `id`'s on the stack if the error is handled by
+a domain or `'uncaughtException'` handler. So there is no need to guard against
+this.


If this addresses my question about lack of try/catch above, it probably needs a bit more text, not less, to describe what will happen.

sam-github · 2017-05-23T19:41:46Z

doc/api/async_hooks.md

+* Returns {undefined}
+
+Call all `destroy()` hooks. This should only ever be called once. An error will
+be thrown if it is called more than once. This **must** be manually called. If


as opposed to automatically called? What callbacks are called automatically?

Some of our resource objects are destroyed automatically by the gc. Since they are implemented in C++ they actually call the destroy callbacks appropriately.

sam-github · 2017-05-23T19:42:17Z

doc/api/async_hooks.md

+
+* Returns {number} the unique `id` assigned to the resource. 
+
+Useful when used with `triggerIdScope()`.


what is triggerIdScope()? should this be a link to something? is that a method on something? what?

sam-github · 2017-05-23T19:43:03Z

doc/api/async_hooks.md

+
+#### `asyncEvent.triggerId()`
+
+* Returns {number} the same `triggerId` that is passed to `init()` hooks.


maybe "triggerId that was passed in the AsyncEvent constructor"?

sam-github · 2017-05-23T19:44:52Z

doc/api/async_hooks.md

+The following API can be used as an alternative to using `AsyncEvent()`, but it
+is left to the embedder to manually track values needed for all emitted events
+(e.g. `id` and `triggerId`). It mainly exists for use in Node.js core itself and
+it is highly recommended that embedders instead use `AsyncEvent`.


why do we doc it? does it have a use-case? is there some rare thing it can be used for that the AsyncEvent API cannot be used for? Why don't we use the AsyncEvent API in node core? If its for efficiency, it seems worrying that we are suggesting user-land use an API that we won't use in node core.

The low-level Embedder API is no longer included in the documentation.

AndreasMadsen

As requested here is the commit resolving the "Standalone JS Embedder API" discussion. Agreement with @trevnorris was that we will keep the API but not document it. Hopefully, we can get a better idea of what is needed from userland.

[squash] remove standalone JS Embedder API docs: 2a79a9b735f79bd1f7eef98073ce88e5c014e134

As a bonus, we renamed AsyncEvent to AsyncResource, here is a commit that updates the documentation:

[squash] AsyncEvent is renamed to AsyncResource: 482fccf30dd57ff2a0fa9dd4ba32cc7ba20cf56e

PS: do we plan to add documentation for the C++ Embedder API in this PR?

addaleax · 2017-05-26T21:56:12Z

do we plan to add documentation for the C++ Embedder API in this PR?

I would say “no”, at least for the node::AsyncResource bits; If they are to receive explicit mention in our markdown docs, I’d say that should be the addon docs.

Just as a data point, we don’t document the current MakeCallback API there either; and since it’s kinda customary for C/C++ API docs to be part of the headers, I think that’s okay.

AndreasMadsen · 2017-05-29T17:55:49Z

I'm going through all the reviews. I'm adding a ❤️ to everything I have seen, I will make an updated PR shortly.

AndreasMadsen · 2017-05-30T11:46:09Z

The new PR is in #13287, please review.

thlorenz · 2017-05-30T19:22:32Z

Thanks @AndreasMadsen should I close this one then?

AndreasMadsen · 2017-05-30T19:35:34Z

Sure, we can reopen if I suddenly don't respond.

thlorenz added 3 commits May 10, 2017 15:04

docs: async-hooks documentation

363c4bb

docs: addressing nits pointed out by Trevor

d103566

docs: renaming Number to number

4775684

nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. doc Issues and PRs related to the documentations. labels May 10, 2017

thlorenz mentioned this pull request May 10, 2017

docs: async-hooks documentation thlorenz/node#2

Closed

jasnell requested changes May 11, 2017

View reviewed changes

BridgeAR requested changes May 11, 2017

View reviewed changes

BridgeAR reviewed May 11, 2017

View reviewed changes

vdeturckheim mentioned this pull request May 11, 2017

Impact of async_hook on the CLS othiym23/node-continuation-local-storage#118

Open

AndreasMadsen mentioned this pull request May 11, 2017

[async_hooks] tracking issue nodejs/diagnostics#29

Closed

27 tasks

mcollina requested changes May 11, 2017

View reviewed changes

addaleax reviewed May 11, 2017

View reviewed changes

Fishrock123 self-requested a review May 12, 2017 00:16

docs: addressing simple nits

6cce6d2

thlorenz force-pushed the thlorenz/async-hooks-docs branch from 3108512 to 6cce6d2 Compare May 18, 2017 16:50

addaleax reviewed May 21, 2017

View reviewed changes

jasnell reviewed May 22, 2017

View reviewed changes

sam-github suggested changes May 23, 2017

View reviewed changes

AndreasMadsen suggested changes May 26, 2017

View reviewed changes

AndreasMadsen mentioned this pull request May 29, 2017

docs: documenting async hooks features #13287

Closed

2 tasks

AndreasMadsen closed this May 30, 2017

This was referenced Jul 14, 2017

Remove async_hooks setTrigger API #14238

Closed

Remove async_hooks runInAsyncIdScope API #14328

Closed

AndreasMadsen added the dont-land-on-v6.x label Jul 18, 2017

AndreasMadsen mentioned this pull request Sep 23, 2017

Remove async_hooks Sensitive/low-level Embedder API #15572

Closed

AndreasMadsen mentioned this pull request Oct 12, 2017

async_hooks: async hook stack corruption when exception prevents AsyncResource.emitAfter being called. #16156

Closed


		### Overview

		Here is a simple overview of the public API. All of this API is explained in


		The `async-hooks` module provides an API to register callbacks tracking the
		lifetime of asynchronous resources created inside a Node.js application.


		## Terminology

		An async resource represents either a "handle" or a "request".


		### `class AsyncEvent()`

		The class `AsyncEvent` was designed to be extended from for embedder's async


		## Terminology

		An async resource represents either a _handle_ or a _request_.


		* Returns {number} the unique `id` assigned to the resource.

		Useful when used with `triggerIdScope()`.


		#### `asyncEvent.triggerId()`

		* Returns {number} the same `triggerId` that is passed to `init()` hooks.

docs: documenting async hooks features #12953

docs: documenting async hooks features #12953

Conversation

thlorenz commented May 10, 2017 • edited Loading

Checklist

Affected core subsystem(s)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax May 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreasMadsen commented May 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naugtur May 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreasMadsen May 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thlorenz commented May 17, 2017

trevnorris commented May 17, 2017

AndreasMadsen commented May 17, 2017 • edited Loading

thlorenz commented May 18, 2017

addaleax left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevnorris commented May 23, 2017

AndreasMadsen commented May 23, 2017

sam-github left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thlorenz commented May 10, 2017 •

edited

Loading

addaleax May 18, 2017 •

edited

Loading

AndreasMadsen commented May 11, 2017 •

edited

Loading

naugtur May 11, 2017 •

edited

Loading

AndreasMadsen May 23, 2017 •

edited

Loading

AndreasMadsen commented May 17, 2017 •

edited

Loading

addaleax left a comment •

edited

Loading