Add support for Error::Fatal

* Added static method `Error::Fatal` to invoke `napi_fatal_error`
* Added `node_internals.cc/h` to shim missing internal functions
* Added a test for `Error::Fatal`
* Replaced usage of assert with calls to `Error::Fatal`

Author: kfarnung

napi-inl.h

@@ -9,7 +9,6 @@
 
 // Note: Do not include this file directly! Include "napi.h" instead.
 
-#include <cassert>
 #include <cstring>
 
 namespace Napi {
@@ -42,6 +41,13 @@ namespace details {
 
 #endif // NAPI_CPP_EXCEPTIONS
 
+#define NAPI_FATAL_IF_FAILED(status, location, message)  \
+  do {                                                   \
+    if ((status) != napi_ok) {                           \
+      Error::Fatal((location), (message));               \
+    }                                                    \
+  } while (0)
+
 // For use in JS to C++ callback wrappers to catch any Napi::Error exceptions
 // and rethrow them as JavaScript exceptions before returning from the callback.
 template <typename Callable>
@@ -1418,24 +1424,24 @@ inline Error Error::New(napi_env env) {
 
   const napi_extended_error_info* info;
   status = napi_get_last_error_info(env, &info);
-  assert(status == napi_ok);
+  NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_get_last_error_info");
 
   if (status == napi_ok) {
     if (info->error_code == napi_pending_exception) {
       status = napi_get_and_clear_last_exception(env, &error);
-      assert(status == napi_ok);
+      NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_get_and_clear_last_exception");
     }
     else {
       const char* error_message = info->error_message != nullptr ?
         info->error_message : "Error in native callback";
 
       bool isExceptionPending;
       status = napi_is_exception_pending(env, &isExceptionPending);
-      assert(status == napi_ok);
+      NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_is_exception_pending");
 
       if (isExceptionPending) {
         status = napi_get_and_clear_last_exception(env, &error);
-        assert(status == napi_ok);
+        NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_get_and_clear_last_exception");
       }
 
       napi_value message;
@@ -1444,7 +1450,7 @@ inline Error Error::New(napi_env env) {
         error_message,
         std::strlen(error_message),
         &message);
-      assert(status == napi_ok);
+      NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_create_string_utf8");
 
       if (status == napi_ok) {
         switch (info->error_code) {
@@ -1458,7 +1464,7 @@ inline Error Error::New(napi_env env) {
           status = napi_create_error(env, message, &error);
           break;
         }
-        assert(status == napi_ok);
+        NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_create_error");
       }
     }
   }
@@ -1474,6 +1480,10 @@ 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 NAPI_NO_RETURN void Error::Fatal(const char* location, const char* message) {
+  napi_fatal_error(location, message);
+}
+
 inline Error::Error() : ObjectReference(), _message(nullptr) {
 }
 
@@ -1483,7 +1493,7 @@ inline Error::Error(napi_env env, napi_value value) : ObjectReference(env, nullp
 
     // Avoid infinite recursion in the failure case.
     // Don't try to construct & throw another Error instance.
-    assert(status == napi_ok);
+    NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_create_reference");
   }
 }
 
@@ -1661,9 +1671,7 @@ inline Reference<T>::Reference(const Reference<T>& other)
     // Copying is a limited scenario (currently only used for Error object) and always creates a
     // strong reference to the given value even if the incoming reference is weak.
     napi_status status = napi_create_reference(_env, value, 1, &_ref);
-
-    // TODO - Switch to napi_fatal_error() once it exists.
-    assert(status == napi_ok);
+    NAPI_FATAL_IF_FAILED(status, "Reference<T>::Reference", "napi_create_reference");
   }
 }
 

napi.h

@@ -1067,6 +1067,8 @@ namespace Napi {
     static Error New(napi_env env, const char* message);
     static Error New(napi_env env, const std::string& message);
 
+    static NAPI_NO_RETURN void Fatal(const char* location, const char* message);
+
     Error();
     Error(napi_env env, napi_value value);
 

src/node_api.cc

@@ -17,6 +17,9 @@
 #include <vector>
 #include "uv.h"
 #include "node_api.h"
+#include "node_internals.h"
+
+#define NAPI_VERSION  1
 
 static
 napi_status napi_set_last_error(napi_env env, napi_status error_code,
@@ -154,14 +157,20 @@ class HandleScopeWrapper {
 // across different versions.
 class EscapableHandleScopeWrapper {
  public:
-  explicit EscapableHandleScopeWrapper(v8::Isolate* isolate) : scope(isolate) {}
+  explicit EscapableHandleScopeWrapper(v8::Isolate* isolate)
+      : scope(isolate), escape_called_(false) {}
+  bool escape_called() const {
+    return escape_called_;
+  }
   template <typename T>
   v8::Local<T> Escape(v8::Local<T> handle) {
+    escape_called_ = true;
     return scope.Escape(handle);
   }
 
  private:
   v8::EscapableHandleScope scope;
+  bool escape_called_;
 };
 
 napi_handle_scope JsHandleScopeFromV8HandleScope(HandleScopeWrapper* s) {
@@ -716,7 +725,8 @@ const char* error_messages[] = {nullptr,
                                 "An array was expected",
                                 "Unknown failure",
                                 "An exception is pending",
-                                "The async work item was cancelled"};
+                                "The async work item was cancelled",
+                                "napi_escape_handle already called on scope"};
 
 static napi_status napi_clear_last_error(napi_env env) {
   CHECK_ENV(env);
@@ -744,10 +754,14 @@ napi_status napi_get_last_error_info(napi_env env,
   CHECK_ENV(env);
   CHECK_ARG(env, result);
 
+  // you must update this assert to reference the last message
+  // in the napi_status enum each time a new error message is added.
+  // We don't have a napi_status_last as this would result in an ABI
+  // change each time a message was added.
   static_assert(
-      (sizeof (error_messages) / sizeof (*error_messages)) == napi_status_last,
+      node::arraysize(error_messages) == napi_escape_called_twice + 1,
       "Count of error messages must match count of error values");
-  assert(env->last_error.error_code < napi_status_last);
+  assert(env->last_error.error_code <= napi_escape_called_twice);
 
   // Wait until someone requests the last error information to fetch the error
   // message string
@@ -758,6 +772,11 @@ napi_status napi_get_last_error_info(napi_env env,
   return napi_ok;
 }
 
+NAPI_NO_RETURN void napi_fatal_error(const char* location,
+                                     const char* message) {
+  node::FatalError(location, message);
+}
+
 napi_status napi_create_function(napi_env env,
                                  const char* utf8name,
                                  napi_callback cb,
@@ -817,9 +836,6 @@ napi_status napi_define_class(napi_env env,
   v8::Local<v8::FunctionTemplate> tpl = v8::FunctionTemplate::New(
       isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata);
 
-  // we need an internal field to stash the wrapped object
-  tpl->InstanceTemplate()->SetInternalFieldCount(1);
-
   v8::Local<v8::String> name_string;
   CHECK_NEW_FROM_UTF8(env, name_string, utf8name);
   tpl->SetClassName(name_string);
@@ -991,6 +1007,28 @@ napi_status napi_get_property(napi_env env,
   return GET_RETURN_STATUS(env);
 }
 
+napi_status napi_delete_property(napi_env env,
+                                 napi_value object,
+                                 napi_value key,
+                                 bool* result) {
+  NAPI_PREAMBLE(env);
+  CHECK_ARG(env, key);
+
+  v8::Isolate* isolate = env->isolate;
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+  v8::Local<v8::Value> k = v8impl::V8LocalValueFromJsValue(key);
+  v8::Local<v8::Object> obj;
+
+  CHECK_TO_OBJECT(env, context, obj, object);
+  v8::Maybe<bool> delete_maybe = obj->Delete(context, k);
+  CHECK_MAYBE_NOTHING(env, delete_maybe, napi_generic_failure);
+
+  if (result != NULL)
+    *result = delete_maybe.FromMaybe(false);
+
+  return GET_RETURN_STATUS(env);
+}
+
 napi_status napi_set_named_property(napi_env env,
                                     napi_value object,
                                     const char* utf8name,
@@ -1128,6 +1166,26 @@ napi_status napi_get_element(napi_env env,
   return GET_RETURN_STATUS(env);
 }
 
+napi_status napi_delete_element(napi_env env,
+                                napi_value object,
+                                uint32_t index,
+                                bool* result) {
+  NAPI_PREAMBLE(env);
+
+  v8::Isolate* isolate = env->isolate;
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+  v8::Local<v8::Object> obj;
+
+  CHECK_TO_OBJECT(env, context, obj, object);
+  v8::Maybe<bool> delete_maybe = obj->Delete(context, index);
+  CHECK_MAYBE_NOTHING(env, delete_maybe, napi_generic_failure);
+
+  if (result != NULL)
+    *result = delete_maybe.FromMaybe(false);
+
+  return GET_RETURN_STATUS(env);
+}
+
 napi_status napi_define_properties(napi_env env,
                                    napi_value object,
                                    size_t property_count,
@@ -1948,14 +2006,24 @@ napi_status napi_wrap(napi_env env,
   CHECK_ARG(env, js_object);
 
   v8::Isolate* isolate = env->isolate;
-  v8::Local<v8::Object> obj =
-      v8impl::V8LocalValueFromJsValue(js_object).As<v8::Object>();
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+
+  v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(js_object);
+  RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
+  v8::Local<v8::Object> obj = value.As<v8::Object>();
 
-  // Only objects that were created from a NAPI constructor's prototype
-  // via napi_define_class() can be (un)wrapped.
-  RETURN_STATUS_IF_FALSE(env, obj->InternalFieldCount() > 0, napi_invalid_arg);
+  // Create a wrapper object with an internal field to hold the wrapped pointer.
+  v8::Local<v8::ObjectTemplate> wrapperTemplate =
+      v8::ObjectTemplate::New(isolate);
+  wrapperTemplate->SetInternalFieldCount(1);
+  v8::Local<v8::Object> wrapper =
+      wrapperTemplate->NewInstance(context).ToLocalChecked();
+  wrapper->SetInternalField(0, v8::External::New(isolate, native_object));
 
-  obj->SetInternalField(0, v8::External::New(isolate, native_object));
+  // Insert the wrapper into the object's prototype chain.
+  v8::Local<v8::Value> proto = obj->GetPrototype();
+  CHECK(wrapper->SetPrototype(context, proto).FromJust());
+  CHECK(obj->SetPrototype(context, wrapper).FromJust());
 
   if (result != nullptr) {
     // The returned reference should be deleted via napi_delete_reference()
@@ -1986,11 +2054,18 @@ napi_status napi_unwrap(napi_env env, napi_value js_object, void** result) {
   RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
   v8::Local<v8::Object> obj = value.As<v8::Object>();
 
-  // Only objects that were created from a NAPI constructor's prototype
-  // via napi_define_class() can be (un)wrapped.
-  RETURN_STATUS_IF_FALSE(env, obj->InternalFieldCount() > 0, napi_invalid_arg);
-
-  v8::Local<v8::Value> unwrappedValue = obj->GetInternalField(0);
+  // 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();
@@ -2195,9 +2270,12 @@ napi_status napi_escape_handle(napi_env env,
 
   v8impl::EscapableHandleScopeWrapper* s =
       v8impl::V8EscapableHandleScopeFromJsEscapableHandleScope(scope);
-  *result = v8impl::JsValueFromV8LocalValue(
-      s->Escape(v8impl::V8LocalValueFromJsValue(escapee)));
-  return napi_clear_last_error(env);
+  if (!s->escape_called()) {
+    *result = v8impl::JsValueFromV8LocalValue(
+        s->Escape(v8impl::V8LocalValueFromJsValue(escapee)));
+    return napi_clear_last_error(env);
+  }
+  return napi_set_last_error(env, napi_escape_called_twice);
 }
 
 napi_status napi_new_instance(napi_env env,
@@ -2250,7 +2328,7 @@ napi_status napi_instanceof(napi_env env,
   }
 
   if (env->has_instance_available) {
-    napi_value value, js_result, has_instance = nullptr;
+    napi_value value, js_result = nullptr, has_instance = nullptr;
     napi_status status = napi_generic_failure;
     napi_valuetype value_type;
 
@@ -2530,7 +2608,7 @@ napi_status napi_create_arraybuffer(napi_env env,
       v8::ArrayBuffer::New(isolate, byte_length);
 
   // Optionally return a pointer to the buffer's data, to avoid another call to
-  // retreive it.
+  // retrieve it.
   if (data != nullptr) {
     *data = buffer->GetContents().Data();
   }
@@ -2713,6 +2791,13 @@ napi_status napi_get_typedarray_info(napi_env env,
   return napi_clear_last_error(env);
 }
 
+napi_status napi_get_version(napi_env env, uint32_t* result) {
+  CHECK_ENV(env);
+  CHECK_ARG(env, result);
+  *result = NAPI_VERSION;
+  return napi_clear_last_error(env);
+}
+
 namespace uvimpl {
 
 static napi_status ConvertUVErrorCode(int code) {
@@ -2781,7 +2866,7 @@ class Work {
       // report it as a fatal exception. (There is no JavaScript on the
       // callstack that can possibly handle it.)
       if (!env->last_exception.IsEmpty()) {
-        v8::TryCatch try_catch;
+        v8::TryCatch try_catch(env->isolate);
         env->isolate->ThrowException(
           v8::Local<v8::Value>::New(env->isolate, env->last_exception));
         node::FatalException(env->isolate, try_catch);

src/node_api.gyp

@@ -8,6 +8,7 @@
       'type': 'static_library',
       'sources': [
         'node_api.cc',
+        'node_internals.cc',
       ],
       'defines': [
          'EXTERNAL_NAPI',