search

nodejs / node-addon-api

Module for using Node-API from C++
MIT License
2.1k stars 457 forks source link

How about add C++20 coroutine support to `Napi::Value`? #1456

Closed toyobayashi closed 1 month ago

toyobayashi commented 4 months ago

embind already has coroutine implementation

https://github.com/emscripten-core/emscripten/blob/b5b7fedda835bdf8f172a700726109a4a3899909/system/include/emscripten/val.h#L703-L789

I just now tried to write a toy version, that makes it possible to co_await a JavaScript Promise in C++.

class CoPromise : public Napi::Promise
```cpp #include #include #include class CoPromise : public Napi::Promise { public: CoPromise(napi_env env, napi_value value): Napi::Promise(env, value) {}; class promise_type { private: Napi::Env env_; Napi::Promise::Deferred deferred_; public: promise_type(const Napi::CallbackInfo& info): env_(info.Env()), deferred_(Napi::Promise::Deferred::New(info.Env())) {} CoPromise get_return_object() const { return deferred_.Promise().As(); } std::suspend_never initial_suspend () const noexcept { return {}; } std::suspend_never final_suspend () const noexcept { return {}; } void unhandled_exception() const { std::exception_ptr exception = std::current_exception(); try { std::rethrow_exception(exception); } catch (const Napi::Error& e) { deferred_.Reject(e.Value()); } catch (const std::exception &e) { deferred_.Reject(Napi::Error::New(env_, e.what()).Value()); } catch (const std::string& e) { deferred_.Reject(Napi::Error::New(env_, e).Value()); } catch (const char* e) { deferred_.Reject(Napi::Error::New(env_, e).Value()); } catch (...) { deferred_.Reject(Napi::Error::New(env_, "Unknown Error").Value()); } } void return_value(Value value) const { Resolve(value); } void Resolve(Value value) const { deferred_.Resolve(value); } void Reject(Value value) const { deferred_.Reject(value); } }; class Awaiter { private: Napi::Promise promise_; std::coroutine_handle handle_; Napi::Value fulfilled_result_; public: Awaiter(Napi::Promise promise): promise_(promise), handle_(), fulfilled_result_() {} constexpr bool await_ready() const noexcept { return false; } void await_suspend(std::coroutine_handle handle) { handle_ = handle; promise_.Get("then").As().Call(promise_, { Napi::Function::New(promise_.Env(), [this](const Napi::CallbackInfo& info) -> Value { fulfilled_result_ = info[0]; handle_.resume(); return info.Env().Undefined(); }), Napi::Function::New(promise_.Env(), [this](const Napi::CallbackInfo& info) -> Value { handle_.promise().Reject(info[0]); handle_.destroy(); return info.Env().Undefined(); }) }); } Value await_resume() const { return fulfilled_result_; } }; Awaiter operator co_await() const { return Awaiter(*this); } }; ```
binding.gyp
```python { "target_defaults": { "cflags_cc": [ "-std=c++20" ], "xcode_settings": { "CLANG_CXX_LANGUAGE_STANDARD":"c++20" }, # https://github.com/nodejs/node-gyp/issues/1662#issuecomment-754332545 "msbuild_settings": { "ClCompile": { "LanguageStandard": "stdcpp20" } }, }, "targets": [ { "target_name": "binding", "include_dirs": [ "
binding.cpp
```cpp CoPromise NestedCoroutine(const Napi::CallbackInfo& info) { Napi::Env env = info.Env(); Napi::Value async_function = info[0]; if (!async_function.IsFunction()) { throw Napi::Error::New(env, "not function"); } Napi::Value result = co_await async_function.As()({}).As(); co_return Napi::Number::New(env, result.As().DoubleValue() * 2); } CoPromise Coroutine(const Napi::CallbackInfo& info) { Napi::Env env = info.Env(); Napi::Value number = co_await NestedCoroutine(info); co_return Napi::Number::New(env, number.As().DoubleValue() * 2); } CoPromise CoroutineThrow(const Napi::CallbackInfo& info) { Napi::Env env = info.Env(); Napi::Value number = co_await NestedCoroutine(info); throw Napi::Error::New(env, "test error"); co_return Napi::Value(); } Napi::Object Init(Napi::Env env, Napi::Object exports) { exports.Set("coroutine", Napi::Function::New(env, Coroutine)); exports.Set("coroutineThrow", Napi::Function::New(env, CoroutineThrow)); return exports; } NODE_API_MODULE(addon, Init) ```
index.js
```js const binding = require('./build/Release/binding.node') async function main () { await binding.coroutine(function () { return new Promise((resolve, _) => { setTimeout(() => { resolve(42) }, 1000) }) }).then(value => { console.log(value) }).catch(err => { console.error('JS caught error', err) }) await binding.coroutine(function () { return new Promise((_, reject) => { setTimeout(() => { reject(42) }, 1000) }) }).then(value => { console.log(value) }).catch(err => { console.error('JS caught error', err) }) await binding.coroutineThrow(function () { return new Promise((resolve, _) => { setTimeout(() => { resolve(42) }, 1000) }) }).then(value => { console.log(value) }).catch(err => { console.error('JS caught error', err) }) } main() ```
node index.js
``` (1000ms after) 168 (1000ms after) JS caught error 42 (1000ms after) JS caught error [Error: test error] ```
output
KevinEady commented 4 months ago

FWIW, node-addon-api is restricted to the same build restrictions as node, which is c++17.

NickNaso commented 4 months ago

@KevinEady you are right, but we have two choices:

Whaty do you think about?

KevinEady commented 4 months ago

I think in instances where functionality is added that is not specifically a wrapper for Node-API functionality, we defer to placing the functionality in a separate module/package owned by the original code writer (and therefore not maintained by us), eg. https://github.com/nodejs/node-addon-api/issues/1163

toyobayashi commented 4 months ago

@KevinEady Is it a better choice to add promise_type and operator co_await to Napi::Value instead of Napi::Promise? It's similar to JavaScript that can await any type of JavaScript values and the coroutine suspends when await a Thenable. If go this way, since the Napi::Value is the base class of all values, I think place changes of Napi::Value in node-addon-api repo is reasonable. Also adding #if __cplusplus >= 202002L guard to allow optin.

{
  "cflags_cc": [ "-std=c++20" ],
  "xcode_settings": {
    "CLANG_CXX_LANGUAGE_STANDARD":"c++20",
    "OTHER_CPLUSPLUSFLAGS": [ "-std=c++20" ]
  },
  # https://github.com/nodejs/node-gyp/issues/1662#issuecomment-754332545
  "msbuild_settings": {
    "ClCompile": {
      "LanguageStandard": "stdcpp20"
    }
  },
}

for example, I changed my toy implementation and placed it in node_modules/node-addon-api/napi.h

diff --git a/node_modules/node-addon-api/napi.h b/node_modules/node-addon-api/napi.h
```patch diff --git a/node_modules/node-addon-api/napi.h b/node_modules/node-addon-api/napi.h index 9f20cb8..8edc558 100644 --- a/node_modules/node-addon-api/napi.h +++ b/node_modules/node-addon-api/napi.h @@ -20,6 +20,11 @@ #include #include +#if __cplusplus >= 202002L +#include +#include +#endif + // VS2015 RTM has bugs with constexpr, so require min of VS2015 Update 3 (known // good version) #if !defined(_MSC_VER) || _MSC_FULL_VER >= 190024210 @@ -169,6 +174,10 @@ namespace NAPI_CPP_CUSTOM_NAMESPACE { // Forward declarations class Env; class Value; +#if __cplusplus >= 202002L +class ValuePromiseType; +class ValueAwaiter; +#endif class Boolean; class Number; #if NAPI_VERSION > 5 @@ -482,6 +491,12 @@ class Value { MaybeOrValue ToObject() const; ///< Coerces a value to a JavaScript object. +#if __cplusplus >= 202002L + using promise_type = ValuePromiseType; + + ValueAwaiter operator co_await() const; +#endif + protected: /// !cond INTERNAL napi_env _env; @@ -3189,6 +3204,117 @@ class Addon : public InstanceWrap { }; #endif // NAPI_VERSION > 5 +#if __cplusplus >= 202002L + +class ValuePromiseType { + private: + Env env_; + Promise::Deferred deferred_; + + public: + ValuePromiseType(const CallbackInfo& info): + env_(info.Env()), deferred_(Promise::Deferred::New(info.Env())) {} + + Value get_return_object() const { + return deferred_.Promise(); + } + std::suspend_never initial_suspend () const NAPI_NOEXCEPT { return {}; } + std::suspend_never final_suspend () const NAPI_NOEXCEPT { return {}; } + + void unhandled_exception() const { + std::exception_ptr exception = std::current_exception(); +#ifdef NAPI_CPP_EXCEPTIONS + try { + std::rethrow_exception(exception); + } catch (const Error& e) { + deferred_.Reject(e.Value()); + } catch (const std::exception &e) { + deferred_.Reject(Error::New(env_, e.what()).Value()); + } catch (const Value& e) { + deferred_.Reject(e); + } catch (const std::string& e) { + deferred_.Reject(Error::New(env_, e).Value()); + } catch (const char* e) { + deferred_.Reject(Error::New(env_, e).Value()); + } catch (...) { + deferred_.Reject(Error::New(env_, "Unknown Error").Value()); + } +#else + std::rethrow_exception(exception); +#endif + } + + void return_value(Value value) const { + if (env_.IsExceptionPending()) { + Reject(env_.GetAndClearPendingException().Value()); + } else { + Resolve(value); + } + } + + void Resolve(Value value) const { + deferred_.Resolve(value); + } + + void Reject(Value value) const { + deferred_.Reject(value); + } +}; + +class ValueAwaiter { + private: + std::variant state_; + + public: + ValueAwaiter(Value value): state_(std::in_place_index<0>, value) {} + + bool await_ready() { + const Value* value = std::get_if<0>(&state_); + if (value->IsPromise() || (value->IsObject() && value->As().Get("then").IsFunction())) { + return false; + } + state_.emplace<1>(*value); + return true; + } + + void await_suspend(std::coroutine_handle handle) { + Object thenable = std::get_if<0>(&state_)->As(); + Env env = thenable.Env(); + thenable.Get("then").As().Call(thenable, { + Function::New(env, [this, handle](const CallbackInfo& info) -> Value { + state_.emplace<1>(info[0]); + handle.resume(); + return info.Env().Undefined(); + }), + Function::New(env, [this, handle](const CallbackInfo& info) -> Value { + state_.emplace<2>(info[0]); +#ifdef NAPI_CPP_EXCEPTIONS + handle.resume(); +#else + handle.promise().Reject(info[0]); + handle.destroy(); +#endif + return info.Env().Undefined(); + }) + }); + } + + Value await_resume() const { + const Value* ok = std::get_if<1>(&state_); + if (ok) { + return *ok; + } + const Value* err = std::get_if<2>(&state_); + NAPI_THROW(Error(err->Env(), *err), Value()); + } +}; + +inline ValueAwaiter Value::operator co_await() const { + return { *this }; +} + +#endif // __cplusplus >= 202002L + #ifdef NAPI_CPP_CUSTOM_NAMESPACE } // namespace NAPI_CPP_CUSTOM_NAMESPACE #endif ```

