Skip to content

Commit

Permalink
n-api: add code parameter to error helpers
Browse files Browse the repository at this point in the history
In support of the effort to add error codes to all errors
generated by Node.js, add an optional code parameter to the
helper functions used to throw/create errors in N-API.

PR-URL: nodejs#13988
Fixes: nodejs#13933
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
mhdawson authored and Gabriel Schulhof committed Apr 10, 2018
1 parent fb572c4 commit 5278223
Show file tree
Hide file tree
Showing 9 changed files with 354 additions and 32 deletions.
49 changes: 46 additions & 3 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,31 @@ code needs to create an Error object: [`napi_create_error`][],
where result is the napi_value that refers to the newly created
JavaScript Error object.
The Node.js project is adding error codes to all of the errors
generated internally. The goal is for applications to use these
error codes for all error checking. The associated error messages
will remain, but will only be meant to be used for logging and
display with the expectation that the message can change without
SemVer applying. In order to support this model with N-API, both
in internal functionality and for module specific functionality
(as its good practice), the `throw_` and `create_` functions
take an optional code parameter which is the string for the code
to be added to the error object. If the optional parameter is NULL
then no code will be associated with the error. If a code is provided,
the name associated with the error is also updated to be:
```
originalName [code]
```
where originalName is the original name associated with the error
and code is the code that was provided. For example if the code
is 'ERR_ERROR_1' and a TypeError is being created the name will be:
```
TypeError [ERR_ERROR_1]
```
#### napi_throw
<!-- YAML
added: v8.0.0
Expand All @@ -343,9 +368,12 @@ This API throws the JavaScript Error provided.
added: v8.0.0
-->
```C
NODE_EXTERN napi_status napi_throw_error(napi_env env, const char* msg);
NODE_EXTERN napi_status napi_throw_error(napi_env env,
const char* code,
const char* msg);
```
- `[in] env`: The environment that the API is invoked under.
- `[in] code`: Optional error code to be set on the error.
- `[in] msg`: C string representing the text to be associated with
the error.
Expand All @@ -358,9 +386,12 @@ This API throws a JavaScript Error with the text provided.
added: v8.0.0
-->
```C
NODE_EXTERN napi_status napi_throw_type_error(napi_env env, const char* msg);
NODE_EXTERN napi_status napi_throw_type_error(napi_env env,
const char* code,
const char* msg);
```
- `[in] env`: The environment that the API is invoked under.
- `[in] code`: Optional error code to be set on the error.
- `[in] msg`: C string representing the text to be associated with
the error.

Expand All @@ -373,9 +404,12 @@ This API throws a JavaScript TypeError with the text provided.
added: v8.0.0
-->
```C
NODE_EXTERN napi_status napi_throw_range_error(napi_env env, const char* msg);
NODE_EXTERN napi_status napi_throw_range_error(napi_env env,
const char* code,
const char* msg);
```
- `[in] env`: The environment that the API is invoked under.
- `[in] code`: Optional error code to be set on the error.
- `[in] msg`: C string representing the text to be associated with
the error.
Expand Down Expand Up @@ -409,10 +443,13 @@ added: v8.0.0
-->
```C
NODE_EXTERN napi_status napi_create_error(napi_env env,
napi_value code,
napi_value msg,
napi_value* result);
```
- `[in] env`: The environment that the API is invoked under.
- `[in] code`: Optional `napi_value` with the string for the error code to
be associated with the error.
- `[in] msg`: napi_value that references a JavaScript String to be
used as the message for the Error.
- `[out] result`: `napi_value` representing the error created.
Expand All @@ -427,10 +464,13 @@ added: v8.0.0
-->
```C
NODE_EXTERN napi_status napi_create_type_error(napi_env env,
napi_value code,
napi_value msg,
napi_value* result);
```
- `[in] env`: The environment that the API is invoked under.
- `[in] code`: Optional `napi_value` with the string for the error code to
be associated with the error.
- `[in] msg`: napi_value that references a JavaScript String to be
used as the message for the Error.
- `[out] result`: `napi_value` representing the error created.
Expand All @@ -446,10 +486,13 @@ added: v8.0.0
-->
```C
NODE_EXTERN napi_status napi_create_range_error(napi_env env,
napi_value code,
const char* msg,
napi_value* result);
```
- `[in] env`: The environment that the API is invoked under.
- `[in] code`: Optional `napi_value` with the string for the error code to
be associated with the error.
- `[in] msg`: napi_value that references a JavaScript String to be
used as the message for the Error.
- `[out] result`: `napi_value` representing the error created.
Expand Down
120 changes: 106 additions & 14 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "node_internals.h"
#include "env-inl.h"
#include "node_api_backport.h"
#include "util.h"

#define NAPI_VERSION 1

Expand Down Expand Up @@ -1525,7 +1526,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 @@ -1535,13 +1590,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 @@ -1551,13 +1611,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 @@ -1567,8 +1632,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 @@ -1741,40 +1810,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 @@ -2394,7 +2481,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 @@ -2462,7 +2551,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
3 changes: 2 additions & 1 deletion test/addons-napi/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
const char* error_message = error_info->error_message != NULL ? \
error_info->error_message : \
"empty error message"; \
napi_throw_error((env), error_message); \
napi_throw_error((env), NULL, error_message); \
} \
} while (0)

Expand All @@ -21,6 +21,7 @@
if (!(assertion)) { \
napi_throw_error( \
(env), \
NULL, \
"assertion (" #assertion ") failed: " message); \
return ret_val; \
} \
Expand Down
Loading

0 comments on commit 5278223

Please sign in to comment.