From fd8cfa8fe3e61c9dc2bf2e7918a24c883982b6b6 Mon Sep 17 00:00:00 2001
From: Jacob Perron <jacob@openrobotics.org>
Date: Wed, 8 Apr 2020 20:58:22 -0700
Subject: [PATCH] Add InvalidParameterTypeException (#1027)

* Add InvalidParameterTypeException

Used to wrap the ParameterTypeException coming from ParameterValue::get() for improving the error message.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Describe new exception

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update tests

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
---
 rclcpp/include/rclcpp/exceptions.hpp | 19 +++++++++++++++++++
 rclcpp/include/rclcpp/node.hpp       |  2 +-
 rclcpp/include/rclcpp/node_impl.hpp  | 16 ++++++++++------
 rclcpp/include/rclcpp/parameter.hpp  |  9 +++++++--
 rclcpp/test/test_node.cpp            |  6 +++---
 5 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/rclcpp/include/rclcpp/exceptions.hpp b/rclcpp/include/rclcpp/exceptions.hpp
index 1976902..4fe5988 100644
--- a/rclcpp/include/rclcpp/exceptions.hpp
+++ b/rclcpp/include/rclcpp/exceptions.hpp
@@ -226,6 +226,25 @@ class InvalidParameterValueException : public std::runtime_error
   using std::runtime_error::runtime_error;
 };
 
+/// Thrown if requested parameter type is invalid.
+/**
+ * Essentially the same as rclcpp::ParameterTypeException, but with parameter
+ * name in the error message.
+ */
+class InvalidParameterTypeException : public std::runtime_error
+{
+public:
+  /// Construct an instance.
+  /**
+   * \param[in] name the name of the parameter.
+   * \param[in] message custom exception message.
+   */
+  RCLCPP_PUBLIC
+  InvalidParameterTypeException(const std::string & name, const std::string message)
+  : std::runtime_error("parameter '" + name + "' has invalid type: " + message)
+  {}
+};
+
 /// Thrown if parameter is already declared.
 class ParameterAlreadyDeclaredException : public std::runtime_error
 {
diff --git a/rclcpp/include/rclcpp/node.hpp b/rclcpp/include/rclcpp/node.hpp
index b9398f5..43e3a94 100644
--- a/rclcpp/include/rclcpp/node.hpp
+++ b/rclcpp/include/rclcpp/node.hpp
@@ -301,7 +301,7 @@ public:
    *
    * If the type of the default value, and therefore also the type of return
    * value, differs from the initial value provided in the node options, then
-   * a rclcpp::ParameterTypeException may be thrown.
+   * a rclcpp::exceptions::InvalidParameterTypeException may be thrown.
    * To avoid this, use the declare_parameter() method which returns an
    * rclcpp::ParameterValue instead.
    *
diff --git a/rclcpp/include/rclcpp/node_impl.hpp b/rclcpp/include/rclcpp/node_impl.hpp
index 60b32b5..b6ed04b 100644
--- a/rclcpp/include/rclcpp/node_impl.hpp
+++ b/rclcpp/include/rclcpp/node_impl.hpp
@@ -156,12 +156,16 @@ Node::declare_parameter(
   const rcl_interfaces::msg::ParameterDescriptor & parameter_descriptor,
   bool ignore_override)
 {
-  return this->declare_parameter(
-    name,
-    rclcpp::ParameterValue(default_value),
-    parameter_descriptor,
-    ignore_override
-  ).get<ParameterT>();
+  try {
+    return this->declare_parameter(
+      name,
+      rclcpp::ParameterValue(default_value),
+      parameter_descriptor,
+      ignore_override
+    ).get<ParameterT>();
+  } catch (const ParameterTypeException & ex) {
+    throw exceptions::InvalidParameterTypeException(name, ex.what());
+  }
 }
 
 template<typename ParameterT>
diff --git a/rclcpp/include/rclcpp/parameter.hpp b/rclcpp/include/rclcpp/parameter.hpp
index cee5585..e01d5aa 100644
--- a/rclcpp/include/rclcpp/parameter.hpp
+++ b/rclcpp/include/rclcpp/parameter.hpp
@@ -22,6 +22,7 @@
 #include <vector>
 
 #include "rcl_interfaces/msg/parameter.hpp"
+#include "rclcpp/exceptions.hpp"
 #include "rclcpp/parameter_value.hpp"
 #include "rclcpp/visibility_control.hpp"
 
@@ -221,8 +222,12 @@ template<typename T>
 decltype(auto)
 Parameter::get_value() const
 {
-  // use the helper to specialize for the ParameterValue and Parameter cases.
-  return detail::get_value_helper<T>(this);
+  try {
+    // use the helper to specialize for the ParameterValue and Parameter cases.
+    return detail::get_value_helper<T>(this);
+  } catch (const ParameterTypeException & ex) {
+    throw exceptions::InvalidParameterTypeException(this->name_, ex.what());
+  }
 }
 
 }  // namespace rclcpp
diff --git a/rclcpp/test/test_node.cpp b/rclcpp/test/test_node.cpp
index 982b131..c85954a 100644
--- a/rclcpp/test/test_node.cpp
+++ b/rclcpp/test/test_node.cpp
@@ -574,7 +574,7 @@ TEST_F(TestNode, declare_parameter_with_overrides) {
     // default type and initial value type do not match
     EXPECT_THROW(
       {node->declare_parameter("parameter_type_mismatch", 42);},
-      rclcpp::ParameterTypeException);
+      rclcpp::exceptions::InvalidParameterTypeException);
   }
 }
 
@@ -1717,7 +1717,7 @@ TEST_F(TestNode, get_parameter_undeclared_parameters_not_allowed) {
       int value;
       node->get_parameter(name, value);
     },
-      rclcpp::ParameterTypeException);
+      rclcpp::exceptions::InvalidParameterTypeException);
   }
 }
 
@@ -1951,7 +1951,7 @@ TEST_F(TestNode, get_parameters_undeclared_parameters_not_allowed) {
       {
         node_local->get_parameters("", actual);
       },
-        rclcpp::ParameterTypeException);
+        rclcpp::exceptions::InvalidParameterTypeException);
     }
   }
 }