Then the usage becomes more nature

#ifdef NAPI_CPP_EXCEPTIONS
#define NAPI_THROW_CO_RETURN(e, ...) throw e
#else
#define NAPI_THROW_CO_RETURN(e, ...)                                           \
  do {                                                                         \
    (e).ThrowAsJavaScriptException();                                          \
    co_return __VA_ARGS__;                                                     \
  } while (0)
#endif

Napi::Value NestedCoroutine(const Napi::CallbackInfo& info) {
  Napi::Env env = info.Env();
  Napi::Value async_function = info[0];
  if (!async_function.IsFunction()) {
    NAPI_THROW_CO_RETURN(Napi::Error::New(env, "not function"), Napi::Value());
  }
  Napi::Value result = co_await async_function.As<Napi::Function>()({});
  result = co_await result; // ok
  co_return Napi::Number::New(env, result.As<Napi::Number>().DoubleValue() * 2);
}

Napi::Value Coroutine(const Napi::CallbackInfo& info) {
  Napi::Env env = info.Env();
  Napi::Value number = co_await NestedCoroutine(info);
  co_return Napi::Number::New(env, number.As<Napi::Number>().DoubleValue() * 2);
}

Napi::Value CoroutineThrow(const Napi::CallbackInfo& info) {
  Napi::Env env = info.Env();
  co_await NestedCoroutine(info);
  NAPI_THROW_CO_RETURN(Napi::Error::New(env, "test error"), Napi::Value());
  co_return Napi::Value();
}

