Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing status message list update for napi_handle_scope_mismatch #16973

Closed
RReverser opened this issue Nov 12, 2017 · 5 comments
Closed

Missing status message list update for napi_handle_scope_mismatch #16973

RReverser opened this issue Nov 12, 2017 · 5 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. good first issue Issues that are suitable for first-time contributors. node-api Issues and PRs related to the Node-API.

Comments

@RReverser
Copy link
Member

  • Subsystem: n-api

Unless I'm missing something, #16201 should've also updated

node/src/node_api.cc

Lines 927 to 934 in c5a49e1

// 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(
node::arraysize(error_messages) == napi_escape_called_twice + 1,
"Count of error messages must match count of error values");
CHECK_LE(env->last_error.error_code, napi_escape_called_twice);
and

node/src/node_api.cc

Lines 889 to 901 in c5a49e1

const char* error_messages[] = {nullptr,
"Invalid argument",
"An object was expected",
"A string was expected",
"A string or symbol was expected",
"A function was expected",
"A number was expected",
"A boolean was expected",
"An array was expected",
"Unknown failure",
"An exception is pending",
"The async work item was cancelled",
"napi_escape_handle already called on scope"};
, as currently assertion at
CHECK_LE(env->last_error.error_code, napi_escape_called_twice);
will fail whenever someone tries to call napi_get_last_error_info right after napi_handle_scope_mismatch occurred.

@RReverser RReverser added c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API. labels Nov 12, 2017
@addaleax addaleax added good first issue Issues that are suitable for first-time contributors. mentor-available labels Nov 12, 2017
@shivanth
Copy link
Contributor

Can I take this up ?

@mhdawson
Copy link
Member

@RReverser thanks for catching this.

@shivanth sure.

@neta-kedem
Copy link

hi, I'm working on this issue

@neta-kedem
Copy link

I've made a PR for this issue: #17161
#goodness_squad

@mhdawson
Copy link
Member

#17161 landed closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. good first issue Issues that are suitable for first-time contributors. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

No branches or pull requests

5 participants