Skip to content

Ensure binding method returns with undefined when error happens #1179

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 23, 2025
Merged

Ensure binding method returns with undefined when error happens #1179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c29dca
Ensure binding method returns with undefined when error happens
minggangw Jun 23, 2025
96b9922
Address comments
minggangw Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,33 @@
{ \
if (lhs op rhs) { \
rcl_reset_error(); \
Napi::Error::New(rclnodejs::GetEnv(), message) \
Napi::Error::New(env, message) \
.ThrowAsJavaScriptException(); \
return env.Undefined(); \
} \
}

#define CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE_NO_RETURN(op, lhs, rhs, message) \
Copy link

Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the captured env parameter consistently (e.g., Napi::Error::New(env, message)) instead of calling rclnodejs::GetEnv(), to avoid any mismatch between environments.

Copilot uses AI. Check for mistakes.
{ \
if (lhs op rhs) { \
rcl_reset_error(); \
Napi::Error::New(env, message) \
.ThrowAsJavaScriptException(); \
} \
}

#define THROW_ERROR_IF_NOT_EQUAL(lhs, rhs, message) \
CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE(!=, lhs, rhs, message)

#define THROW_ERROR_IF_EQUAL(lhs, rhs, message) \
CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE(==, lhs, rhs, message)

#define THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(lhs, rhs, message) \
CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE_NO_RETURN(!=, lhs, rhs, message)

#define THROW_ERROR_IF_EQUAL_NO_RETURN(lhs, rhs, message) \
CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE_NO_RETURN(==, lhs, rhs, message)

#define PACKAGE_NAME "rclnodejs"

