docs: documenting async hooks features by AndreasMadsen · Pull Request #13287 · nodejs/node · GitHub

Checklist

documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

async hooks
docs

This is a continuation on #12953 it should "fix" all the suggestions.

It took me 3 hours fix all the issues, so I would like if nits are done by you in a followup PR I simply don't have this much time. Also please be very specific with the suggestions, as in replace "this" with "this".

LGTM modulo typos (if it helps I can fix them myself and push to this PR)

typo: forceing → forcing

call → be called

typo: operations → operation

write → be called

typo: build in → built-in

typos: created. Not **why**. So to → created, not **why**, so to

I think we run the linter on docs now, so this might need to be (conn) instead of conn

comma after made

remove the apostrophe (`id`'s → `id`s)

LGTM modulo typos (if it helps I can fix them myself and push to this PR)

that would be awesome as I will go to bed soon. For minor things, I find this much more helpful.

A word is missing somewhere in this sentence: a resource can also be closed the callback is called

Thanks.

a resource can also be closed the before callback is called

Looking good. My bad on not seeing most of these before now.

The class name is actually AsyncHook. Reason the module is called async_hooks (plural) was to match events and timers (both plural but their class names aren't).

A resource can also be closed before the callback is called.

It looks like we're abstracting the difference between a request and an handle? Personally that would make me sad. Spent a lot of time precisely describing the difference between the two. :P

There are zero references to request and handle in the of the documentation (except for the previous terminology section). If request and handle are to be included, it should be described exactly how those concepts are visible in async_hooks.

see: #12953 (comment)

Should we just drop this whole section and replace it with common patterns and questions? e.g. "Handling events only once" in Events.

There are some corrections this section needs, but not going to comment b/c it may possibly be removed.

We definitely need a common patterns section, but let's not do it in this PR. When we write that I'm fine with removing this.

why the early return after "operation."? Looks like this should be a single paragraph.

I'd either simplify this to:

The callbacks init()/before()/after()/destroy() are called for the respective asynchronous event during a resource's lifetime.

Or take the time to explain what "registered" means:

The callbacks init()/before()/after()/destroy() are registered to a global list of hooks that are called for each respective asynchronous event during a resource's lifetime.

Possibly add that enabling an empty AsyncHook instance is a noop. Or add this in the enable() API section.

I presented the idea elsewhere, but what about just disabling the ability to call a hook while other hooks are running? That would solve this easily (and I think it's reasonable that we don't need to give async statistics while the async hooks are running).

please respond to #12953 (comment)

/cc @jasnell

We should make the document consistent between before()/after() or before/after or before/after. No personal preference, but is more aesthetically pleasing.

Instead:

It is important that before/after calls are unwound in the same order [...]

If this is supposed to be an API overview then it should probably have the sub-heading that says that. But it may also want to be removed, like I discussed above or the other similar section.

Add that the default is currentId()

Again, we can't document details in the summary.

I think this should either be changed to asyncId() or currentAsyncId() (to follow the change for the function argument names). If the latter then triggerId() should probably be changed to currentTriggerId(). I'm down to hear alternatives, but would like to see it be more explicit and uniform. (sorry, my bad)

This would have been a really nice comment to make before node.js 8 was released.

/cc @addaleax

Yeah, it would be nice to have done that before, but I don’t think it needs to be a big deal.

currentAsyncId() and currentTriggerId() sound fine to me. We can always make the old names (non-enumerable?) aliases for those, so we don’t break any existing code.

I'm okay with renaming it, but I think it would be much better to do that in another PR.

May want to do super('DBQuery'). Since this would throw. from source:

    if (typeof type !== 'string' || type.length <= 0)
      throw new TypeError('type must be a string with length > 0');

@trevnorris thanks for the review. I'm not going to fix any of this in the next 24 hours. I have addressed the comments I don't understand.

@AndreasMadsen No worries. I'll get to those you did leave comments on.

@trevnorris please take a look.

I don't see a need for the handle and request terminology, I've described some different cases/examples in the before().
I didn't add the common patterns section, I think we can do that in a follow-up PR. When that is done I'm fine with removing some of the API overviews.
I didn't rename async_hooks.currentId(). I just want this to be a documentation PR, we can rename it in a follow-up PR.

Much thanks for getting this ready.

Landed in 126170a 🎉

(fixed up 2 linter complaints while landing.)

Author: Thorsten Lorenz <thlorenz@gmx.de> Author: Andreas Madsen <amwebdk@gmail.com> PR-URL: #13287 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

This does not land cleanly in LTS. Please feel free to manually backport. Please also feel free to replace do-not-land if it is being backported

thlorenz and others added 14 commits May 29, 2017 21:16

docs: async-hooks documentation

59088f3

[squash] addressing nits pointed out by Trevor

1e35d73

[squash] renaming Number to number

c5db0a6

[squash] addressing simple nits

3218d31

[squash] remove standalone JS Embedder API docs

aa830d7

[squash] AsyncEvent is renamed to AsyncResource

d09735f

[squash] use fs.writeSync and describe issue and solution

fa37457

[squash] async-hooks -> async_hooks

9b9cc15

[squash] addaleax sugestions

affa7a7

[squash] jasnell sugestions

1f23ab2

[squash] id => ID

47a51c1

[squash] sam-github suggesions

9864609

[squash] document resource object and its limitations

de40c74

[squash] more sam-github suggesions

7c9ecd4

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 29, 2017

AndreasMadsen mentioned this pull request May 29, 2017

8.0.0 Release Proposal #12220

Closed

addaleax approved these changes May 29, 2017

View reviewed changes

addaleax and others added 2 commits May 29, 2017 22:03

[squash] fix typos + make linter pass

de82ef4

[squash] fix internal link

d3ee97e

mikesherov reviewed May 29, 2017

View reviewed changes

AndreasMadsen mentioned this pull request May 30, 2017

[async_hooks] tracking issue nodejs/diagnostics#29

Closed

27 tasks

[squash] mikesherov suggesion

4ba706f

AndreasMadsen mentioned this pull request May 30, 2017

docs: documenting async hooks features #12953

Closed

2 tasks

trevnorris suggested changes May 30, 2017

View reviewed changes

Fishrock123 mentioned this pull request May 31, 2017

How do I try the new async_hook debugging features in node 8? nodejs/help#645

Closed

[squash] trevnorris suggestions

82149a8

trevnorris approved these changes Jun 1, 2017

View reviewed changes

addaleax closed this Jun 1, 2017

AndreasMadsen deleted the async-hooks-docs branch June 1, 2017 21:05

AndreasMadsen mentioned this pull request Jun 4, 2017

async_hooks: use resource objects for Promises #13452

Closed

4 tasks

jasnell mentioned this pull request Jun 5, 2017

8.1.0 Proposal #13483

Closed

gibfahn mentioned this pull request Jun 15, 2017

Auditing for 6.11.1 nodejs/Release#230

Closed

3 tasks

MylesBorins added the dont-land-on-v6.x label Jul 17, 2017

soreavis mentioned this pull request Jul 18, 2026

doc: fix triggerAsyncId() comment in async_hooks example #64583

Open

+              If any `AsyncHook` callbacks throw, the application will print the stack trace
+              and exit. The exit path does follow that of an uncaught exception but
+              all `uncaughtException` listeners are removed, thus forceing the process to
+              exit. The `'exit'` callbacks will still call unless the application is run with

+              An asynchronous resource represents an object with an associated callback.
+              This callback may be called multiple times, for example, the `connection` event
+              in `net.createServer`, or just a single time like in `fs.open`. A resource
+              can also be closed before the callback is called. AsyncHooks does not

+              All callbacks are optional. So, for example, if only resource cleanup needs to
+              be tracked then only the `destroy()` callback needs to be passed. The
+              specifics of all functions that can be passed to `callbacks` is in the section
+              [`Hook Callbacks`][].

+              track of what caused the asynchronous operation using the information
+              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.

+              * arguments
+                * `type` {string} the type of ascyc event
+                * `triggerId` {number} the ID of the execution context that created this async
+                  event


		##### Printing in AsyncHooks callbacks

		Because printing to the console is an asynchronous operations `console.log()`

+              ##### Printing in AsyncHooks callbacks
+              Because printing to the console is an asynchronous operations `console.log()`
+              will cause the AsyncHooks callbacks to write. Using `console.log()` or similar

+              The `type` is a string that represents the type of resource that caused
+              `init()` to call. Generally it will be the name of the resource's constructor.
+              The resource types provided by the build in Node.js modules are:

+              the user's callback is placed in a `process.nextTick()`.
+              The graph only shows **when** a resource was created. Not **why**. So to track
+              the **why** use `triggerId`.

+              For example:
+              ```js
+              const server = net.createServer(conn => {

+              * Returns {undefined}
+              Call all `before()` callbacks and let them know a new asynchronous execution
+              context is being entered. If nested calls to `emitBefore()` are made the stack


		* Returns {undefined}

		Call all `after()` callbacks. If nested calls to `emitBefore()` were made then

+              make sure the stack is unwound properly. Otherwise an error will be thrown.
+              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

+              // destroy() is called when an AsyncWrap instance is destroyed.
+              function destroy(id) { }
+              ```

+              Registers functions to be called for different lifetime events of each async
+              operation.
+              The callbacks `init()`/`before()`/`after()`/`destroy()` are registered via an


		The `init()` hook will trigger when an `AsyncResource` is instantiated.

		It is important that before/after calls are unwined

+              // Return the trigger ID for the AsyncResource instance.
+              asyncResource.triggerId();
+              ```

+              never called, causing a memory leak in the application. Of course if
+              the resource doesn't depend on GC then this isn't an issue.
+              #### `async_hooks.currentId()`

+              ```js
+              class DBQuery extends AsyncResource {
+                constructor(db) {
+                  super();

docs: documenting async hooks features by AndreasMadsen · Pull Request #13287 · nodejs/node · GitHub

Uh oh!

Uh oh!

docs: documenting async hooks features - #13287

docs: documenting async hooks features#13287AndreasMadsen wants to merge 18 commits intonodejs:masternodejs/node:masterfrom AndreasMadsen:async-hooks-docsCopy head branch name to clipboard

Conversation

AndreasMadsen commented May 29, 2017

Checklist

Affected core subsystem(s)

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndreasMadsen commented May 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trevnorris left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

docs: documenting async hooks features#13287
AndreasMadsen wants to merge 18 commits into
nodejs:masterfrom
AndreasMadsen:async-hooks-docs

AndreasMadsen commented May 29, 2017 •
edited
Loading

AndreasMadsen commented Jun 1, 2017 •
edited
Loading