diff --git a/napi-inl.h b/napi-inl.h index 82dae5bea..58c69078e 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -1,4 +1,4 @@ -#ifndef SRC_NAPI_INL_H_ +#ifndef SRC_NAPI_INL_H_ #define SRC_NAPI_INL_H_ //////////////////////////////////////////////////////////////////////////////// @@ -38,9 +38,7 @@ inline void RegisterModule(napi_env env, Napi::Object(env, module)); } catch (const Error& e) { - if (!Napi::Env(env).IsExceptionPending()) { - e.ThrowAsJavaScriptException(); - } + e.ThrowAsJavaScriptException(); } } @@ -48,9 +46,7 @@ inline void RegisterModule(napi_env env, // and rethrow them as JavaScript exceptions before returning from the callback. #define NAPI_RETHROW_JS_ERROR(env) \ catch (const Error& e) { \ - if (!Napi::Env(env).IsExceptionPending()) { \ - e.ThrowAsJavaScriptException(); \ - } \ + e.ThrowAsJavaScriptException(); \ return nullptr; \ } @@ -1215,14 +1211,6 @@ inline Value Function::Call(napi_value recv, const std::vector& args return Value(_env, result); } -inline Value Function::MakeCallback(const std::initializer_list& args) const { - return MakeCallback(Env().Undefined(), args); -} - -inline Value Function::MakeCallback(const std::vector& args) const { - return MakeCallback(Env().Undefined(), args); -} - inline Value Function::MakeCallback( napi_value recv, const std::initializer_list& args) const { napi_value result; @@ -1888,16 +1876,6 @@ inline Napi::Value FunctionReference::Call( return scope.Escape(Value().Call(recv, args)); } -inline Napi::Value FunctionReference::MakeCallback(const std::initializer_list& args) const { - EscapableHandleScope scope(_env); - return scope.Escape(Value().MakeCallback(args)); -} - -inline Napi::Value FunctionReference::MakeCallback(const std::vector& args) const { - EscapableHandleScope scope(_env); - return scope.Escape(Value().MakeCallback(args)); -} - inline Napi::Value FunctionReference::MakeCallback( napi_value recv, const std::initializer_list& args) const { EscapableHandleScope scope(_env); @@ -2481,7 +2459,7 @@ inline napi_value ObjectWrap::InstanceVoidMethodCallbackWrapper( InstanceVoidMethodCallbackData* callbackData = reinterpret_cast(callbackInfo.Data()); callbackInfo.SetData(callbackData->data); - T* instance = Unwrap(callbackInfo.This()); + T* instance = Unwrap(callbackInfo.This().As()); auto cb = callbackData->callback; (instance->*cb)(callbackInfo); return nullptr; @@ -2498,7 +2476,7 @@ inline napi_value ObjectWrap::InstanceMethodCallbackWrapper( InstanceMethodCallbackData* callbackData = reinterpret_cast(callbackInfo.Data()); callbackInfo.SetData(callbackData->data); - T* instance = Unwrap(callbackInfo.This()); + T* instance = Unwrap(callbackInfo.This().As()); auto cb = callbackData->callback; return (instance->*cb)(callbackInfo); } @@ -2514,7 +2492,7 @@ inline napi_value ObjectWrap::InstanceGetterCallbackWrapper( InstanceAccessorCallbackData* callbackData = reinterpret_cast(callbackInfo.Data()); callbackInfo.SetData(callbackData->data); - T* instance = Unwrap(callbackInfo.This()); + T* instance = Unwrap(callbackInfo.This().As()); auto cb = callbackData->getterCallback; return (instance->*cb)(callbackInfo); } @@ -2530,7 +2508,7 @@ inline napi_value ObjectWrap::InstanceSetterCallbackWrapper( InstanceAccessorCallbackData* callbackData = reinterpret_cast(callbackInfo.Data()); callbackInfo.SetData(callbackData->data); - T* instance = Unwrap(callbackInfo.This()); + T* instance = Unwrap(callbackInfo.This().As()); auto cb = callbackData->setterCallback; (instance->*cb)(callbackInfo, callbackInfo[0]); return nullptr; @@ -2606,9 +2584,13 @@ inline Value EscapableHandleScope::Escape(napi_value escapee) { //////////////////////////////////////////////////////////////////////////////// inline AsyncWorker::AsyncWorker(const Function& callback) - : _callback(Napi::Persistent(callback)), - _persistent(Napi::Persistent(Object::New(callback.Env()))), - _env(callback.Env()) { + : AsyncWorker(Object::New(callback.Env()), callback) { +} + +inline AsyncWorker::AsyncWorker(const Object& receiver, const Function& callback) + : _env(callback.Env()), + _receiver(Napi::Persistent(receiver)), + _callback(Napi::Persistent(callback)) { napi_status status = napi_create_async_work( _env, OnExecute, OnWorkComplete, this, &_work); if (status != napi_ok) throw Error::New(_env); @@ -2626,7 +2608,8 @@ inline AsyncWorker::AsyncWorker(AsyncWorker&& other) { other._env = nullptr; _work = other._work; other._work = nullptr; - _persistent = std::move(other._persistent); + _receiver = std::move(other._receiver); + _callback = std::move(other._callback); _error = std::move(other._error); } @@ -2635,7 +2618,8 @@ inline AsyncWorker& AsyncWorker::operator =(AsyncWorker&& other) { other._env = nullptr; _work = other._work; other._work = nullptr; - _persistent = std::move(other._persistent); + _receiver = std::move(other._receiver); + _callback = std::move(other._callback); _error = std::move(other._error); return *this; } @@ -2658,30 +2642,23 @@ inline void AsyncWorker::Cancel() { if (status != napi_ok) throw Error::New(_env); } -inline void AsyncWorker::WorkComplete() { - HandleScope scope(_env); - if (_error.IsEmpty()) { - OnOK(); - } - else { - OnError(_error); - } +inline ObjectReference& AsyncWorker::Receiver() { + return _receiver; } -inline ObjectReference& AsyncWorker::Persistent() { - return _persistent; +inline FunctionReference& AsyncWorker::Callback() { + return _callback; } inline void AsyncWorker::OnOK() { - _callback.MakeCallback(Env().Undefined(), std::vector()); + _callback.MakeCallback(_receiver.Value(), {}); } -inline void AsyncWorker::OnError(Error e) { - HandleScope scope(Env()); - _callback.MakeCallback(Env().Undefined(), std::vector({ e.Value() })); +inline void AsyncWorker::OnError(Error& e) { + _callback.MakeCallback(_receiver.Value(), { e.Value() }); } -inline void AsyncWorker::SetError(Error error) { +inline void AsyncWorker::SetError(const std::string& error) { _error = error; } @@ -2689,9 +2666,8 @@ inline void AsyncWorker::OnExecute(napi_env env, void* this_pointer) { AsyncWorker* self = static_cast(this_pointer); try { self->Execute(); - } - catch (const Error& e) { - self->SetError(e); + } catch (const std::exception& e) { + self->SetError(e.what()); } } @@ -2699,7 +2675,17 @@ inline void AsyncWorker::OnWorkComplete( napi_env env, napi_status status, void* this_pointer) { AsyncWorker* self = static_cast(this_pointer); if (status != napi_cancelled) { - self->WorkComplete(); + HandleScope scope(self->_env); + try { + if (self->_error.size() == 0) { + self->OnOK(); + } + else { + self->OnError(Error::New(self->_env, self->_error)); + } + } catch (const Error& e) { + e.ThrowAsJavaScriptException(); + } } delete self; } diff --git a/napi.h b/napi.h index 79af620db..d4bce0e17 100644 --- a/napi.h +++ b/napi.h @@ -1,4 +1,4 @@ -#ifndef SRC_NAPI_H_ +#ifndef SRC_NAPI_H_ #define SRC_NAPI_H_ //////////////////////////////////////////////////////////////////////////////// @@ -401,8 +401,6 @@ namespace Napi { Value Call(napi_value recv, const std::initializer_list& args) const; Value Call(napi_value recv, const std::vector& args) const; - Value MakeCallback(const std::initializer_list& args) const; - Value MakeCallback(const std::vector& args) const; Value MakeCallback(napi_value recv, const std::initializer_list& args) const; Value MakeCallback(napi_value recv, const std::vector& args) const; @@ -548,8 +546,6 @@ namespace Napi { Napi::Value Call(napi_value recv, const std::initializer_list& args) const; Napi::Value Call(napi_value recv, const std::vector& args) const; - Napi::Value MakeCallback(const std::initializer_list& args) const; - Napi::Value MakeCallback(const std::vector& args) const; Napi::Value MakeCallback(napi_value recv, const std::initializer_list& args) const; Napi::Value MakeCallback(napi_value recv, const std::vector& args) const; @@ -966,21 +962,18 @@ namespace Napi { void Queue(); void Cancel(); - virtual void Execute() = 0; - virtual void WorkComplete(); - - ObjectReference& Persistent(); + ObjectReference& Receiver(); + FunctionReference& Callback(); protected: explicit AsyncWorker(const Function& callback); + explicit AsyncWorker(const Object& receiver, const Function& callback); + virtual void Execute() = 0; virtual void OnOK(); - virtual void OnError(Error e); + virtual void OnError(Error& e); - void SetError(Error error); - - FunctionReference _callback; - ObjectReference _persistent; + void SetError(const std::string& error); private: static void OnExecute(napi_env env, void* this_pointer); @@ -990,7 +983,9 @@ namespace Napi { napi_env _env; napi_async_work _work; - Error _error; + ObjectReference _receiver; + FunctionReference _callback; + std::string _error; }; } // namespace Napi diff --git a/test/asyncworker.cc b/test/asyncworker.cc new file mode 100644 index 000000000..0a3c7e211 --- /dev/null +++ b/test/asyncworker.cc @@ -0,0 +1,34 @@ +#include "napi.h" + +using namespace Napi; + +class TestWorker : public AsyncWorker { +public: + static void DoWork(const CallbackInfo& info) { + bool succeed = info[0].As(); + Function cb = info[1].As(); + Value data = info[2]; + + TestWorker* worker = new TestWorker(cb); + worker->Receiver().Set("data", data); + worker->_succeed = succeed; + worker->Queue(); + } + +protected: + void Execute() override { + if (!_succeed) { + SetError("test error"); + } + } + +private: + TestWorker(Function cb) : AsyncWorker(cb) {} + bool _succeed; +}; + +Object InitAsyncWorker(Env env) { + Object exports = Object::New(env); + exports["doWork"] = Function::New(env, TestWorker::DoWork); + return exports; +} diff --git a/test/asyncworker.js b/test/asyncworker.js new file mode 100644 index 000000000..e43dcc4d0 --- /dev/null +++ b/test/asyncworker.js @@ -0,0 +1,25 @@ +'use strict'; +const buildType = process.config.target_defaults.default_configuration; +const binding = require(`./build/${buildType}/binding.node`); +const assert = require('assert'); + +// Use setTimeout() when asserting after async callbacks because +// unhandled JS exceptions in async callbacks are currently ignored. +// See the TODO comment in AsyncWorker::OnWorkComplete(). + +binding.asyncworker.doWork(true, function (e) { + setTimeout(() => { + assert.strictEqual(typeof e, 'undefined'); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + }); +}, 'test data'); + +binding.asyncworker.doWork(false, function (e) { + setTimeout(() => { + assert.ok(e instanceof Error); + assert.strictEqual(e.message, 'test error'); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + }); +}, 'test data'); diff --git a/test/binding.cc b/test/binding.cc index d858e8d66..414719a62 100644 --- a/test/binding.cc +++ b/test/binding.cc @@ -3,6 +3,7 @@ using namespace Napi; Object InitArrayBuffer(Env env); +Object InitAsyncWorker(Env env); Object InitBuffer(Env env); Object InitError(Env env); Object InitExternal(Env env); @@ -12,6 +13,7 @@ Object InitObject(Env env); void Init(Env env, Object exports, Object module) { exports.Set("arraybuffer", InitArrayBuffer(env)); + exports.Set("asyncworker", InitAsyncWorker(env)); exports.Set("buffer", InitBuffer(env)); exports.Set("error", InitError(env)); exports.Set("external", InitExternal(env)); diff --git a/test/binding.gyp b/test/binding.gyp index a76e9e51d..38e9443a6 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -4,6 +4,7 @@ 'target_name': 'binding', 'sources': [ 'arraybuffer.cc', + 'asyncworker.cc', 'binding.cc', 'buffer.cc', 'error.cc', diff --git a/test/index.js b/test/index.js index 1ab257561..285d7ce41 100644 --- a/test/index.js +++ b/test/index.js @@ -6,6 +6,7 @@ if (typeof global.gc !== 'function') { let testModules = [ 'arraybuffer', + 'asyncworker', 'buffer', 'error', 'external',