WIP: N-API: Implement Promise

[gabrielschulhof]

napi_create_promise() creates a promise object and returns the resolve
and reject callbacks as JS functions which can be called with
napi_call_function(), passing a napi_value as the sole parameter to
indicate the resolution of the promise resp. the reason for rejection.

napi_is_promise() is a type check API returning true if the value
passed in is a promise, and false otherwise.

Fixes nodejs/abi-stable-node#242

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

N-API

[TimothyGu]

Personally I'd much prefer the v8::Promise::Resolver pattern which provides a kind of "deferred" object. It's also seen in the ES spec as a PromiseCapability.

// status checks omitted
napi_value promise, resolution;
napi_create_promise(env, &promise);
napi_create_number(env, 42, &resolution);
napi_resolve_promise(env, &promise, &resolution);

[addaleax]

Can you use added: REPLACEME here (and below)?

[addaleax]

I think there should be a comma after actions

[addaleax]

I think it might be worth mentioning that this checks whether it is a built-in native Promise, not just any promise object that implements the Promises/A+ interface

[addaleax]

Can you use left-aligning style (i.e. void* data)?

[addaleax]

static_cast

[addaleax]

Can you try to align the arguments vertically? (If that brings the line over 80 chars: wrap after the ( and indent both arguments with 4 spaces)

[addaleax]

Why the parentheses?

[addaleax]

If you want to be lazy, you can just do common.mustNotCall() instead of the assert.ok(false) calllback

[addaleax]

The reject callback should probably be wrapped in common.mustCall(…)

[gabrielschulhof]

@TimothyGu I also started with that concept at first, but I realized that if I want to provide that interface, I have to tack the v8::Resolver onto the v8::Promise in some inconspicuous way so that when it comes back from JS, I can retrieve it and call Resolve() or Reject() on it.

Another drawback is that, if you get a Promise that was created in JS, you can't resolve because it doesn't have any v8::Resolver tacked on, even though you have an API napi_resolve_promise() that sounds like it should work on any promise, even those created in JS. So, on the native side, not all promises are created equal.

Now, the fact that you cannot resolve a promise you receive from JS is also true with the current API, but with the current API you do not have a function that sounds like it's able to resolve a promise.

[addaleax]

Another drawback is that, if you get a Promise that was created in JS, you can't resolve because it doesn't have any v8::Resolver tacked on, even though you have an API napi_resolve_promise() that sounds like it should work on any promise, even those created in JS. So, on the native side, not all promises are created equal.

Just by the way, V8 doesn’t actually make a difference between the Promise::Resolver object and the Promise itself; v8::Promise::Resolver::GetPromise() is essentially a { return this }.

[gabrielschulhof]

OTOH, we could go with

// We introduce the concept of the deferred in the documentation, and that
// resolving a promise requires the deferred, not the promise.
NAPI_EXTERN napi_status napi_create_deferred(napi_env env,
                                             napi_value* result);
NAPI_EXTERN napi_status napi_get_promise_from_deferred(napi_env env,
                                                       napi_value deferred,
                                                       napi_value* promise);

NAPI_EXTERN napi_status napi_resolve_deferred(napi_env env,
                                              napi_value deferred,
                                              napi_value conclusion);
NAPI_EXTERN napi_status napi_reject_deferred(napi_env env,
                                             napi_value deferred,
                                             napi_value conclusion);

[gabrielschulhof]

@addaleax OK, but is that an implementation detail I can rely on? I also need to see how other engines do this because, whatever we go with needs to work across multiple engines.

[gabrielschulhof]

If we go with the deferred, we have considerable leeway in how we construct the deferred, meaning we should have ample elbow room across engines.

[gabrielschulhof]

For deferred we could even introduce a new type, like napi_deferred, which should indicate that it is not a value to be passed into JS.

[mhdawson]

@gabrielschulhof is there a fundamental reason we can't resolve a promise created in js in native code ?

[mhdawson]

The test uses the pattern where the promise is created in the native code and the resolve, reject are called through JS. If this is the most common use case I wonder a helper (or simply returning it from the original call) to create JS object would make any sense.

[gabrielschulhof]

I don't believe the primary use case is to return a promise to JS so that it may be resolved in JS. That can be accomplished without native code.

I believe the primary use case is to return a promise to JS that will be resolved from the native side - in which case you would never construct such an object. In fact, I only construct it here to make testing out the promise easier by allowing one to use JS to resolve/reject the promise.

OTOH, you're right in that you might have to pass the resolve and reject around the native code such that they are always together, resulting in helper structures or a fatter stack. The deferred paradigm helps here too, because then you only have to pass one object around - the deferred.

[gabrielschulhof]

... and in V8 at least, the deferred has engine support for implementing napi_resolve_deferred() and napi_reject_deferred() in the form of v8::Promise::Resolver::Resolve() resp. v8::Promise::Resolver::Reject().

In ChakraCore I suspect the deferred will take the form of a JS object with two methods.

[gabrielschulhof]

@mhdawson if we look at the following code

addon.resolve_a_promise(new Promise(function(resolve, reject) {
  do_something_asynchronous(function(error, data) {
    if (error) {
      reject(error);
      return;
    }
    resolve(data);
  });
}));

then, if the native implementation of resolve_a_promise() were to resolve/reject the promise it receives as its argument, that would mean that it shoves aside the JS code performing do_something_asynchronous() and resolves/rejects it in its own way. This is non-obvious and unintuitive. Thus, it's best if the API does not allow resolving/rejecting promises created in JS even if it's fundamentally possible.

At the same time, we must not write an API which ends up having to distinguish between promises created by native code and promises created with JS and passed into native code.

Fundamentally, only the place where the promise was created should be the place where it can be resolved. In JS this is easy to enforce by the way in which a promise is constructed. On the native side this locality could never really be enforced because one could simply take persistent references of the resolve and reject and pass them wherever.

Thus, I believe the deferred paradigm is better. The code which has access to the deferred is able to resolve/reject the promise, whereas the code which has access to only the promise is only able to chain off of it, but is unable to resolve/reject it.

[TimothyGu]

@gabrielschulhof Thank you for the explanation, and it makes sense to me why a deferred paradigm is preferred. Now about implementation: I'm somewhat unconvinced that it should be a new type rather than napi_value, for the simple reason that existing infrastructure (like escapable scope handle) would need to be duplicated for this new type. On the other hand, I see the rationale for using a new type to prevent type confusion. What do you think?

[gabrielschulhof]

@TimothyGu @addaleax @mhdawson I have now pushed the implementation that uses deferred.

[TimothyGu]

napi_create_deferred (as it is in the heading of the section)?

[TimothyGu]

Do we need a corresponding napi_is_deferred?

[gabrielschulhof]

There's no corresponding IsResolver() in V8. I could mark it with some funky property, but that would be duck typing.

[TimothyGu]

There should be a test making sure any attempt to pass a deferred into JS will error out (in fact I'm not sure it will currently).

[gabrielschulhof]

It's not forbidden to pass a deferred into JS, it's just not very useful.

[TimothyGu]

From #13717 (comment), it seems like V8 treats a Resolver just like a Promise, which means that if the user passes the deferred (backed by a Resolver) back into JS the separation between deferred and promise would not be enforced, and the problem you outlined in #13717 (comment) would still be there

[gabrielschulhof]

@TimothyGu the fact that a v8::Promise::Resolver === v8::Promise seems like a shortcoming of V8. The only thing I can think of doing on the N-API side to enforce the distinction between a deferred and a promise is somewhat convoluted:

• To cover creation:
  1. Declare a JS class "Deferred" and give it an internal field. This would currently have to be done once per N-API module, because we don't have proper per-isolate storage. This means that passing a "Deferred" between N-API modules would not work - which is perhaps not a bad thing.
  2. Store a persistent reference to an instance of v8::Resolver in the internal field of an instance of Deferred.
  3. Allow the instance of Deferred to be passed into JS.
• To cover usage:
  1. Given a napi_value that claims to be a Deferred, test it with napi_instanceof() against the Deferred constructor.
  2. If it passes, then retrieve the internal field and obtain a local reference to the Resolver from the persistent reference.

The only alternative I can currently think of is documentation. Put bluntly, "Yes, you can pass a Deferred into JS, yes it may even behave like a promise, but we're telling you not to do that, because it's not guaranteed to behave like a promise if we ever swap out the underlying engine or if V8 decides to introduce a distinction, and we will make no efforts to support that use case given that you can easily retrieve a bona fide JavaScript promise from a Deferred."

[gabrielschulhof]

Of course, if we were to use the original interface of

napi_create_promise(env, &promise, &resolve, &reject);

where resolve and reject are to be called with napi_call_function() then this problem would go away, but, like you said, this interface is preferable to that one.

[gabrielschulhof]

I guess, come to think of it, doing things the convoluted way is not that different from going part-ways towards implementing the original &promise, &resolve, &reject interface, except that resolve and reject are rolled into one.

[mhdawson]

It does seem like we'd want to prevent the deferred object from being used as promise as I don't think we can ensure that's the case for all js implementations. I think it will just be too easy for an impl to pass back a deferred and then use it as a promise, and if it works we'll have code that breaks across different runtimes.

We use this for wrap/unwrap:

  // Search the object's prototype chain for the wrapper with an internal field.
  // Usually the wrapper would be the first in the chain, but it is OK for
  // other objects to be inserted in the prototype chain.
  v8::Local<v8::Object> wrapper = obj;
  do {
    v8::Local<v8::Value> proto = wrapper->GetPrototype();
    RETURN_STATUS_IF_FALSE(
      env, !proto.IsEmpty() && proto->IsObject(), napi_invalid_arg);
    wrapper = proto.As<v8::Object>();
  } while (wrapper->InternalFieldCount() != 1);

  v8::Local<v8::Value> unwrappedValue = wrapper->GetInternalField(0);
  RETURN_STATUS_IF_FALSE(env, unwrappedValue->IsExternal(), napi_invalid_arg);

  *result = unwrappedValue.As<v8::External>()->Value();

So maybe we should generalize the code and use it for wrap and for creating a deferred object where the resolver is hidden in an internal field of the prototype chain ?

We could also strengthen it by adding a type as a secondary field so that we can more strongly validate it is what we expect it is.

[gabrielschulhof]

OK, then I'll take the type-enforcing wrapping approach.

[TimothyGu]

And there should be a test making sure passing a promise into napi_conclude_deferred is disallowed.

[gabrielschulhof]

I'd like to wait on the outcome of #13872 before proceeding here, because I'd like to factor out how we wrap things once that lands.