What is promise_async_tag() used for, preventing garbage collection? If so I think a comment should be added.

The only reference to it is held by the promise. We use its lifespan to approximate when the promise was garbage collected. I've added a comment describing this.

Do we want the user to be able to alter these? In src/async-wrap.cc I used

  v8::PropertyAttribute ReadOnlyDontDelete =
      static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontDelete);

Though I probably should have added DontEnum to that list.

This property will be going away as soon as we move to an internal property on promises (blocked on #13175). I can modify this but I intend to get the promise internal property refactoring in before 8.0.

I've added this for now.

kResolve only happens once per promise, when the promise is resolved, right? In that case, I think we can just put the current async id that caused the resolve on the PromiseWrap object. That should provide most of the additional information.

Yes, there is also the actual event but that information can be inferred when kBefore is emitted. In case .then() is never called, I don't this it is that interesting when the promise is resolved since it is never used.

kResolve only happens once per promise

yep :)

when the promise is resolved, right?

You’d think so, wouldn’t you? 😄 This isn’t called when the promise is resolved, it’s at the beginning of when the resolve function (as in new Promise((resolve) => …)) or some equivalent of it is called. That might not be when the Promise is resolved, though (because you can call resolve() with other pending promises).

I’m not actually sure kResolve is a very interesting thing for async hooks?

I'm not sure if a promise's then correlates with kResolve, but a promise's then can be called multiple times.

This isn’t called when the promise is resolved, it’s at the beginning of when the resolve function (as in new Promise((resolve) => …)) or some equivalent of it is called.

Haha, that was actually what I meant, when resolve() is called. I always mix up my promise terminology.

I’m not actually sure kResolve is a very interesting thing for async hooks?

This was perhaps the most discussed thing in the async_hooks EPS. This comment descibes the two viewpoints nodejs/node-eps#18 (comment), though I don't really agree with the "who needs what" part (stack-trace vs CLS) after talking with APM providers (@watson).

I think adding the async id that called resolve() to the PromiseWrap is the best option for satisfying both parties.

This is an example scenario that almost captures all the possibilities that Promises and async hooks can interact:

(function runner() {
  const p = new Promise(function p_fn0(res) {
    setImmediate(function si0() {
      res('foo')
    });
  }).then(function then0(val) {
    return Fn(val);
  });

  setImmediate(function si0() {
    p.catch(function catch0(err) {
      printLongErrorStack(err);  // not defined
    });
  });

  function Fn(val) {
    if (val === 'foo') throw new Error('bam');
  }
})();

If each Promise executor, onFulfilled or onRejected only collect the stack when new Promise() or .then() runs then the stack would be:

    at printLongErrorStack
    at catch0
    at si0
    at runner

Though it would be helpful to produce the following long stack:

    at printLongErrorStack
    at catch0
    at Fn
    at then0
    at si0
    at p_fn0
    at runner

I've had some lengthy discussions about this and, though I don't remember with whom, I recall agreeing that the logical place to execute the AsyncEvent hooks would produce the first call stack; but it should also be possible to produce the second call stack (@matthewloring was this conversation with you?).

So for this PR the first stack above is expected, but we should also address how to produce the second. IIRC the API in in place to allow watching for resolve or reject calls. At which point a stack can be produced then (somehow?) attached to the Promise handle that's responsible for the current execution scope.

Yup, this conversation was with me and I think it captures the concern with attaching the async id to the promise resource at resolution time so that it can be read in kBefore. This approach does not allow async_hook users to "react" to a promise resolution (state cannot be observed/updated at resolution time). Would it be too much to add another async_hook type for the resolution of async work (promise or otherwise)? With the addition of the C++ embedder api I can imagine that other types of async work could notify when async operations completes even if the JS callback isn't invoked immediately.

@matthewloring Adding another type that fires at a different time wouldn't be a problem.

This approach does not allow async_hook users to "react" to a promise resolution (state cannot be observed/updated at resolution time). Would it be too much to add another async_hook type for the resolution of async work (promise or otherwise)? With the addition of the C++ embedder api I can imagine that other types of async work could notify when async operations completes even if the JS callback isn't invoked immediately.

While it is not a technical issue to add another type (I assume you mean event type and not resource type), I think it would be better to do in a future PR where we have a better chance to evaluate the needs for such event type for other cases than promises.

So far the only concern mentioned in the EPS has been regarding context across async boundaries, which "attaching the async id to the promise resource" does solve, although not in a particularly nice way.

Addressing this in a follow on seems fine to me.