Skip to content

Commit 9c29dca

Browse files
minggangwminggangw
committed
Ensure binding method returns with undefined when error happens
1 parent b42699f commit 9c29dca

14 files changed

+69
-37
lines changed

‎src/macros.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,31 @@
2525
rcl_reset_error(); \
2626
Napi::Error::New(rclnodejs::GetEnv(), 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(rclnodejs::GetEnv(), 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)