Make Error a persistent reference

napi-inl.h

@@ -1381,6 +1381,8 @@ inline void Buffer<T>::EnsureInfo() const {
 ////////////////////////////////////////////////////////////////////////////////
 
 inline Error Error::New(napi_env env) {
+  HandleScope scope(env);
+
   napi_status status;
   napi_value error = nullptr;
   if (Napi::Env(env).IsExceptionPending()) {
@@ -1432,16 +1434,49 @@ inline Error Error::New(napi_env env, const std::string& message) {
   return Error::New<Error>(env, message.c_str(), message.size(), napi_create_error);
 }
 
-inline Error::Error() : Object(), _message(nullptr) {
+inline Error::Error() : ObjectReference(), _message(nullptr) {
 }
 
-inline Error::Error(napi_env env, napi_value value) : Object(env, value) {
+inline Error::Error(napi_env env, napi_value value) : ObjectReference(env, nullptr) {
+  if (value != nullptr) {
+    napi_status status = napi_create_reference(env, value, 1, &_ref);
+
+    // Avoid infinite recursion in the failure case.
+    // Don't try to construct & throw another Error instance.
+    assert(status == napi_ok);
+  }
+}
+
+inline Error::Error(Error&& other) : ObjectReference(std::move(other)) {
+}
+
+inline Error& Error::operator =(Error&& other) {
+  static_cast<Reference<Object>*>(this)->operator=(std::move(other));
+  return *this;
+}
+
+inline Error::Error(const Error& other) : Error(other.Env(), other.Value()) {
+}
+
+inline Error& Error::operator =(Error& other) {
+  Reset();
+
+  _env = other.Env();
+  HandleScope scope(_env);
+
+  napi_value value = other.Value();
+  if (value != nullptr) {
+    napi_status status = napi_create_reference(_env, value, 1, &_ref);
+    if (status != napi_ok) throw Error::New(_env);
+  }
+
+  return *this;
 }
 
 inline const std::string& Error::Message() const NAPI_NOEXCEPT {
   if (_message.size() == 0 && _env != nullptr) {
     try {
-      _message = (*this)["message"].As<String>();
+      _message = Get("message").As<String>();
     }
     catch (...) {
       // Catch all errors here, to include e.g. a std::bad_alloc from
@@ -1453,8 +1488,10 @@ inline const std::string& Error::Message() const NAPI_NOEXCEPT {
 }
 
 inline void Error::ThrowAsJavaScriptException() const {
-  if (_value != nullptr) {
-    napi_throw(_env, _value);
+  HandleScope scope(_env);
+  if (!IsEmpty()) {
+    napi_status status = napi_throw(_env, Value());
+    if (status != napi_ok) throw Error::New(_env);
   }
 }
 
@@ -1532,8 +1569,8 @@ inline Reference<T>::Reference() : _env(nullptr), _ref(nullptr), _suppressDestru
 }
 
 template <typename T>
-inline Reference<T>::Reference(napi_env env, napi_ref persistent)
-  : _env(env), _ref(persistent) {
+inline Reference<T>::Reference(napi_env env, napi_ref ref)
+  : _env(env), _ref(ref) {
 }
 
 template <typename T>
@@ -1557,6 +1594,7 @@ inline Reference<T>::Reference(Reference<T>&& other) {
 
 template <typename T>
 inline Reference<T>& Reference<T>::operator =(Reference<T>&& other) {
+  Reset();
   _env = other._env;
   _ref = other._ref;
   other._env = nullptr;
@@ -2626,7 +2664,7 @@ inline void AsyncWorker::WorkComplete() {
     OnOK();
   }
   else {
-    OnError(_error.Value());
+    OnError(_error);
   }
 }
 
@@ -2639,11 +2677,12 @@ inline void AsyncWorker::OnOK() {
 }
 
 inline void AsyncWorker::OnError(Error e) {
-  _callback.MakeCallback(Env().Undefined(), std::vector<napi_value>({ e }));
+  HandleScope scope(Env());
+  _callback.MakeCallback(Env().Undefined(), std::vector<napi_value>({ e.Value() }));
 }
 
 inline void AsyncWorker::SetError(Error error) {
-  _error.Reset(error, 1);
+  _error = error;
 }
 
 inline void AsyncWorker::OnExecute(napi_env env, void* this_pointer) {

napi.h

@@ -443,99 +443,6 @@ namespace Napi {
     void EnsureInfo() const;
   };
 
-  /*
-   * The NAPI Error class wraps a JavaScript Error object in a way that enables it
-   * to traverse a C++ stack and be thrown and caught as a C++ exception.
-   *
-   * If a NAPI API call fails without executing any JavaScript code (for example due
-   * to an invalid argument), then the NAPI wrapper automatically converts and throws
-   * the error as a C++ exception of type Napi::Error.
-   *
-   * If a JavaScript function called by C++ code via NAPI throws a JavaScript exception,
-   * then the NAPI wrapper automatically converts and throws it as a C++ exception of type
-   * Napi::Error.
-   *
-   * If a C++ exception of type Napi::Error escapes from a NAPI C++ callback, then
-   * the NAPI wrapper automatically converts and throws it as a JavaScript exception.
-   *
-   * Catching a C++ exception of type Napi::Error also clears the JavaScript exception.
-   * Of course it may be then re-thrown, which restores the JavaScript exception.
-   *
-   * Example 1 - Throwing an exception:
-   *
-   *   Napi::Env env = ...
-   *   throw env.NewError("Example exception");
-   *   // Following C++ statements will not be executed.
-   *   // The exception will bubble up as a C++ exception of type Napi::Error,
-   *   // until it is either caught while still in C++, or else automatically
-   *   // re-thrown as a JavaScript exception when the callback returns to JavaScript.
-   *
-   * Example 2 - Ignoring a NAPI exception:
-   *
-   *   Napi::Function jsFunctionThatThrows = someObj.AsFunction();
-   *   jsFunctionThatThrows({ arg1, arg2 });
-   *   // Following C++ statements will not be executed.
-   *   // The exception will bubble up as a C++ exception of type Napi::Error,
-   *   // until it is either caught while still in C++, or else automatically
-   *   // re-thrown as a JavaScript exception when the callback returns to JavaScript.
-   *
-   * Example 3 - Handling a NAPI exception:
-   *
-   *   Napi::Function jsFunctionThatThrows = someObj.AsFunction();
-   *   try {
-   *      jsFunctionThatThrows({ arg1, arg2 });
-   *   }
-   *   catch (const Napi::Error& e) {
-   *     cerr << "Caught JavaScript exception: " + e.what();
-   *     // Since the exception was caught here, it will not be re-thrown as
-   *     // a JavaScript exception.
-   *   }
-   */
-  class Error : public Object, public std::exception {
-  public:
-    static Error New(napi_env env);
-    static Error New(napi_env env, const char* message);
-    static Error New(napi_env env, const std::string& message);
-
-    Error();
-    Error(napi_env env, napi_value value);
-
-    const std::string& Message() const NAPI_NOEXCEPT;
-    void ThrowAsJavaScriptException() const;
-
-    const char* what() const NAPI_NOEXCEPT override;
-
-  protected:
-    typedef napi_status (*create_error_fn)(napi_env envb, napi_value msg, napi_value* result);
-
-    template <typename TError>
-    static TError New(napi_env env,
-                      const char* message,
-                      size_t length,
-                      create_error_fn create_error);
-
-  private:
-    mutable std::string _message;
-  };
-
-  class TypeError : public Error {
-  public:
-    static TypeError New(napi_env env, const char* message);
-    static TypeError New(napi_env env, const std::string& message);
-
-    TypeError();
-    TypeError(napi_env env, napi_value value);
-  };
-
-  class RangeError : public Error {
-  public:
-    static RangeError New(napi_env env, const char* message);
-    static RangeError New(napi_env env, const std::string& message);
-
-    RangeError();
-    RangeError(napi_env env, napi_value value);
-  };
-
   /*
    * Holds a counted reference to a value; initially a weak reference unless otherwise specified.
    * May be changed to/from a strong reference by adjusting the refcount. The referenced value
@@ -660,6 +567,104 @@ namespace Napi {
   ObjectReference Persistent(Object value);
   FunctionReference Persistent(Function value);
 
+  /*
+   * The NAPI Error class wraps a JavaScript Error object in a way that enables it
+   * to traverse a C++ stack and be thrown and caught as a C++ exception.
+   *
+   * If a NAPI API call fails without executing any JavaScript code (for example due
+   * to an invalid argument), then the NAPI wrapper automatically converts and throws
+   * the error as a C++ exception of type Napi::Error.
+   *
+   * If a JavaScript function called by C++ code via NAPI throws a JavaScript exception,
+   * then the NAPI wrapper automatically converts and throws it as a C++ exception of type
+   * Napi::Error.
+   *
+   * If a C++ exception of type Napi::Error escapes from a NAPI C++ callback, then
+   * the NAPI wrapper automatically converts and throws it as a JavaScript exception.
+   *
+   * Catching a C++ exception of type Napi::Error also clears the JavaScript exception.
+   * Of course it may be then re-thrown, which restores the JavaScript exception.
+   *
+   * Example 1 - Throwing a N-API exception:
+   *
+   *   Napi::Env env = ...
+   *   throw Napi::Error::New(env, "Example exception");
+   *   // Following C++ statements will not be executed.
+   *   // The exception will bubble up as a C++ exception of type Napi::Error,
+   *   // until it is either caught while still in C++, or else automatically
+   *   // re-thrown as a JavaScript exception when the callback returns to JavaScript.
+   *
+   * Example 2 - Ignoring a N-API exception:
+   *
+   *   Napi::Function jsFunctionThatThrows = someObj.As<Napi::Function>();
+   *   jsFunctionThatThrows({ arg1, arg2 });
+   *   // Following C++ statements will not be executed.
+   *   // The exception will bubble up as a C++ exception of type Napi::Error,
+   *   // until it is either caught while still in C++, or else automatically
+   *   // re-thrown as a JavaScript exception when the callback returns to JavaScript.
+   *
+   * Example 3 - Handling a N-API exception:
+   *
+   *   Napi::Function jsFunctionThatThrows = someObj.As<Napi::Function>();
+   *   try {
+   *     jsFunctionThatThrows({ arg1, arg2 });
+   *   } catch (const Napi::Error& e) {
+   *     cerr << "Caught JavaScript exception: " + e.Message();
+   *     // Since the exception was caught here, it will not be re-thrown as
+   *     // a JavaScript exception.
+   *   }
+   */
+  class Error : public ObjectReference, public std::exception {
+  public:
+    static Error New(napi_env env);
+    static Error New(napi_env env, const char* message);
+    static Error New(napi_env env, const std::string& message);
+
+    Error();
+    Error(napi_env env, napi_value value);
+
+    // An error can be moved or copied.
+    Error(Error&& other);
+    Error& operator =(Error&& other);
+    Error(const Error&);
+    Error& operator =(Error&);
+
+    const std::string& Message() const NAPI_NOEXCEPT;
+    void ThrowAsJavaScriptException() const;
+
+    const char* what() const NAPI_NOEXCEPT override;
+
+  protected:
+    typedef napi_status (*create_error_fn)(napi_env envb, napi_value msg, napi_value* result);
+
+    template <typename TError>
+    static TError New(napi_env env,
+                      const char* message,
+                      size_t length,
+                      create_error_fn create_error);
+
+  private:
+    mutable std::string _message;
+  };
+
+  class TypeError : public Error {
+  public:
+    static TypeError New(napi_env env, const char* message);
+    static TypeError New(napi_env env, const std::string& message);
+
+    TypeError();
+    TypeError(napi_env env, napi_value value);
+  };
+
+  class RangeError : public Error {
+  public:
+    static RangeError New(napi_env env, const char* message);
+    static RangeError New(napi_env env, const std::string& message);
+
+    RangeError();
+    RangeError(napi_env env, napi_value value);
+  };
+
   class CallbackInfo {
   public:
     CallbackInfo(napi_env env, napi_callback_info info);
@@ -985,7 +990,7 @@ namespace Napi {
 
     napi_env _env;
     napi_async_work _work;
-    Reference<Error> _error;
+    Error _error;
   };
 
 } // namespace Napi

test/error.cc

@@ -24,7 +24,7 @@ Value CatchError(const CallbackInfo& info) {
   try {
     thrower({});
   } catch (const Error& e) {
-     return e;
+     return e.Value();
   }
   return info.Env().Null();
 }
@@ -50,11 +50,28 @@ void CatchAndRethrowError(const CallbackInfo& info) {
   try {
     thrower({});
   } catch (Error& e) {
-     e["caught"] = Boolean::New(info.Env(), true);
+     e.Set("caught", Boolean::New(info.Env(), true));
      throw e;
   }
 }
 
+void ThrowErrorThatEscapesScope(const CallbackInfo& info) {
+  HandleScope scope(info.Env());
+
+  std::string message = info[0].As<String>().Utf8Value();
+  throw Error::New(info.Env(), message);
+}
+
+void CatchAndRethrowErrorThatEscapesScope(const CallbackInfo& info) {
+  HandleScope scope(info.Env());
+  try {
+    ThrowErrorThatEscapesScope(info);
+  } catch (Error& e) {
+    e.Set("caught", Boolean::New(info.Env(), true));
+    throw e;
+  }
+}
+
 } // end anonymous namespace
 
 Object InitError(Env env) {
@@ -66,5 +83,8 @@ Object InitError(Env env) {
   exports["catchErrorMessage"] = Function::New(env, CatchErrorMessage);
   exports["doNotCatch"] = Function::New(env, DoNotCatch);
   exports["catchAndRethrowError"] = Function::New(env, CatchAndRethrowError);
+  exports["throwErrorThatEscapesScope"] = Function::New(env, ThrowErrorThatEscapesScope);
+  exports["catchAndRethrowErrorThatEscapesScope"] =
+    Function::New(env, CatchAndRethrowErrorThatEscapesScope);
   return exports;
 }

test/error.js

@@ -41,3 +41,11 @@ assert.strictEqual(err.message, 'test');
 const msg = binding.error.catchErrorMessage(
    () => { throw new TypeError('test'); });
 assert.strictEqual(msg, 'test');
+
+assert.throws(() => binding.error.throwErrorThatEscapesScope('test'), err => {
+   return err instanceof Error && err.message === 'test';
+});
+
+assert.throws(() => binding.error.catchAndRethrowErrorThatEscapesScope('test'), err => {
+  return err instanceof Error && err.message === 'test' && err.caught;
+});