Backport error message improvements (ros2#754)

* Improve launch file parsing error messages

Backport of ros2#626:
* Parser errors clearly show that values are substituted and not part of
  the error message itself.
* On an InvalidLaunchFileError, all parsing errors are logged.

* Add exception type to error output

Backport of ros2#753:
* Output exception type for InvalidLaunchFileErrors
* Add test checking for exception type in output

Signed-off-by: David Yackzan <[email protected]>

Author: dyackzan

launch/launch/invalid_launch_file_error.py

@@ -28,8 +28,12 @@ def __init__(self, extension='', *, likely_errors=None):
             )
         else:
             self._error_message = (
-                'Caught exception when trying to load file of format [{}]: {}'
-            ).format(self._extension, self._likely_errors[0])
+                'Caught {} when trying to load file of format [{}]:'
+            ).format('multiple exceptions' if len(self._likely_errors) > 1 else 'exception',
+                     self._extension)
+            for error in self._likely_errors:
+                self._error_message += '\n - {}: {}'.format(type(error).__name__, error)
+
             self.__cause__ = self._likely_errors[0]
 
     def __str__(self):

launch/test/launch/test_invalid_launch_file_error.py

@@ -0,0 +1,33 @@
+# Copyright 2024 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from launch.invalid_launch_file_error import InvalidLaunchFileError
+
+
+def test_invalid_launch_file_error():
+    try:
+        exception = KeyError('Test')
+        raise InvalidLaunchFileError(extension='.py', likely_errors=[exception])
+    except InvalidLaunchFileError as ex:
+        assert 'KeyError' in ex.__str__()
+
+
+def test_invalid_launch_file_errors():
+    try:
+        exceptions = [ValueError('Test1'), AttributeError('Test2'), BufferError('Test3')]
+        raise InvalidLaunchFileError(extension='.py', likely_errors=exceptions)
+    except InvalidLaunchFileError as ex:
+        assert 'ValueError' in ex.__str__()
+        assert 'AttributeError' in ex.__str__()
+        assert 'BufferError' in ex.__str__()

launch_xml/launch_xml/entity.py

@@ -36,7 +36,7 @@ def __init__(
         *,
         parent: 'Entity' = None
     ) -> Text:
-        """Construnctor."""
+        """Construct the Entity."""
         self.__xml_element = xml_element
         self.__parent = parent
         self.__read_attributes = set()
@@ -91,7 +91,7 @@ def get_attr(
         If coercion fails, `ValueError` will be raised.
         """
         attr_error = AttributeError(
-            'Attribute {} of type {} not found in Entity {}'.format(
+            "Attribute '{}' of type '{}' not found in Entity '{}'".format(
                 name, data_type, self.type_name
             )
         )
@@ -123,8 +123,8 @@ def get_attr(
             value = get_typed_value(value, data_type, can_be_str=can_be_str)
         except ValueError:
             raise TypeError(
-                'Attribute {} of Entity {} expected to be of type {}.'
-                '`{}` can not be converted to one of those types'.format(
+                "Attribute '{}' of Entity '{}' expected to be of type '{}'."
+                "'{}' can not be converted to one of those types".format(
                     name, self.type_name, data_type, value
                 )
             )

launch_yaml/launch_yaml/entity.py

@@ -116,7 +116,7 @@ def get_attr(
         if name not in self.__element:
             if not optional:
                 raise AttributeError(
-                    'Can not find attribute {} in Entity {}'.format(
+                    "Can not find attribute '{}' in Entity '{}'".format(
                         name, self.type_name))
             else:
                 return None
@@ -126,13 +126,13 @@ def get_attr(
             if isinstance(data, list) and isinstance(data[0], dict):
                 return [Entity(child, name) for child in data]
             raise TypeError(
-                'Attribute {} of Entity {} expected to be a list of dictionaries.'.format(
+                "Attribute '{}' of Entity '{}' expected to be a list of dictionaries.".format(
                     name, self.type_name
                 )
             )
         if not is_instance_of(data, data_type, can_be_str=can_be_str):
             raise TypeError(
-                'Attribute {} of Entity {} expected to be of type {}, got {}'.format(
+                "Attribute '{}' of Entity '{}' expected to be of type '{}', got '{}'".format(
                     name, self.type_name, data_type, type(data)
                 )
             )