Skip to content

Commit c4278a7

Browse files
minggangwminggangw
committed
Ensure binding method returns with undefined when error happens (#1179)
This PR ensures that binding methods return undefined when an error occurs by updating the error handling behavior. Key changes include updating lambda capture lists to include the JavaScript environment (env) and replacing THROW_ERROR_IF_NOT_EQUAL with THROW_ERROR_IF_NOT_EQUAL_NO_RETURN to modify error handling flows. Fox: #1178
1 parent 1a23985 commit c4278a7

14 files changed

+70
-38
lines changed

‎src/macros.h‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,33 @@
2323
{ \
2424
if (lhs op rhs) { \
2525
rcl_reset_error(); \
26-
Napi::Error::New(rclnodejs::GetEnv(), message) \
26+
Napi::Error::New(env, message) \
2727
.ThrowAsJavaScriptException(); \
28+
return env.Undefined(); \
2829
} \
2930
}
3031

32+
#define CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE_NO_RETURN(op, lhs, rhs, message) \
33+
{ \
34+
if (lhs op rhs) { \
35+
rcl_reset_error(); \
36+
Napi::Error::New(env, message) \
37+
.ThrowAsJavaScriptException(); \
38+
} \
39+
}
40+
3141
#define THROW_ERROR_IF_NOT_EQUAL(lhs, rhs, message) \
3242
CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE(!=, lhs, rhs, message)
3343

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

47+
#define THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(lhs, rhs, message) \
48+
CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE_NO_RETURN(!=, lhs, rhs, message)
49+
50+
#define THROW_ERROR_IF_EQUAL_NO_RETURN(lhs, rhs, message) \
51+
CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE_NO_RETURN(==, lhs, rhs, message)
52+
3753
#define PACKAGE_NAME "rclnodejs"
3854

3955
#ifdef DEBUG_ON

‎src/rcl_action_client_bindings.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,13 @@ Napi::Value ActionCreateClient(const Napi::CallbackInfo& info) {
7575
&action_client_ops),
7676
RCL_RET_OK, rcl_get_error_string().str);
7777
auto js_obj = RclHandle::NewInstance(
78-
env, action_client, node_handle, [node](void* ptr) {
78+
env, action_client, node_handle, [node, env](void* ptr) {
7979
rcl_action_client_t* action_client =
8080
reinterpret_cast<rcl_action_client_t*>(ptr);
8181
rcl_ret_t ret = rcl_action_client_fini(action_client, node);
8282
free(ptr);
83-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
83+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
84+
rcl_get_error_string().str);
8485
});
8586

8687
return js_obj;

‎src/rcl_action_goal_bindings.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,13 @@ Napi::Value ActionAcceptNewGoal(const Napi::CallbackInfo& info) {
4949
malloc(sizeof(rcl_action_goal_handle_t)));
5050
*goal_handle = *new_goal;
5151
auto js_obj =
52-
RclHandle::NewInstance(env, goal_handle, nullptr, [](void* ptr) {
52+
RclHandle::NewInstance(env, goal_handle, nullptr, [env](void* ptr) {
5353
rcl_action_goal_handle_t* goal_handle =
5454
reinterpret_cast<rcl_action_goal_handle_t*>(ptr);
5555
rcl_ret_t ret = rcl_action_goal_handle_fini(goal_handle);
5656
free(ptr);
57-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
57+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
58+
rcl_get_error_string().str);
5859
});
5960

6061
return js_obj;

‎src/rcl_action_server_bindings.cpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,13 @@ Napi::Value ActionCreateServer(const Napi::CallbackInfo& info) {
8181
action_name.c_str(), &action_server_ops),
8282
RCL_RET_OK, rcl_get_error_string().str);
8383
auto js_obj = RclHandle::NewInstance(
84-
env, action_server, node_handle, [node](void* ptr) {
84+
env, action_server, node_handle, [node, env](void* ptr) {
8585
rcl_action_server_t* action_server =
8686
reinterpret_cast<rcl_action_server_t*>(ptr);
8787
rcl_ret_t ret = rcl_action_server_fini(action_server, node);
8888
free(ptr);
89-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
89+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
90+
rcl_get_error_string().str);
9091
});
9192

9293
return js_obj;
@@ -390,13 +391,14 @@ Napi::Value ActionProcessCancelRequest(const Napi::CallbackInfo& info) {
390391
}
391392

392393
*response = cancel_response_ptr->msg;
393-
auto js_obj =
394-
RclHandle::NewInstance(env, cancel_response_ptr, nullptr, [](void* ptr) {
394+
auto js_obj = RclHandle::NewInstance(
395+
env, cancel_response_ptr, nullptr, [env](void* ptr) {
395396
rcl_action_cancel_response_t* cancel_response_ptr =
396397
reinterpret_cast<rcl_action_cancel_response_t*>(ptr);
397398
rcl_ret_t ret = rcl_action_cancel_response_fini(cancel_response_ptr);
398399
free(ptr);
399-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
400+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
401+
rcl_get_error_string().str);
400402
});
401403
return js_obj;
402404
}