Napi::Object Init(Napi::Env env, Napi::Object exports) {
  exports.Set("coroutine", Napi::Function::New(env, Coroutine));
  exports.Set("coroutineThrow", Napi::Function::New(env, CoroutineThrow));
  return exports;
}

It would be cool if node-addon-api can get this feature.

toyobayashi commented 4 months ago

https://github.com/nodejs/node-addon-api/compare/main...toyobayashi:node-addon-api:coroutine

I added changes and test in my fork. This is a very simple implementation and have not tested complex use case.

mhdawson commented 4 months ago

Following up on @KevinEady's earlier comment about node-addon-api being a thin wrapper, this is documented in https://github.com/nodejs/node-addon-api/blob/main/CONTRIBUTING.md#source-changes.

We discussed in the node-api team meeting today and based on our documented approach we believe this functionality is best covered in a separated module outside of node-addon-api unless that is impossible.

Some team members are going to take a deeper look and we'll talk about it again next time.

github-actions[bot] commented 1 month ago

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

mhdawson commented 1 month ago

We discussed again today, from the discussion in the meeting today, the feeling of the team continuies to be that it can be implemented on top of node-addon-api without needing to be integrated. Can you confirm that?

That along with the agreed approach of keeping node-api and node-addon-api lean as documented in https://github.com/nodejs/node-addon-api/blob/main/CONTRIBUTING.md#source-changes means that we believe it should be implemented in an external libray versus integrated into node-addon-api itself.

Our suggestion is that you create a separate repo/npm package to provide the functionality. If you do that please let us know and we will consider linking to it from the node-addon-api README.md for those who would be interested in co-routine support.

Let us know if that makes sense to you so we can close the issue?

toyobayashi commented 1 month ago

Respect team's opinion, I create this issue just inspired by embind. Thanks for your suggestion.

  • © Githubissues.
  • Githubissues is a development platform for aggregating issues.