AsyncWorker: make callback optional

`AsyncWorker` assumed that after work is complete, a JavaScript callback would need to execute. This change removes the restriction of specifying a `Function` callback, and instead replaces it with an `Env` parameter. Since the purpose of `receiver` was for the `this` context for the callback, it has also been removed from the constructors. Re: #231 (comment) PR-URL: #489 Reviewed-By: Michael Dawson <[email protected]>
nodejs · Jun 18, 2019 · b2b0812 · b2b0812
1 parent a0cac77
commit b2b0812
Show file tree

Hide file tree

Showing 6 changed files with 142 additions and 8 deletions.
diff --git a/doc/async_worker.md b/doc/async_worker.md
@@ -105,8 +105,8 @@ virtual void Napi::AsyncWorker::Execute() = 0;
 
 ### OnOK
 
-This method is invoked when the computation in the `Excecute` method ends.
-The default implementation runs the Callback provided when the AsyncWorker class
+This method is invoked when the computation in the `Execute` method ends.
+The default implementation runs the Callback optionally provided when the AsyncWorker class
 was created.
 
 ```cpp
@@ -149,7 +149,7 @@ explicit Napi::AsyncWorker(const Napi::Function& callback);
 - `[in] callback`: The function which will be called when an asynchronous
 operations ends. The given function is called from the main event loop thread.
 
-Returns a`Napi::AsyncWork` instance which can later be queued for execution by calling
+Returns a `Napi::AsyncWorker` instance which can later be queued for execution by calling
 `Queue`.
 
 ### Constructor
@@ -166,7 +166,7 @@ operations ends. The given function is called from the main event loop thread.
 identifier for the kind of resource that is being provided for diagnostic
 information exposed by the async_hooks API.
 
-Returns a `Napi::AsyncWork` instance which can later be queued for execution by
+Returns a `Napi::AsyncWorker` instance which can later be queued for execution by
 calling `Napi::AsyncWork::Queue`.
 
 ### Constructor
@@ -185,7 +185,7 @@ information exposed by the async_hooks API.
 - `[in] resource`: Object associated with the asynchronous operation that
 will be passed to possible async_hooks.
 
-Returns a `Napi::AsyncWork` instance which can later be queued for execution by
+Returns a `Napi::AsyncWorker` instance which can later be queued for execution by
 calling `Napi::AsyncWork::Queue`.
 
 ### Constructor
@@ -200,7 +200,7 @@ explicit Napi::AsyncWorker(const Napi::Object& receiver, const Napi::Function& c
 - `[in] callback`: The function which will be called when an asynchronous
 operations ends. The given function is called from the main event loop thread.
 
-Returns a `Napi::AsyncWork` instance which can later be queued for execution by
+Returns a `Napi::AsyncWorker` instance which can later be queued for execution by
 calling `Napi::AsyncWork::Queue`.
 
 ### Constructor
@@ -241,6 +241,54 @@ will be passed to possible async_hooks.
 Returns a `Napi::AsyncWork` instance which can later be queued for execution by
 calling `Napi::AsyncWork::Queue`.
 
+
+### Constructor
+
+Creates a new `Napi::AsyncWorker`.
+
+```cpp
+explicit Napi::AsyncWorker(Napi::Env env);
+```
+
+- `[in] env`: The environment in which to create the `Napi::AsyncWorker`.
+
+Returns an `Napi::AsyncWorker` instance which can later be queued for execution by calling
+`Napi::AsyncWorker::Queue`.
+
+### Constructor
+
+Creates a new `Napi::AsyncWorker`.
+
+```cpp
+explicit Napi::AsyncWorker(Napi::Env env, const char* resource_name);
+```
+
+- `[in] env`: The environment in which to create the `Napi::AsyncWorker`.
+- `[in] resource_name`: Null-terminated strings that represents the
+identifier for the kind of resource that is being provided for diagnostic
+information exposed by the async_hooks API.
+
+Returns a `Napi::AsyncWorker` instance which can later be queued for execution by
+calling `Napi::AsyncWorker::Queue`.
+
+### Constructor
+
+Creates a new `Napi::AsyncWorker`.
+
+```cpp
+explicit Napi::AsyncWorker(Napi::Env env, const char* resource_name, const Napi::Object& resource);
+```
+
+- `[in] env`: The environment in which to create the `Napi::AsyncWorker`.
+- `[in] resource_name`:  Null-terminated strings that represents the
+identifier for the kind of resource that is being provided for diagnostic
+information exposed by the async_hooks API.
+- `[in] resource`: Object associated with the asynchronous operation that
+will be passed to possible async_hooks.
+
+Returns a `Napi::AsyncWorker` instance which can later be queued for execution by
+calling `Napi::AsyncWorker::Queue`.
+
 ### Destructor
 
 Deletes the created work object that is used to execute logic asynchronously.

diff --git a/napi-inl.h b/napi-inl.h
@@ -3522,6 +3522,32 @@ inline AsyncWorker::AsyncWorker(const Object& receiver,
   NAPI_THROW_IF_FAILED_VOID(_env, status);
 }
 
+inline AsyncWorker::AsyncWorker(Napi::Env env)
+  : AsyncWorker(env, "generic") {
+}
+
+inline AsyncWorker::AsyncWorker(Napi::Env env,
+                                const char* resource_name)
+  : AsyncWorker(env, resource_name, Object::New(env)) {
+}
+
+inline AsyncWorker::AsyncWorker(Napi::Env env,
+                                const char* resource_name,
+                                const Object& resource)
+  : _env(env),
+    _receiver(),
+    _callback(),
+    _suppress_destruct(false) {
+  napi_value resource_id;
+  napi_status status = napi_create_string_latin1(
+      _env, resource_name, NAPI_AUTO_LENGTH, &resource_id);
+  NAPI_THROW_IF_FAILED_VOID(_env, status);
+
+  status = napi_create_async_work(_env, resource, resource_id, OnExecute,
+                                  OnWorkComplete, this, &_work);
+  NAPI_THROW_IF_FAILED_VOID(_env, status);
+}
+
 inline AsyncWorker::~AsyncWorker() {
   if (_work != nullptr) {
     napi_delete_async_work(_env, _work);
@@ -3587,11 +3613,15 @@ inline void AsyncWorker::SuppressDestruct() {
 }
 
 inline void AsyncWorker::OnOK() {
-  _callback.Call(_receiver.Value(), std::initializer_list<napi_value>{});
+  if (!_callback.IsEmpty()) {
+    _callback.Call(_receiver.Value(), std::initializer_list<napi_value>{});
+  }
 }
 
 inline void AsyncWorker::OnError(const Error& e) {
-  _callback.Call(_receiver.Value(), std::initializer_list<napi_value>{ e.Value() });
+  if (!_callback.IsEmpty()) {
+    _callback.Call(_receiver.Value(), std::initializer_list<napi_value>{ e.Value() });
+  }
 }
 
 inline void AsyncWorker::SetError(const std::string& error) {

diff --git a/napi.h b/napi.h
@@ -1800,6 +1800,13 @@ namespace Napi {
                          const char* resource_name,
                          const Object& resource);
 
+    explicit AsyncWorker(Napi::Env env);
+    explicit AsyncWorker(Napi::Env env,
+                         const char* resource_name);
+    explicit AsyncWorker(Napi::Env env,
+                         const char* resource_name,
+                         const Object& resource);
+
     virtual void Execute() = 0;
     virtual void OnOK();
     virtual void OnError(const Error& e);

diff --git a/test/asyncworker-nocallback.js b/test/asyncworker-nocallback.js
@@ -0,0 +1,15 @@
+'use strict';
+const buildType = process.config.target_defaults.default_configuration;
+const common = require('./common');
+
+test(require(`./build/${buildType}/binding.node`));
+test(require(`./build/${buildType}/binding_noexcept.node`));
+
+function test(binding) {
+  const resolving = binding.asyncworker.doWorkNoCallback(true, {});
+  resolving.then(common.mustCall()).catch(common.mustNotCall());
+
+  const rejecting = binding.asyncworker.doWorkNoCallback(false, {});
+  rejecting.then(common.mustNotCall()).catch(common.mustCall());
+  return;
+}
diff --git a/test/asyncworker.cc b/test/asyncworker.cc
@@ -29,8 +29,41 @@ class TestWorker : public AsyncWorker {
   bool _succeed;
 };
 
+class TestWorkerNoCallback : public AsyncWorker {
+public:
+  static Value DoWork(const CallbackInfo& info) {
+    napi_env env = info.Env();
+    bool succeed = info[0].As<Boolean>();
+    Object resource = info[1].As<Object>();
+
+    TestWorkerNoCallback* worker = new TestWorkerNoCallback(env, "TestResource", resource);
+    worker->_succeed = succeed;
+    worker->Queue();
+    return worker->_deferred.Promise();
+  }
+
+protected:
+  void Execute() override {
+  }
+  virtual void OnOK() override {
+      _deferred.Resolve(Env().Undefined());
+
+  }
+  virtual void OnError(const Napi::Error& /* e */) override {
+      _deferred.Reject(Env().Undefined());
+  }
+
+private:
+  TestWorkerNoCallback(napi_env env, const char* resource_name, const Object& resource)
+      : AsyncWorker(env, resource_name, resource), _deferred(Napi::Promise::Deferred::New(env)) {
+      }
+  Promise::Deferred _deferred;
+  bool _succeed;
+};
+
 Object InitAsyncWorker(Env env) {
   Object exports = Object::New(env);
   exports["doWork"] = Function::New(env, TestWorker::DoWork);
+  exports["doWorkNoCallback"] = Function::New(env, TestWorkerNoCallback::DoWork);
   return exports;
 }
diff --git a/test/index.js b/test/index.js
@@ -11,6 +11,7 @@ let testModules = [
   'arraybuffer',
   'asynccontext',
   'asyncworker',
+  'asyncworker-nocallback',
   'asyncworker-persistent',
   'basic_types/array',
   'basic_types/boolean',