n-api: tighten null-checking and clean up last error

We left out null-checks for many of the parameters passed to our APIs.
In particular, arguments of type `napi_value` were often accepted
without a null-check, even though they should never be null.

Additionally, many APIs simply returned `napi_ok` on success. This
leaves in place an error that may have occurred in a previous N-API
call. Others (those which perform `NAPI_PREAMBLE(env)` at the top)
OTOH explicitly clear the last error before proceeding. With this
modification all APIs explicitly clear the last error on success.

Fixes: https://github.com/nodejs/abi-stable-node/issues/227
PR-URL: https://github.com/nodejs/node/pull/12539
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

1 files changed, 112 insertions, 46 deletions

diff --git a/src/node_api.cc b/src/node_api.cc
index 49b7bcedca..5fdd879d6f 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -22,7 +22,7 @@ napi_status napi_set_last_error(napi_env env, napi_status error_code,
                                 uint32_t engine_error_code = 0,
                                 void* engine_reserved = nullptr);
 static
-void napi_clear_last_error(napi_env env);
+napi_status napi_clear_last_error(napi_env env);
 
 struct napi_env__ {
   explicit napi_env__(v8::Isolate* _isolate): isolate(_isolate),
@@ -69,11 +69,20 @@ struct napi_env__ {
 
 #define CHECK_TO_TYPE(env, type, context, result, src, status)                \
   do {                                                                        \
+    CHECK_ARG((env), (src));                                                  \
     auto maybe = v8impl::V8LocalValueFromJsValue((src))->To##type((context)); \
     CHECK_MAYBE_EMPTY((env), maybe, (status));                                \
     (result) = maybe.ToLocalChecked();                                        \
   } while (0)
 
+#define CHECK_TO_FUNCTION(env, result, src)                                 \
+  do {                                                                      \
+    CHECK_ARG((env), (src));                                                \
+    v8::Local<v8::Value> v8value = v8impl::V8LocalValueFromJsValue((src));  \
+    RETURN_STATUS_IF_FALSE((env), v8value->IsFunction(), napi_invalid_arg); \
+    (result) = v8value.As<v8::Function>();                                  \
+  } while (0)
+
 #define CHECK_TO_OBJECT(env, context, result, src) \
   CHECK_TO_TYPE((env), Object, (context), (result), (src), napi_object_expected)
 
@@ -709,17 +718,20 @@ const char* error_messages[] = {nullptr,
                                 "An exception is pending",
                                 "The async work item was cancelled"};
 
-void napi_clear_last_error(napi_env env) {
+static napi_status napi_clear_last_error(napi_env env) {
+  CHECK_ENV(env);
+
   env->last_error.error_code = napi_ok;
 
   // TODO(boingoing): Should this be a callback?
   env->last_error.engine_error_code = 0;
   env->last_error.engine_reserved = nullptr;
+  return napi_ok;
 }
 
-napi_status napi_set_last_error(napi_env env, napi_status error_code,
-                                uint32_t engine_error_code,
-                                void* engine_reserved) {
+static napi_status napi_set_last_error(napi_env env, napi_status error_code,
+                                       uint32_t engine_error_code,
+                                       void* engine_reserved) {
   env->last_error.error_code = error_code;
   env->last_error.engine_error_code = engine_error_code;
   env->last_error.engine_reserved = engine_reserved;
@@ -730,6 +742,7 @@ napi_status napi_set_last_error(napi_env env, napi_status error_code,
 napi_status napi_get_last_error_info(napi_env env,
                                      const napi_extended_error_info** result) {
   CHECK_ENV(env);
+  CHECK_ARG(env, result);
 
   static_assert(node::arraysize(error_messages) == napi_status_last,
       "Count of error messages must match count of error values");
@@ -741,7 +754,7 @@ napi_status napi_get_last_error_info(napi_env env,
       error_messages[env->last_error.error_code];
 
   *result = &(env->last_error);
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_create_function(napi_env env,
@@ -751,6 +764,7 @@ napi_status napi_create_function(napi_env env,
                                  napi_value* result) {
   NAPI_PREAMBLE(env);
   CHECK_ARG(env, result);
+  CHECK_ARG(env, cb);
 
   v8::Isolate* isolate = env->isolate;
   v8::Local<v8::Function> return_value;
@@ -789,6 +803,7 @@ napi_status napi_define_class(napi_env env,
                               napi_value* result) {
   NAPI_PREAMBLE(env);
   CHECK_ARG(env, result);
+  CHECK_ARG(env, constructor);
 
   v8::Isolate* isolate = env->isolate;
 
@@ -910,6 +925,8 @@ napi_status napi_set_property(napi_env env,
                               napi_value key,
                               napi_value value) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, key);
+  CHECK_ARG(env, value);
 
   v8::Isolate* isolate = env->isolate;
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
@@ -932,6 +949,7 @@ napi_status napi_has_property(napi_env env,
                               bool* result) {
   NAPI_PREAMBLE(env);
   CHECK_ARG(env, result);
+  CHECK_ARG(env, key);
 
   v8::Isolate* isolate = env->isolate;
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
@@ -953,6 +971,7 @@ napi_status napi_get_property(napi_env env,
                               napi_value key,
                               napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, key);
   CHECK_ARG(env, result);
 
   v8::Isolate* isolate = env->isolate;
@@ -976,6 +995,7 @@ napi_status napi_set_named_property(napi_env env,
                                     const char* utf8name,
                                     napi_value value) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
 
   v8::Isolate* isolate = env->isolate;
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
@@ -1049,6 +1069,7 @@ napi_status napi_set_element(napi_env env,
                              uint32_t index,
                              napi_value value) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
 
   v8::Isolate* isolate = env->isolate;
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
@@ -1111,11 +1132,15 @@ napi_status napi_define_properties(napi_env env,
                                    size_t property_count,
                                    const napi_property_descriptor* properties) {
   NAPI_PREAMBLE(env);
+  if (property_count > 0) {
+    CHECK_ARG(env, properties);
+  }
 
   v8::Isolate* isolate = env->isolate;
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
-  v8::Local<v8::Object> obj =
-      v8impl::V8LocalValueFromJsValue(object).As<v8::Object>();
+
+  v8::Local<v8::Object> obj;
+  CHECK_TO_OBJECT(env, context, obj, object);
 
   for (size_t i = 0; i < property_count; i++) {
     const napi_property_descriptor* p = &properties[i];
@@ -1182,6 +1207,7 @@ napi_status napi_define_properties(napi_env env,
 
 napi_status napi_is_array(napi_env env, napi_value value, bool* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
@@ -1194,6 +1220,7 @@ napi_status napi_get_array_length(napi_env env,
                                   napi_value value,
                                   uint32_t* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
@@ -1210,6 +1237,8 @@ napi_status napi_strict_equals(napi_env env,
                                napi_value rhs,
                                bool* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, lhs);
+  CHECK_ARG(env, rhs);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> a = v8impl::V8LocalValueFromJsValue(lhs);
@@ -1344,7 +1373,7 @@ napi_status napi_get_boolean(napi_env env, bool value, napi_value* result) {
     *result = v8impl::JsValueFromV8LocalValue(v8::False(isolate));
   }
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_create_symbol(napi_env env,
@@ -1372,6 +1401,7 @@ napi_status napi_create_error(napi_env env,
                               napi_value msg,
                               napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, msg);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> message_value = v8impl::V8LocalValueFromJsValue(msg);
@@ -1387,6 +1417,7 @@ napi_status napi_create_type_error(napi_env env,
                                    napi_value msg,
                                    napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, msg);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> message_value = v8impl::V8LocalValueFromJsValue(msg);
@@ -1402,6 +1433,7 @@ napi_status napi_create_range_error(napi_env env,
                                     napi_value msg,
                                     napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, msg);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> message_value = v8impl::V8LocalValueFromJsValue(msg);
@@ -1419,6 +1451,7 @@ napi_status napi_typeof(napi_env env,
   // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
   // JS exceptions.
   CHECK_ENV(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> v = v8impl::V8LocalValueFromJsValue(value);
@@ -1448,7 +1481,7 @@ napi_status napi_typeof(napi_env env,
     return napi_set_last_error(env, napi_invalid_arg);
   }
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_get_undefined(napi_env env, napi_value* result) {
@@ -1458,7 +1491,7 @@ napi_status napi_get_undefined(napi_env env, napi_value* result) {
   *result = v8impl::JsValueFromV8LocalValue(
       v8::Undefined(env->isolate));
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_get_null(napi_env env, napi_value* result) {
@@ -1468,7 +1501,7 @@ napi_status napi_get_null(napi_env env, napi_value* result) {
   *result = v8impl::JsValueFromV8LocalValue(
         v8::Null(env->isolate));
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 // Gets all callback info in a single call. (Ugly, but faster.)
@@ -1481,6 +1514,7 @@ napi_status napi_get_cb_info(
     napi_value* this_arg,  // [out] Receives the JS 'this' arg for the call
     void** data) {         // [out] Receives the data pointer for the callback.
   CHECK_ENV(env);
+  CHECK_ARG(env, cbinfo);
 
   v8impl::CallbackWrapper* info =
       reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);
@@ -1499,7 +1533,7 @@ napi_status napi_get_cb_info(
     *data = info->Data();
   }
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_is_construct_call(napi_env env,
@@ -1507,13 +1541,14 @@ napi_status napi_is_construct_call(napi_env env,
                                    bool* result) {
   // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because no V8 APIs are called.
   CHECK_ENV(env);
+  CHECK_ARG(env, cbinfo);
   CHECK_ARG(env, result);
 
   v8impl::CallbackWrapper* info =
       reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);
 
   *result = info->IsConstructCall();
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_call_function(napi_env env,
@@ -1523,16 +1558,19 @@ napi_status napi_call_function(napi_env env,
                                const napi_value* argv,
                                napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, recv);
+  if (argc > 0) {
+    CHECK_ARG(env, argv);
+  }
 
   v8::Isolate* isolate = env->isolate;
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
 
   v8::Local<v8::Value> v8recv = v8impl::V8LocalValueFromJsValue(recv);
 
-  v8::Local<v8::Value> v8value = v8impl::V8LocalValueFromJsValue(func);
-  RETURN_STATUS_IF_FALSE(env, v8value->IsFunction(), napi_invalid_arg);
+  v8::Local<v8::Function> v8func;
+  CHECK_TO_FUNCTION(env, v8func, func);
 
-  v8::Local<v8::Function> v8func = v8value.As<v8::Function>();
   auto maybe = v8func->Call(context, v8recv, argc,
     reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)));
 
@@ -1543,7 +1581,7 @@ napi_status napi_call_function(napi_env env,
       CHECK_MAYBE_EMPTY(env, maybe, napi_generic_failure);
       *result = v8impl::JsValueFromV8LocalValue(maybe.ToLocalChecked());
     }
-    return napi_ok;
+    return napi_clear_last_error(env);
   }
 }
 
@@ -1558,18 +1596,19 @@ napi_status napi_get_global(napi_env env, napi_value* result) {
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
   *result = v8impl::JsValueFromV8LocalValue(context->Global());
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_throw(napi_env env, napi_value error) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, error);
 
   v8::Isolate* isolate = env->isolate;
 
   isolate->ThrowException(v8impl::V8LocalValueFromJsValue(error));
   // any VM calls after this point and before returning
   // to the javascript invoker will fail
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_throw_error(napi_env env, const char* msg) {
@@ -1582,7 +1621,7 @@ napi_status napi_throw_error(napi_env env, const char* msg) {
   isolate->ThrowException(v8::Exception::Error(str));
   // any VM calls after this point and before returning
   // to the javascript invoker will fail
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_throw_type_error(napi_env env, const char* msg) {
@@ -1595,7 +1634,7 @@ napi_status napi_throw_type_error(napi_env env, const char* msg) {
   isolate->ThrowException(v8::Exception::TypeError(str));
   // any VM calls after this point and before returning
   // to the javascript invoker will fail
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_throw_range_error(napi_env env, const char* msg) {
@@ -1608,19 +1647,20 @@ napi_status napi_throw_range_error(napi_env env, const char* msg) {
   isolate->ThrowException(v8::Exception::RangeError(str));
   // any VM calls after this point and before returning
   // to the javascript invoker will fail
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_is_error(napi_env env, napi_value value, bool* result) {
   // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot
   // throw JS exceptions.
   CHECK_ENV(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
   *result = val->IsNativeError();
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_get_value_double(napi_env env,
@@ -1637,7 +1677,7 @@ napi_status napi_get_value_double(napi_env env,
 
   *result = val.As<v8::Number>()->Value();
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_get_value_int32(napi_env env,
@@ -1656,7 +1696,7 @@ napi_status napi_get_value_int32(napi_env env,
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
   *result = val->Int32Value(context).ToChecked();
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_get_value_uint32(napi_env env,
@@ -1675,7 +1715,7 @@ napi_status napi_get_value_uint32(napi_env env,
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
   *result = val->Uint32Value(context).ToChecked();
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_get_value_int64(napi_env env,
@@ -1701,7 +1741,7 @@ napi_status napi_get_value_int64(napi_env env,
     *result = val->IntegerValue(context).ToChecked();
   }
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_get_value_bool(napi_env env, napi_value value, bool* result) {
@@ -1716,7 +1756,7 @@ napi_status napi_get_value_bool(napi_env env, napi_value value, bool* result) {
 
   *result = val.As<v8::Boolean>()->Value();
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 // Copies a JavaScript string into a LATIN-1 string buffer. The result is the
@@ -1733,6 +1773,7 @@ napi_status napi_get_value_string_latin1(napi_env env,
                                          size_t bufsize,
                                          size_t* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
 
   v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
   RETURN_STATUS_IF_FALSE(env, val->IsString(), napi_string_expected);
@@ -1768,6 +1809,7 @@ napi_status napi_get_value_string_utf8(napi_env env,
                                        size_t bufsize,
                                        size_t* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
 
   v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
   RETURN_STATUS_IF_FALSE(env, val->IsString(), napi_string_expected);
@@ -1803,6 +1845,7 @@ napi_status napi_get_value_string_utf16(napi_env env,
                                         size_t bufsize,
                                         size_t* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
 
   v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
   RETURN_STATUS_IF_FALSE(env, val->IsString(), napi_string_expected);
@@ -1829,6 +1872,7 @@ napi_status napi_coerce_to_object(napi_env env,
                                   napi_value value,
                                   napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Isolate* isolate = env->isolate;
@@ -1844,6 +1888,7 @@ napi_status napi_coerce_to_bool(napi_env env,
                                 napi_value value,
                                 napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Isolate* isolate = env->isolate;
@@ -1860,6 +1905,7 @@ napi_status napi_coerce_to_number(napi_env env,
                                   napi_value value,
                                   napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Isolate* isolate = env->isolate;
@@ -1876,6 +1922,7 @@ napi_status napi_coerce_to_string(napi_env env,
                                   napi_value value,
                                   napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Isolate* isolate = env->isolate;
@@ -1945,7 +1992,7 @@ napi_status napi_unwrap(napi_env env, napi_value js_object, void** result) {
 
   *result = unwrappedValue.As<v8::External>()->Value();
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_create_external(napi_env env,
@@ -1997,6 +2044,7 @@ napi_status napi_create_reference(napi_env env,
                                   uint32_t initial_refcount,
                                   napi_ref* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8impl::Reference* reference = v8impl::Reference::New(
@@ -2016,7 +2064,7 @@ napi_status napi_delete_reference(napi_env env, napi_ref ref) {
 
   v8impl::Reference::Delete(reinterpret_cast<v8impl::Reference*>(ref));
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 // Increments the reference count, optionally returning the resulting count.
@@ -2121,6 +2169,7 @@ napi_status napi_escape_handle(napi_env env,
                                napi_value* result) {
   NAPI_PREAMBLE(env);
   CHECK_ARG(env, scope);
+  CHECK_ARG(env, escapee);
   CHECK_ARG(env, result);
 
   v8impl::EscapableHandleScopeWrapper* s =
@@ -2136,15 +2185,17 @@ napi_status napi_new_instance(napi_env env,
                               const napi_value* argv,
                               napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, constructor);
+  if (argc > 0) {
+    CHECK_ARG(env, argv);
+  }
   CHECK_ARG(env, result);
 
   v8::Isolate* isolate = env->isolate;
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
 
-  v8::Local<v8::Value> v8value = v8impl::V8LocalValueFromJsValue(constructor);
-  RETURN_STATUS_IF_FALSE(env, v8value->IsFunction(), napi_invalid_arg);
-
-  v8::Local<v8::Function> ctor = v8value.As<v8::Function>();
+  v8::Local<v8::Function> ctor;
+  CHECK_TO_FUNCTION(env, ctor, constructor);
 
   auto maybe = ctor->NewInstance(context, argc,
     reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)));
@@ -2160,6 +2211,7 @@ napi_status napi_instanceof(napi_env env,
                             napi_value constructor,
                             bool* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, object);
   CHECK_ARG(env, result);
 
   *result = false;
@@ -2275,12 +2327,19 @@ napi_status napi_make_callback(napi_env env,
                                const napi_value* argv,
                                napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, recv);
+  if (argc > 0) {
+    CHECK_ARG(env, argv);
+  }
 
   v8::Isolate* isolate = env->isolate;
-  v8::Local<v8::Object> v8recv =
-      v8impl::V8LocalValueFromJsValue(recv).As<v8::Object>();
-  v8::Local<v8::Function> v8func =
-      v8impl::V8LocalValueFromJsValue(func).As<v8::Function>();
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+
+  v8::Local<v8::Object> v8recv;
+  CHECK_TO_OBJECT(env, context, v8recv, recv);
+
+  v8::Local<v8::Function> v8func;
+  CHECK_TO_FUNCTION(env, v8func, func);
 
   v8::Local<v8::Value> callback_result = node::MakeCallback(
       isolate, v8recv, v8func, argc,
@@ -2301,7 +2360,7 @@ napi_status napi_is_exception_pending(napi_env env, bool* result) {
   CHECK_ARG(env, result);
 
   *result = !env->last_exception.IsEmpty();
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_get_and_clear_last_exception(napi_env env,
@@ -2319,7 +2378,7 @@ napi_status napi_get_and_clear_last_exception(napi_env env,
     env->last_exception.Reset();
   }
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_create_buffer(napi_env env,
@@ -2398,6 +2457,7 @@ napi_status napi_create_buffer_copy(napi_env env,
 
 napi_status napi_is_buffer(napi_env env, napi_value value, bool* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   *result = node::Buffer::HasInstance(v8impl::V8LocalValueFromJsValue(value));
@@ -2409,6 +2469,7 @@ napi_status napi_get_buffer_info(napi_env env,
                                  void** data,
                                  size_t* length) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
 
   v8::Local<v8::Object> buffer =
       v8impl::V8LocalValueFromJsValue(value).As<v8::Object>();
@@ -2425,6 +2486,7 @@ napi_status napi_get_buffer_info(napi_env env,
 
 napi_status napi_is_arraybuffer(napi_env env, napi_value value, bool* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
@@ -2488,6 +2550,7 @@ napi_status napi_get_arraybuffer_info(napi_env env,
                                       void** data,
                                       size_t* byte_length) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, arraybuffer);
 
   v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(arraybuffer);
   RETURN_STATUS_IF_FALSE(env, value->IsArrayBuffer(), napi_invalid_arg);
@@ -2508,6 +2571,7 @@ napi_status napi_get_arraybuffer_info(napi_env env,
 
 napi_status napi_is_typedarray(napi_env env, napi_value value, bool* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
@@ -2523,6 +2587,7 @@ napi_status napi_create_typedarray(napi_env env,
                                    size_t byte_offset,
                                    napi_value* result) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, arraybuffer);
   CHECK_ARG(env, result);
 
   v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(arraybuffer);
@@ -2575,6 +2640,7 @@ napi_status napi_get_typedarray_info(napi_env env,
                                      napi_value* arraybuffer,
                                      size_t* byte_offset) {
   NAPI_PREAMBLE(env);
+  CHECK_ARG(env, typedarray);
 
   v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(typedarray);
   RETURN_STATUS_IF_FALSE(env, value->IsTypedArray(), napi_invalid_arg);
@@ -2626,7 +2692,7 @@ napi_status napi_get_typedarray_info(napi_env env,
 
 namespace uvimpl {
 
-napi_status ConvertUVErrorCode(int code) {
+static napi_status ConvertUVErrorCode(int code) {
   switch (code) {
   case 0:
     return napi_ok;
@@ -2717,7 +2783,7 @@ napi_status napi_create_async_work(napi_env env,
 
   *result = reinterpret_cast<napi_async_work>(work);
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_delete_async_work(napi_env env, napi_async_work work) {
@@ -2726,7 +2792,7 @@ napi_status napi_delete_async_work(napi_env env, napi_async_work work) {
 
   uvimpl::Work::Delete(reinterpret_cast<uvimpl::Work*>(work));
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_queue_async_work(napi_env env, napi_async_work work) {
@@ -2744,7 +2810,7 @@ napi_status napi_queue_async_work(napi_env env, napi_async_work work) {
                              uvimpl::Work::ExecuteCallback,
                              uvimpl::Work::CompleteCallback));
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }
 
 napi_status napi_cancel_async_work(napi_env env, napi_async_work work) {
@@ -2755,5 +2821,5 @@ napi_status napi_cancel_async_work(napi_env env, napi_async_work work) {
 
   CALL_UV(env, uv_cancel(reinterpret_cast<uv_req_t*>(w->Request())));
 
-  return napi_ok;
+  return napi_clear_last_error(env);
 }