‎src/rcl_client_bindings.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,13 @@ Napi::Value CreateClient(const Napi::CallbackInfo& info) {
5353
rcl_client_init(client, node, ts, service_name.c_str(), &client_ops),
5454
RCL_RET_OK, rcl_get_error_string().str);
5555

56-
auto js_obj =
57-
RclHandle::NewInstance(env, client, node_handle, [node](void* ptr) {
56+
auto js_obj = RclHandle::NewInstance(
57+
env, client, node_handle, [node, env](void* ptr) {
5858
rcl_client_t* client = reinterpret_cast<rcl_client_t*>(ptr);
5959
rcl_ret_t ret = rcl_client_fini(client, node);
6060
free(ptr);
61-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
61+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
62+
rcl_get_error_string().str);
6263
});
6364

6465
return js_obj;

‎src/rcl_context_bindings.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ Napi::Value CreateContext(const Napi::CallbackInfo& info) {
120120
rcl_context_t* context = reinterpret_cast<rcl_context_t*>(ptr);
121121
rcl_ret_t ret = DestroyContext(env, context);
122122
free(ptr);
123-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
123+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
124+
rcl_get_error_string().str);
124125
});
125126

126127
return js_obj;

‎src/rcl_guard_condition_bindings.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,12 @@ Napi::Value CreateGuardCondition(const Napi::CallbackInfo& info) {
4141
rcl_guard_condition_init(gc, context, gc_options),
4242
rcl_get_error_string().str);
4343

44-
auto handle = RclHandle::NewInstance(env, gc, nullptr, [](void* ptr) {
44+
auto handle = RclHandle::NewInstance(env, gc, nullptr, [env](void* ptr) {
4545
rcl_guard_condition_t* gc = reinterpret_cast<rcl_guard_condition_t*>(ptr);
4646
rcl_ret_t ret = rcl_guard_condition_fini(gc);
4747
free(ptr);
48-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
48+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
49+
rcl_get_error_string().str);
4950
});
5051

5152
return handle;

‎src/rcl_lifecycle_bindings.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,13 @@ Napi::Value CreateLifecycleStateMachine(const Napi::CallbackInfo& info) {
8383
rcl_get_error_string().str);
8484

8585
auto js_obj = RclHandle::NewInstance(
86-
env, state_machine, node_handle, [node](void* ptr) {
86+
env, state_machine, node_handle, [node, env](void* ptr) {
8787
rcl_lifecycle_state_machine_t* state_machine =
8888
reinterpret_cast<rcl_lifecycle_state_machine_t*>(ptr);
8989
rcl_ret_t ret = rcl_lifecycle_state_machine_fini(state_machine, node);
9090
free(ptr);
91-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
91+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
92+
rcl_get_error_string().str);
9293
});
9394
#else
9495
const rcl_node_options_t* node_options =
@@ -101,13 +102,14 @@ Napi::Value CreateLifecycleStateMachine(const Napi::CallbackInfo& info) {
101102
rcl_get_error_string().str);
102103

103104
auto js_obj = RclHandle::NewInstance(
104-
env, state_machine, node_handle, [node, node_options](void* ptr) {
105+
env, state_machine, node_handle, [node, node_options, env](void* ptr) {
105106
rcl_lifecycle_state_machine_t* state_machine =
106107
reinterpret_cast<rcl_lifecycle_state_machine_t*>(ptr);
107108
rcl_ret_t ret = rcl_lifecycle_state_machine_fini(
108109
state_machine, node, &node_options->allocator);
109110
free(ptr);
110-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
111+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
112+
rcl_get_error_string().str);
111113
});
112114
#endif
113115

‎src/rcl_node_bindings.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,12 @@ Napi::Value CreateNode(const Napi::CallbackInfo& info) {
217217
name_space.c_str(), context, &options),
218218
rcl_get_error_string().str);
219219

220-
auto handle = RclHandle::NewInstance(env, node, nullptr, [](void* ptr) {
220+
auto handle = RclHandle::NewInstance(env, node, nullptr, [env](void* ptr) {
221221
rcl_node_t* node = reinterpret_cast<rcl_node_t*>(ptr);
222222
rcl_ret_t ret = rcl_node_fini(node);
223223
free(ptr);
224-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
224+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
225+
rcl_get_error_string().str);
225226
});
226227

227228
return handle;

‎src/rcl_publisher_bindings.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,13 @@ Napi::Value CreatePublisher(const Napi::CallbackInfo& info) {
5555
rcl_publisher_init(publisher, node, ts, topic.c_str(), &publisher_ops),
5656
RCL_RET_OK, rcl_get_error_string().str);
5757

58-
auto js_obj =
59-
RclHandle::NewInstance(env, publisher, node_handle, [node](void* ptr) {
58+
auto js_obj = RclHandle::NewInstance(
59+
env, publisher, node_handle, [node, env](void* ptr) {
6060
rcl_publisher_t* publisher = reinterpret_cast<rcl_publisher_t*>(ptr);
6161
rcl_ret_t ret = rcl_publisher_fini(publisher, node);
6262
free(ptr);
63-
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
63+
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
64+
rcl_get_error_string().str);
6465
});
6566

6667
return js_obj;

0 commit comments

Comments
 (0)