Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include original exception in ComponentManagerException #1157

Merged
merged 2 commits into from
Jun 26, 2020

Conversation

mbuijs
Copy link
Contributor

@mbuijs mbuijs commented Jun 8, 2020

When a component throws an exception in the constructor (e.g. when declaring a parameter) during loading, there's no other feedback than Component constructor threw an exception. This change introduces a similar construction to what is done earlier in the same file for constructing class_loader::ClassLoader: try to catch std::exception and append it's what() to the new ComponentManagerException.

@mbuijs mbuijs force-pushed the print_component_constructor_exception branch from 079b66d to 89275ac Compare June 8, 2020 09:27
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mjcarroll mjcarroll self-assigned this Jun 8, 2020
Copy link
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking back my approval, because this doesn't pass the linters.

rclcpp_components/src/component_manager.cpp Outdated Show resolved Hide resolved
Signed-off-by: Martijn Buijs <martijn.buijs@gmail.com>
@mbuijs mbuijs force-pushed the print_component_constructor_exception branch from 89275ac to 9e76460 Compare June 8, 2020 20:22
@mbuijs mbuijs requested a review from mjcarroll June 9, 2020 08:04
@mbuijs
Copy link
Contributor Author

mbuijs commented Jun 23, 2020

Taking back my approval, because this doesn't pass the linters.

I've clicked the Re-request review button, but I'm not sure if it actually does something, so through this message I would like to request you to review this pull request again. I have fixed the linter error about the line length in an attempt to be consistent with the line breaks in the rest of the file.

Co-authored-by: tomoya <Tomoya.Fujita@sony.com>
Signed-off-by: Martijn Buijs <martijn.buijs@gmail.com>
@mbuijs mbuijs force-pushed the print_component_constructor_exception branch from 2322c02 to 668959f Compare June 24, 2020 09:21
@mjcarroll mjcarroll merged commit c23572f into ros2:master Jun 26, 2020
@mbuijs mbuijs deleted the print_component_constructor_exception branch June 26, 2020 19:10
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Jul 7, 2020
* Include original exception in ComponentManagerException

Signed-off-by: Martijn Buijs <martijn.buijs@gmail.com>

* Update rclcpp_components/src/component_manager.cpp

Co-authored-by: tomoya <Tomoya.Fujita@sony.com>
Signed-off-by: Martijn Buijs <martijn.buijs@gmail.com>

Co-authored-by: tomoya <Tomoya.Fujita@sony.com>
dawonn-haval pushed a commit to dawonn-haval/rclcpp that referenced this pull request Jul 10, 2020
* Include original exception in ComponentManagerException

Signed-off-by: Martijn Buijs <martijn.buijs@gmail.com>

* Update rclcpp_components/src/component_manager.cpp

Co-authored-by: tomoya <Tomoya.Fujita@sony.com>
Signed-off-by: Martijn Buijs <martijn.buijs@gmail.com>

Co-authored-by: tomoya <Tomoya.Fujita@sony.com>
jacobperron pushed a commit that referenced this pull request Oct 1, 2020
* Include original exception in ComponentManagerException

Signed-off-by: Martijn Buijs <martijn.buijs@gmail.com>

* Update rclcpp_components/src/component_manager.cpp

Co-authored-by: tomoya <Tomoya.Fujita@sony.com>
Signed-off-by: Martijn Buijs <martijn.buijs@gmail.com>

Co-authored-by: tomoya <Tomoya.Fujita@sony.com>

Co-authored-by: Martijn Buijs <Martijn.buijs@gmail.com>
Co-authored-by: tomoya <Tomoya.Fujita@sony.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants