Skip to content

Commit

Permalink
Update to new error helper signatures in n-api
Browse files Browse the repository at this point in the history
Update to use new signatures
Update copies of n-api files so they match. I've not done
a direct copy because more work is needed given that the
n-api files now use node internals.  I have updated so that
they are consistent and will build ok with respect to
this specific chaange. I took this approach to minimize
the time the wrapper is broken by the breaking change
on the n-api side.

PR-URL: #78
Reviewed-By: Kyle Farnung <[email protected]>
  • Loading branch information
mhdawson committed Jul 14, 2017
1 parent 92d14c0 commit 179d135
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 22 deletions.
8 changes: 4 additions & 4 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1452,10 +1452,10 @@ inline Error Error::New(napi_env env) {
case napi_string_expected:
case napi_boolean_expected:
case napi_number_expected:
status = napi_create_type_error(env, message, &error);
status = napi_create_type_error(env, nullptr, message, &error);
break;
default:
status = napi_create_error(env, message, &error);
status = napi_create_error(env, nullptr, message, &error);
break;
}
assert(status == napi_ok);
Expand Down Expand Up @@ -1556,7 +1556,7 @@ inline TError Error::New(napi_env env,
NAPI_THROW_IF_FAILED(env, status, TError());

napi_value error;
status = create_error(env, str, &error);
status = create_error(env, nullptr, str, &error);
NAPI_THROW_IF_FAILED(env, status, TError());

return TError(env, error);
Expand Down Expand Up @@ -2467,7 +2467,7 @@ inline napi_value ObjectWrap<T>::ConstructorCallbackWrapper(
if (status != napi_ok) return nullptr;

if (!isConstructCall) {
napi_throw_type_error(env, "Class constructors cannot be invoked without 'new'");
napi_throw_type_error(env, nullptr, "Class constructors cannot be invoked without 'new'");
return nullptr;
}

Expand Down
2 changes: 1 addition & 1 deletion napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,7 @@ namespace Napi {

protected:
/// !cond INTERNAL
typedef napi_status (*create_error_fn)(napi_env envb, napi_value msg, napi_value* result);
typedef napi_status (*create_error_fn)(napi_env envb, napi_value code, napi_value msg, napi_value* result);

template <typename TError>
static TError New(napi_env env,
Expand Down
119 changes: 105 additions & 14 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1523,7 +1523,61 @@ napi_status napi_create_symbol(napi_env env,
return GET_RETURN_STATUS(env);
}

static napi_status set_error_code(napi_env env,
v8::Local<v8::Value> error,
napi_value code,
const char* code_cstring) {
if ((code != nullptr) || (code_cstring != nullptr)) {
v8::Isolate* isolate = env->isolate;
v8::Local<v8::Context> context = isolate->GetCurrentContext();
v8::Local<v8::Object> err_object = error.As<v8::Object>();

v8::Local<v8::Value> code_value = v8impl::V8LocalValueFromJsValue(code);
if (code != nullptr) {
code_value = v8impl::V8LocalValueFromJsValue(code);
RETURN_STATUS_IF_FALSE(env, code_value->IsString(), napi_string_expected);
} else {
CHECK_NEW_FROM_UTF8(env, code_value, code_cstring);
}

v8::Local<v8::Name> code_key;
CHECK_NEW_FROM_UTF8(env, code_key, "code");

v8::Maybe<bool> set_maybe = err_object->Set(context, code_key, code_value);
RETURN_STATUS_IF_FALSE(env,
set_maybe.FromMaybe(false),
napi_generic_failure);

// now update the name to be "name [code]" where name is the
// original name and code is the code associated with the Error
v8::Local<v8::String> name_string;
CHECK_NEW_FROM_UTF8(env, name_string, "");
v8::Local<v8::Name> name_key;
CHECK_NEW_FROM_UTF8(env, name_key, "name");

auto maybe_name = err_object->Get(context, name_key);
if (!maybe_name.IsEmpty()) {
v8::Local<v8::Value> name = maybe_name.ToLocalChecked();
if (name->IsString()) {
name_string = v8::String::Concat(name_string, name.As<v8::String>());
}
}
name_string = v8::String::Concat(name_string,
FIXED_ONE_BYTE_STRING(isolate, " ["));
name_string = v8::String::Concat(name_string, code_value.As<v8::String>());
name_string = v8::String::Concat(name_string,
FIXED_ONE_BYTE_STRING(isolate, "]"));

set_maybe = err_object->Set(context, name_key, name_string);
RETURN_STATUS_IF_FALSE(env,
set_maybe.FromMaybe(false),
napi_generic_failure);
}
return napi_ok;
}

napi_status napi_create_error(napi_env env,
napi_value code,
napi_value msg,
napi_value* result) {
NAPI_PREAMBLE(env);
Expand All @@ -1533,13 +1587,18 @@ napi_status napi_create_error(napi_env env,
v8::Local<v8::Value> message_value = v8impl::V8LocalValueFromJsValue(msg);
RETURN_STATUS_IF_FALSE(env, message_value->IsString(), napi_string_expected);

*result = v8impl::JsValueFromV8LocalValue(v8::Exception::Error(
message_value.As<v8::String>()));
v8::Local<v8::Value> error_obj =
v8::Exception::Error(message_value.As<v8::String>());
napi_status status = set_error_code(env, error_obj, code, nullptr);
if (status != napi_ok) return status;

*result = v8impl::JsValueFromV8LocalValue(error_obj);

return GET_RETURN_STATUS(env);
}

napi_status napi_create_type_error(napi_env env,
napi_value code,
napi_value msg,
napi_value* result) {
NAPI_PREAMBLE(env);
Expand All @@ -1549,13 +1608,18 @@ napi_status napi_create_type_error(napi_env env,
v8::Local<v8::Value> message_value = v8impl::V8LocalValueFromJsValue(msg);
RETURN_STATUS_IF_FALSE(env, message_value->IsString(), napi_string_expected);

*result = v8impl::JsValueFromV8LocalValue(v8::Exception::TypeError(
message_value.As<v8::String>()));
v8::Local<v8::Value> error_obj =
v8::Exception::TypeError(message_value.As<v8::String>());
napi_status status = set_error_code(env, error_obj, code, nullptr);
if (status != napi_ok) return status;

*result = v8impl::JsValueFromV8LocalValue(error_obj);

return GET_RETURN_STATUS(env);
}

napi_status napi_create_range_error(napi_env env,
napi_value code,
napi_value msg,
napi_value* result) {
NAPI_PREAMBLE(env);
Expand All @@ -1565,8 +1629,12 @@ napi_status napi_create_range_error(napi_env env,
v8::Local<v8::Value> message_value = v8impl::V8LocalValueFromJsValue(msg);
RETURN_STATUS_IF_FALSE(env, message_value->IsString(), napi_string_expected);

*result = v8impl::JsValueFromV8LocalValue(v8::Exception::RangeError(
message_value.As<v8::String>()));
v8::Local<v8::Value> error_obj =
v8::Exception::RangeError(message_value.As<v8::String>());
napi_status status = set_error_code(env, error_obj, code, nullptr);
if (status != napi_ok) return status;

*result = v8impl::JsValueFromV8LocalValue(error_obj);

return GET_RETURN_STATUS(env);
}
Expand Down Expand Up @@ -1739,40 +1807,58 @@ napi_status napi_throw(napi_env env, napi_value error) {
return napi_clear_last_error(env);
}

napi_status napi_throw_error(napi_env env, const char* msg) {
napi_status napi_throw_error(napi_env env,
const char* code,
const char* msg) {
NAPI_PREAMBLE(env);

v8::Isolate* isolate = env->isolate;
v8::Local<v8::String> str;
CHECK_NEW_FROM_UTF8(env, str, msg);

isolate->ThrowException(v8::Exception::Error(str));
v8::Local<v8::Value> error_obj = v8::Exception::Error(str);
napi_status status = set_error_code(env, error_obj, nullptr, code);
if (status != napi_ok) return status;

isolate->ThrowException(error_obj);
// any VM calls after this point and before returning
// to the javascript invoker will fail
return napi_clear_last_error(env);
}

napi_status napi_throw_type_error(napi_env env, const char* msg) {
napi_status napi_throw_type_error(napi_env env,
const char* code,
const char* msg) {
NAPI_PREAMBLE(env);

v8::Isolate* isolate = env->isolate;
v8::Local<v8::String> str;
CHECK_NEW_FROM_UTF8(env, str, msg);

isolate->ThrowException(v8::Exception::TypeError(str));
v8::Local<v8::Value> error_obj = v8::Exception::TypeError(str);
napi_status status = set_error_code(env, error_obj, nullptr, code);
if (status != napi_ok) return status;

isolate->ThrowException(error_obj);
// any VM calls after this point and before returning
// to the javascript invoker will fail
return napi_clear_last_error(env);
}

napi_status napi_throw_range_error(napi_env env, const char* msg) {
napi_status napi_throw_range_error(napi_env env,
const char* code,
const char* msg) {
NAPI_PREAMBLE(env);

v8::Isolate* isolate = env->isolate;
v8::Local<v8::String> str;
CHECK_NEW_FROM_UTF8(env, str, msg);

isolate->ThrowException(v8::Exception::RangeError(str));
v8::Local<v8::Value> error_obj = v8::Exception::RangeError(str);
napi_status status = set_error_code(env, error_obj, nullptr, code);
if (status != napi_ok) return status;

isolate->ThrowException(error_obj);
// any VM calls after this point and before returning
// to the javascript invoker will fail
return napi_clear_last_error(env);
Expand Down Expand Up @@ -2392,7 +2478,9 @@ napi_status napi_instanceof(napi_env env,
CHECK_TO_OBJECT(env, context, ctor, constructor);

if (!ctor->IsFunction()) {
napi_throw_type_error(env, "constructor must be a function");
napi_throw_type_error(env,
"ERR_NAPI_CONS_FUNCTION",
"Constructor must be a function");

return napi_set_last_error(env, napi_function_expected);
}
Expand Down Expand Up @@ -2460,7 +2548,10 @@ napi_status napi_instanceof(napi_env env,

v8::Local<v8::Value> prototype_property = maybe_prototype.ToLocalChecked();
if (!prototype_property->IsObject()) {
napi_throw_type_error(env, "constructor.prototype must be an object");
napi_throw_type_error(
env,
"ERR_NAPI_CONS_PROTOTYPE_OBJECT",
"Constructor.prototype must be an object");

return napi_set_last_error(env, napi_object_expected);
}
Expand Down
15 changes: 12 additions & 3 deletions src/node_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,15 @@ NAPI_EXTERN napi_status napi_create_function(napi_env env,
void* data,
napi_value* result);
NAPI_EXTERN napi_status napi_create_error(napi_env env,
napi_value code,
napi_value msg,
napi_value* result);
NAPI_EXTERN napi_status napi_create_type_error(napi_env env,
napi_value code,
napi_value msg,
napi_value* result);
NAPI_EXTERN napi_status napi_create_range_error(napi_env env,
napi_value code,
napi_value msg,
napi_value* result);

Expand Down Expand Up @@ -404,9 +407,15 @@ NAPI_EXTERN napi_status napi_escape_handle(napi_env env,

// Methods to support error handling
NAPI_EXTERN napi_status napi_throw(napi_env env, napi_value error);
NAPI_EXTERN napi_status napi_throw_error(napi_env env, const char* msg);
NAPI_EXTERN napi_status napi_throw_type_error(napi_env env, const char* msg);
NAPI_EXTERN napi_status napi_throw_range_error(napi_env env, const char* msg);
NAPI_EXTERN napi_status napi_throw_error(napi_env env,
const char* code,
const char* msg);
NAPI_EXTERN napi_status napi_throw_type_error(napi_env env,
const char* code,
const char* msg);
NAPI_EXTERN napi_status napi_throw_range_error(napi_env env,
const char* code,
const char* msg);
NAPI_EXTERN napi_status napi_is_error(napi_env env,
napi_value value,
bool* result);
Expand Down

0 comments on commit 179d135

Please sign in to comment.