Ensure binding method returns with undefined when error happens

[minggangw]

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

[coveralls]

coverage: 84.526%. remained the same
when pulling 96b9922 on minggangw:fix-1178
into b42699f on RobotWebTools:develop.

[Copilot]

Pull Request Overview

This PR updates error handling in binding finalizers to return undefined on error by introducing NO_RETURN macro variants and capturing the JS environment in lambdas.

• Added CHECK_OP_AND_THROW_ERROR_IF_NOT_TRUE_NO_RETURN and related NO_RETURN macros to macros.h
• Updated all Create* binding functions to capture env in their destructor lambdas
• Replaced THROW_ERROR_IF_NOT_EQUAL with THROW_ERROR_IF_NOT_EQUAL_NO_RETURN in each binding destructor

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/macros.h	Introduce NO_RETURN error-handling macros
src/rcl_timer_bindings.cpp	Capture env and use NO_RETURN macro in timer destructor
src/rcl_time_point_bindings.cpp	Capture env and swap to NO_RETURN macro in time point destructor
src/rcl_subscription_bindings.cpp	Capture env and swap to NO_RETURN macro in subscription destructor
src/rcl_service_bindings.cpp	Capture env and swap to NO_RETURN macro in service destructor
src/rcl_publisher_bindings.cpp	Capture env and swap to NO_RETURN macro in publisher destructor
src/rcl_node_bindings.cpp	Capture env and swap to NO_RETURN macro in node destructor
src/rcl_lifecycle_bindings.cpp	Capture env and swap to NO_RETURN macro in lifecycle destructor
src/rcl_guard_condition_bindings.cpp	Capture env and swap to NO_RETURN macro in guard condition destructor
src/rcl_context_bindings.cpp	Swap to NO_RETURN macro in context destructor
src/rcl_client_bindings.cpp	Capture env and swap to NO_RETURN macro in client destructor
src/rcl_action_server_bindings.cpp	Capture env and swap to NO_RETURN macro in action server destructor
src/rcl_action_goal_bindings.cpp	Capture env and swap to NO_RETURN macro in action goal destructor
src/rcl_action_client_bindings.cpp	Capture env and swap to NO_RETURN macro in action client destructor

Comments suppressed due to low confidence (3)

src/macros.h:47

• Add a brief comment above this macro to explain that the NO_RETURN variant only throws an exception without returning from the enclosing function, clarifying its intended use.

#define THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(lhs, rhs, message) \

src/macros.h:47

• Introduce unit tests that simulate a failing rcl_*_fini call and verify the NO_RETURN macros correctly throw a JS exception and that the binding method returns undefined.

#define THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(lhs, rhs, message) \

src/rcl_timer_bindings.cpp:61

• [nitpick] The line break and indentation for RclHandle::NewInstance here differ from other bindings; consider keeping these calls inline or applying a consistent wrapping style across all binding files.

  auto js_obj =

[Copilot]

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