Skip to content

Commit

Permalink
Do not crash Executor when send_response fails due to client failure. (
Browse files Browse the repository at this point in the history
…#2276) (#2280)

* Do not crash Executor when send_response fails due to client failure.

Related to ros2/ros2#1253

It is not sane that a faulty client can crash our service Executor, as
discussed in the referred issue, if the client is not setup properly,
send_response may return RCL_RET_TIMEOUT, we should not throw an error
in this case.

Signed-off-by: Zang MingJie <zealot0630@gmail.com>

* Update rclcpp/include/rclcpp/service.hpp

Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Zang MingJie <zealot0630@gmail.com>

* address review comments.

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

---------

Signed-off-by: Zang MingJie <zealot0630@gmail.com>
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Zang MingJie <zealot0630@gmail.com>
(cherry picked from commit fbe8f28)

Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
  • Loading branch information
mergify[bot] and fujitatomoya committed Sep 1, 2023
1 parent ec4d00e commit 689e510
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions rclcpp/include/rclcpp/service.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,14 @@ class Service
{
rcl_ret_t ret = rcl_send_response(get_service_handle().get(), &req_id, &response);

if (ret == RCL_RET_TIMEOUT) {
RCLCPP_WARN(
node_logger_.get_child("rclcpp"),
"failed to send response to %s (timeout): %s",
this->get_service_name(), rcl_get_error_string().str);
rcl_reset_error();
return;
}
if (ret != RCL_RET_OK) {
rclcpp::exceptions::throw_from_rcl_error(ret, "failed to send response");
}
Expand Down

0 comments on commit 689e510

Please sign in to comment.