#ifdef DEBUG_ON
Expand Down
5 changes: 3 additions & 2 deletions src/rcl_action_client_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,13 @@ Napi::Value ActionCreateClient(const Napi::CallbackInfo& info) {
&action_client_ops),
RCL_RET_OK, rcl_get_error_string().str);
auto js_obj = RclHandle::NewInstance(
env, action_client, node_handle, [node](void* ptr) {
env, action_client, node_handle, [node, env](void* ptr) {
rcl_action_client_t* action_client =
reinterpret_cast<rcl_action_client_t*>(ptr);
rcl_ret_t ret = rcl_action_client_fini(action_client, node);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return js_obj;
Expand Down
5 changes: 3 additions & 2 deletions src/rcl_action_goal_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,13 @@ Napi::Value ActionAcceptNewGoal(const Napi::CallbackInfo& info) {
malloc(sizeof(rcl_action_goal_handle_t)));
*goal_handle = *new_goal;
auto js_obj =
RclHandle::NewInstance(env, goal_handle, nullptr, [](void* ptr) {
RclHandle::NewInstance(env, goal_handle, nullptr, [env](void* ptr) {
rcl_action_goal_handle_t* goal_handle =
reinterpret_cast<rcl_action_goal_handle_t*>(ptr);
rcl_ret_t ret = rcl_action_goal_handle_fini(goal_handle);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return js_obj;
Expand Down
12 changes: 7 additions & 5 deletions src/rcl_action_server_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,13 @@ Napi::Value ActionCreateServer(const Napi::CallbackInfo& info) {
action_name.c_str(), &action_server_ops),
RCL_RET_OK, rcl_get_error_string().str);
auto js_obj = RclHandle::NewInstance(
env, action_server, node_handle, [node](void* ptr) {
env, action_server, node_handle, [node, env](void* ptr) {
rcl_action_server_t* action_server =
reinterpret_cast<rcl_action_server_t*>(ptr);
rcl_ret_t ret = rcl_action_server_fini(action_server, node);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return js_obj;
Expand Down Expand Up @@ -390,13 +391,14 @@ Napi::Value ActionProcessCancelRequest(const Napi::CallbackInfo& info) {
}

*response = cancel_response_ptr->msg;
auto js_obj =
RclHandle::NewInstance(env, cancel_response_ptr, nullptr, [](void* ptr) {
auto js_obj = RclHandle::NewInstance(
env, cancel_response_ptr, nullptr, [env](void* ptr) {
rcl_action_cancel_response_t* cancel_response_ptr =
reinterpret_cast<rcl_action_cancel_response_t*>(ptr);
rcl_ret_t ret = rcl_action_cancel_response_fini(cancel_response_ptr);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});
return js_obj;
}
Expand Down
7 changes: 4 additions & 3 deletions src/rcl_client_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ Napi::Value CreateClient(const Napi::CallbackInfo& info) {
rcl_client_init(client, node, ts, service_name.c_str(), &client_ops),
RCL_RET_OK, rcl_get_error_string().str);

auto js_obj =
RclHandle::NewInstance(env, client, node_handle, [node](void* ptr) {
auto js_obj = RclHandle::NewInstance(
env, client, node_handle, [node, env](void* ptr) {
rcl_client_t* client = reinterpret_cast<rcl_client_t*>(ptr);
rcl_ret_t ret = rcl_client_fini(client, node);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return js_obj;
Expand Down
3 changes: 2 additions & 1 deletion src/rcl_context_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ Napi::Value CreateContext(const Napi::CallbackInfo& info) {
rcl_context_t* context = reinterpret_cast<rcl_context_t*>(ptr);
rcl_ret_t ret = DestroyContext(env, context);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return js_obj;
Expand Down
5 changes: 3 additions & 2 deletions src/rcl_guard_condition_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ Napi::Value CreateGuardCondition(const Napi::CallbackInfo& info) {
rcl_guard_condition_init(gc, context, gc_options),
rcl_get_error_string().str);

auto handle = RclHandle::NewInstance(env, gc, nullptr, [](void* ptr) {
auto handle = RclHandle::NewInstance(env, gc, nullptr, [env](void* ptr) {
rcl_guard_condition_t* gc = reinterpret_cast<rcl_guard_condition_t*>(ptr);
rcl_ret_t ret = rcl_guard_condition_fini(gc);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return handle;
Expand Down
10 changes: 6 additions & 4 deletions src/rcl_lifecycle_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,13 @@ Napi::Value CreateLifecycleStateMachine(const Napi::CallbackInfo& info) {
rcl_get_error_string().str);

auto js_obj = RclHandle::NewInstance(
env, state_machine, node_handle, [node](void* ptr) {
env, state_machine, node_handle, [node, env](void* ptr) {
rcl_lifecycle_state_machine_t* state_machine =
reinterpret_cast<rcl_lifecycle_state_machine_t*>(ptr);
rcl_ret_t ret = rcl_lifecycle_state_machine_fini(state_machine, node);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});
#else
const rcl_node_options_t* node_options =
Expand All @@ -101,13 +102,14 @@ Napi::Value CreateLifecycleStateMachine(const Napi::CallbackInfo& info) {
rcl_get_error_string().str);

auto js_obj = RclHandle::NewInstance(
env, state_machine, node_handle, [node, node_options](void* ptr) {
env, state_machine, node_handle, [node, node_options, env](void* ptr) {
rcl_lifecycle_state_machine_t* state_machine =
reinterpret_cast<rcl_lifecycle_state_machine_t*>(ptr);
rcl_ret_t ret = rcl_lifecycle_state_machine_fini(
state_machine, node, &node_options->allocator);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});
#endif

Expand Down
5 changes: 3 additions & 2 deletions src/rcl_node_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,12 @@ Napi::Value CreateNode(const Napi::CallbackInfo& info) {
name_space.c_str(), context, &options),
rcl_get_error_string().str);

auto handle = RclHandle::NewInstance(env, node, nullptr, [](void* ptr) {
auto handle = RclHandle::NewInstance(env, node, nullptr, [env](void* ptr) {
rcl_node_t* node = reinterpret_cast<rcl_node_t*>(ptr);
rcl_ret_t ret = rcl_node_fini(node);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return handle;
Expand Down
7 changes: 4 additions & 3 deletions src/rcl_publisher_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,13 @@ Napi::Value CreatePublisher(const Napi::CallbackInfo& info) {
rcl_publisher_init(publisher, node, ts, topic.c_str(), &publisher_ops),
RCL_RET_OK, rcl_get_error_string().str);

auto js_obj =
RclHandle::NewInstance(env, publisher, node_handle, [node](void* ptr) {
auto js_obj = RclHandle::NewInstance(
env, publisher, node_handle, [node, env](void* ptr) {
rcl_publisher_t* publisher = reinterpret_cast<rcl_publisher_t*>(ptr);
rcl_ret_t ret = rcl_publisher_fini(publisher, node);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return js_obj;
Expand Down
7 changes: 4 additions & 3 deletions src/rcl_service_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,13 @@ Napi::Value CreateService(const Napi::CallbackInfo& info) {
THROW_ERROR_IF_NOT_EQUAL(
rcl_service_init(service, node, ts, service_name.c_str(), &service_ops),
RCL_RET_OK, rcl_get_error_string().str);
auto js_obj =
RclHandle::NewInstance(env, service, node_handle, [node](void* ptr) {
auto js_obj = RclHandle::NewInstance(
env, service, node_handle, [node, env](void* ptr) {
rcl_service_t* service = reinterpret_cast<rcl_service_t*>(ptr);
rcl_ret_t ret = rcl_service_fini(service, node);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return js_obj;
Expand Down
5 changes: 3 additions & 2 deletions src/rcl_subscription_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,13 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) {
rcl_get_error_string().str);

auto js_obj = RclHandle::NewInstance(
env, subscription, node_handle, [node](void* ptr) {
env, subscription, node_handle, [node, env](void* ptr) {
rcl_subscription_t* subscription =
reinterpret_cast<rcl_subscription_t*>(ptr);
rcl_ret_t ret = rcl_subscription_fini(subscription, node);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return js_obj;
Expand Down
5 changes: 3 additions & 2 deletions src/rcl_time_point_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,12 @@ Napi::Value CreateClock(const Napi::CallbackInfo& info) {
rcl_clock_init(clock_type, clock, &allocator),
rcl_get_error_string().str);

return RclHandle::NewInstance(env, clock, nullptr, [](void* ptr) {
return RclHandle::NewInstance(env, clock, nullptr, [env](void* ptr) {
rcl_clock_t* clock = reinterpret_cast<rcl_clock_t*>(ptr);
rcl_ret_t ret = rcl_clock_fini(clock);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});
}

Expand Down
14 changes: 8 additions & 6 deletions src/rcl_timer_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@ Napi::Value CreateTimer(const Napi::CallbackInfo& info) {
rcl_get_error_string().str);
#endif

auto js_obj = RclHandle::NewInstance(env, timer, clock_handle, [](void* ptr) {
rcl_timer_t* timer = reinterpret_cast<rcl_timer_t*>(ptr);
rcl_ret_t ret = rcl_timer_fini(timer);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
});
auto js_obj =
RclHandle::NewInstance(env, timer, clock_handle, [env](void* ptr) {
rcl_timer_t* timer = reinterpret_cast<rcl_timer_t*>(ptr);
rcl_ret_t ret = rcl_timer_fini(timer);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
});

return js_obj;
}
Expand Down
Loading