More Helpful Error Messages (#275)

* More Helpful Error Messages

Signed-off-by: David V. Lu <[email protected]>

* Linting

Signed-off-by: David V. Lu <[email protected]>

Author: DLu

launch_ros/launch_ros/utilities/evaluate_parameters.py

@@ -70,9 +70,17 @@ def check_sequence_type_is_allowed(sequence):
             if isinstance(value[0], Substitution):
                 # Value is a list of substitutions, so perform them to make a string
                 evaluated_value = perform_substitutions(context, list(value))
-                yaml_evaluated_value = yaml.safe_load(evaluated_value)
-                if yaml_evaluated_value is None:
-                    yaml_evaluated_value = ''
+
+                try:
+                    yaml_evaluated_value = yaml.safe_load(evaluated_value)
+                except yaml.YAMLError:
+                    raise TypeError(
+                        'Unable to parse the value of parameter {} as yaml. '
+                        'If the parameter is meant to be a string, try wrapping it in '
+                        'launch_ros.parameter_descriptions.ParameterValue'
+                        '(value, value_type=str)'.format(evaluated_name)
+                    )
+
                 if type(yaml_evaluated_value) in (bool, int, float, str, bytes):
                     evaluated_value = yaml_evaluated_value
                 elif isinstance(yaml_evaluated_value, Sequence):
@@ -87,9 +95,10 @@ def check_sequence_type_is_allowed(sequence):
                 else:
                     raise TypeError(
                         'Allowed value types are bytes, bool, int, float, str, Sequence[bool]'
-                        ', Sequence[int], Sequence[float], Sequence[str]. Got {}.'.format(
-                            type(yaml_evaluated_value)
-                        )
+                        ', Sequence[int], Sequence[float], Sequence[str]. Got {}.'
+                        'If the parameter is meant to be a string, try wrapping it in '
+                        'launch_ros.parameter_descriptions.ParameterValue'
+                        '(value, value_type=str)'.format(type(yaml_evaluated_value))
                     )
             elif isinstance(value[0], Sequence):
                 # Value is an array of a list of substitutions

test_launch_ros/test/test_launch_ros/utilities/test_normalize_parameters.py

@@ -15,10 +15,12 @@
 """Tests for the normalizing parameters utility."""
 
 import pathlib
+from typing import List
 
 from launch import LaunchContext
 from launch.substitutions import TextSubstitution
 
+from launch_ros.parameter_descriptions import ParameterValue
 from launch_ros.utilities import evaluate_parameters
 from launch_ros.utilities import normalize_parameters
 
@@ -339,3 +341,58 @@ def test_unallowed_yaml_types_in_substitutions():
         norm = normalize_parameters(orig)
         evaluate_parameters(LaunchContext(), norm)
     assert 'Expected a non-empty sequence' in str(exc.value)
+
+    with pytest.raises(TypeError) as exc:
+        orig = [{'foo': 1, 'fiz': TextSubstitution(text='Text That : Cannot Be Parsed As : Yaml')}]
+        norm = normalize_parameters(orig)
+        evaluate_parameters(LaunchContext(), norm)
+    assert 'Unable to parse' in str(exc.value)
+
+
+def test_unallowed_yaml_types_as_strings():
+    # All the tests from test_unallowed_yaml_types_in_substitutions
+    # but coerced to the proper type with ParameterValue
+    orig = [{'foo': 1, 'fiz': ParameterValue(TextSubstitution(text="{'asd': 3}"), value_type=str)}]
+    norm = normalize_parameters(orig)
+    expected = ({'foo': 1, 'fiz': "{'asd': 3}"},)
+    assert evaluate_parameters(LaunchContext(), norm) == expected
+
+    orig = [{'foo': 1, 'fiz': ParameterValue(TextSubstitution(text='[1, 2.0, 3]'),
+                                             value_type=str)}]
+    norm = normalize_parameters(orig)
+    evaluate_parameters(LaunchContext(), norm)
+    expected = ({'foo': 1, 'fiz': '[1, 2.0, 3]'},)
+    assert evaluate_parameters(LaunchContext(), norm) == expected
+
+    orig = [{'foo': 1, 'fiz': ParameterValue(TextSubstitution(text='[[2, 3], [2, 3], [2, 3]]'),
+                                             value_type=str)}]
+    norm = normalize_parameters(orig)
+    evaluate_parameters(LaunchContext(), norm)
+    expected = ({'foo': 1, 'fiz': '[[2, 3], [2, 3], [2, 3]]'},)
+    assert evaluate_parameters(LaunchContext(), norm) == expected
+
+    orig = [{'foo': 1, 'fiz': ParameterValue(TextSubstitution(text='[]'), value_type=str)}]
+    norm = normalize_parameters(orig)
+    evaluate_parameters(LaunchContext(), norm)
+    expected = ({'foo': 1, 'fiz': '[]'},)
+    assert evaluate_parameters(LaunchContext(), norm) == expected
+
+    orig = [{
+        'foo': 1,
+        'fiz': ParameterValue([
+            [TextSubstitution(text="['asd', 'bsd']")],
+            [TextSubstitution(text="['asd', 'csd']")]
+        ], value_type=List[str])
+    }]
+    norm = normalize_parameters(orig)
+    evaluate_parameters(LaunchContext(), norm)
+    expected = ({'foo': 1, 'fiz': ["['asd', 'bsd']", "['asd', 'csd']"]},)
+    assert evaluate_parameters(LaunchContext(), norm) == expected
+
+    orig = [{'foo': 1,
+             'fiz': ParameterValue(TextSubstitution(text='Text That : Cannot Be Parsed As : Yaml'),
+                                   value_type=str)}]
+    norm = normalize_parameters(orig)
+    evaluate_parameters(LaunchContext(), norm)
+    expected = ({'foo': 1, 'fiz': 'Text That : Cannot Be Parsed As : Yaml'},)
+    assert evaluate_parameters(LaunchContext(), norm) == expected