fix(agnocastlib): error handling agnocastlib#1138
Open
Conversation
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces exit(EXIT_FAILURE) calls in agnocastlib with thrown C++ exceptions to enable proper error propagation. Previously, any failure in agnocastlib would immediately kill the process; now errors can be caught and handled upstream.
Changes:
- Exception replacement: All
RCLCPP_ERROR+close(agnocast_fd)+exit(EXIT_FAILURE)patterns replaced withthrow std::runtime_error(for ioctl/mq/shm failures) orthrow std::invalid_argument(for invalid caller arguments). - Widened catch blocks: Four component containers (both in
agnocastlibandagnocast_components) changed fromcatch (rclcpp_components::ComponentManagerException&)tocatch (const std::exception&)to intercept the newly thrown exceptions. - New
<stdexcept>include: Added toagnocast_utils.hppto make exception types available throughout the header chain.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/agnocastlib/src/agnocast_utils.cpp |
Replaces exit() with throw std::runtime_error in validate_ld_preload and create_mq_name |
src/agnocastlib/src/agnocast_subscription.cpp |
Replaces exit() with throw std::runtime_error in initialize, get_publisher_count_core, open_mq_for_subscription |
src/agnocastlib/src/agnocast_publisher.cpp |
Replaces exit() with throw std::runtime_error in initialize_publisher, publish_core, get_subscription_count_core, get_intra_subscription_count_core |
src/agnocastlib/src/agnocast_client.cpp |
Replaces exit() with throw std::runtime_error in get_agnocast_sub_count |
src/agnocastlib/src/agnocast_callback_info.cpp |
Replaces exit() with throw std::runtime_error in receive_and_execute_message |
src/agnocastlib/include/agnocast/agnocast_utils.hpp |
Adds <stdexcept> include; replaces exit() with throw std::invalid_argument in validate_qos |
src/agnocastlib/include/agnocast/agnocast_subscription.hpp |
Replaces exit() with throw std::invalid_argument in get_valid_callback_group; replaces exit() with throw std::runtime_error in BasicTakeSubscription::take |
src/agnocastlib/include/agnocast/agnocast_publisher.hpp |
Replaces exit() with throw std::invalid_argument in BasicPublisher::publish |
src/agnocastlib/include/agnocast/agnocast_callback_info.hpp |
Replaces exit() with throw std::runtime_error in get_erased_callback |
src/agnocastlib/src/agnocast_component_container.cpp |
Widens catch from ComponentManagerException to std::exception |
src/agnocastlib/src/agnocast_component_container_mt.cpp |
Widens catch from ComponentManagerException to std::exception |
src/agnocast_components/src/agnocast_component_container.cpp |
Widens catch from ComponentManagerException to std::exception |
src/agnocast_components/src/agnocast_component_container_mt.cpp |
Widens catch from ComponentManagerException to std::exception |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replace
exit(EXIT_FAILURE)with exception throwing in agnocastlib for better error handling.Changes
RCLCPP_ERROR+exit()to throwing exceptionsstd::runtime_error: Runtime errors (ioctl/mq_open/shm failures)std::invalid_argument: Invalid arguments from caller (invalid message, unsupported QoS, callback group not in node)std::logic_error: Programming errors (publish withoutborrow_loaned_message)ComponentManagerException) to catch (std::exception) in component containersstd::terminate()from uncaught exceptions. Usesstd::atomic<bool>flag to propagate failure to the main thread, which rethrows after joining so thatmain()can perform cleanup (close(agnocast_fd)) and exit withEXIT_FAILUREEXPECT_EXIT(..., ExitedWithCode(EXIT_FAILURE), ...) toEXPECT_THROW(..., <exception_type>) for tests that verify error handling behaviorRelated links
How was this PR tested?
bash scripts/test/e2e_test_1to1(required)bash scripts/test/e2e_test_2to2(required)Notes for reviewers
Version Update Label (Required)
Please add exactly one of the following labels to this PR:
need-major-update: User API breaking changesneed-minor-update: Internal API breaking changes (heaphook/kmod/agnocastlib compatibility)need-patch-update: Bug fixes and other changesImportant notes:
need-major-updateorneed-minor-update, please include this in the PR title as well.fix(foo)[needs major version update]: barorfeat(baz)[needs minor version update]: quxrun-build-testlabel. The PR can only be merged after the build tests pass.See CONTRIBUTING.md for detailed versioning rules.