additional verbosity for vars parsing exceptions

awx/api/serializers.py

@@ -25,7 +25,7 @@ from django.utils.timezone import now
 from django.utils.functional import cached_property
 
 # Django REST Framework
-from rest_framework.exceptions import ValidationError, PermissionDenied
+from rest_framework.exceptions import ValidationError, PermissionDenied, ParseError
 from rest_framework import fields
 from rest_framework import serializers
 from rest_framework import validators
@@ -3278,10 +3278,9 @@ class JobLaunchSerializer(BaseSerializer):
 
         try:
             extra_vars = parse_yaml_or_json(extra_vars, silent_failure=False)
-        except:
+        except ParseError as e:
-            # Log exception, in case of yet-unknown parsing failures
+            # Catch known user variable formatting errors
-            logger.exception('extra_vars parsing error on Job Template launch.')
+            errors['extra_vars'] = str(e)
-            errors['extra_vars'] = _('Must be valid JSON or YAML.')
 
         if self.get_survey_enabled(obj):
             validation_errors = obj.survey_variable_validation(extra_vars)
@@ -3357,10 +3356,9 @@ class WorkflowJobLaunchSerializer(BaseSerializer):
 
         try:
             extra_vars = parse_yaml_or_json(extra_vars, silent_failure=False)
-        except:
+        except ParseError as e:
-            # Log exception, in case of yet-unknown parsing failures
+            # Catch known user variable formatting errors
-            logger.exception('extra_vars parsing error on Workflow JT launch.')
+            errors['extra_vars'] = str(e)
-            errors['extra_vars'] = _('Must be valid JSON or YAML.')
 
         if self.get_survey_enabled(obj):
             validation_errors = obj.survey_variable_validation(extra_vars)

awx/main/tests/functional/api/test_job_runtime_params.py

@@ -199,7 +199,7 @@ def test_job_reject_invalid_prompted_extra_vars(runtime_data, job_template_promp
         dict(extra_vars='{"unbalanced brackets":'), admin_user, expect=400)
 
     assert 'extra_vars' in response.data
-    assert 'valid JSON or YAML' in str(response.data['extra_vars'][0])
+    assert 'Cannot parse as' in str(response.data['extra_vars'][0])
 
 
 @pytest.mark.django_db

awx/main/tests/unit/utils/test_common.py

@@ -5,9 +5,13 @@
 import os
 import pytest
 from uuid import uuid4
+import json
+import yaml
 
 from django.core.cache import cache
 
+from rest_framework.exceptions import ParseError
+
 from awx.main.utils import common
 
 from awx.main.models import (
@@ -38,6 +42,42 @@ def test_parse_yaml_or_json(input_, output):
     assert common.parse_yaml_or_json(input_) == output
 
 
+class TestParserExceptions:
+
+    @staticmethod
+    def json_error(data):
+        try:
+            json.loads(data)
+            return None
+        except Exception as e:
+            return str(e)
+
+    @staticmethod
+    def yaml_error(data):
+        try:
+            yaml.load(data)
+            return None
+        except Exception as e:
+            return str(e)
+
+    def test_invalid_JSON_and_YAML(self):
+        data = "{key:val"
+        with pytest.raises(ParseError) as exc:
+            common.parse_yaml_or_json(data, silent_failure=False)
+        message = str(exc.value)
+        assert "Cannot parse as" in message
+        assert self.json_error(data) in message
+        assert self.yaml_error(data) in message
+
+    def test_invalid_vars_type(self):
+        data = "[1, 2, 3]"
+        with pytest.raises(ParseError) as exc:
+            common.parse_yaml_or_json(data, silent_failure=False)
+        message = str(exc.value)
+        assert "Cannot parse as" in message
+        assert "Input type `list` is not a dictionary" in message
+
+
 def test_set_environ():
     key = str(uuid4())
     old_environ = os.environ.copy()

awx/main/utils/common.py

@@ -582,12 +582,25 @@ def cache_list_capabilities(page, prefetch_list, model, user):
                 obj.capabilities_cache[display_method] = True
 
 
+def validate_vars_type(vars_obj):
+    if not isinstance(vars_obj, dict):
+        vars_type = type(vars_obj)
+        if hasattr(vars_type, '__name__'):
+            data_type = vars_type.__name__
+        else:
+            data_type = str(vars_type)
+        raise AssertionError(
+            _('Input type `{data_type}` is not a dictionary').format(
+                data_type=data_type)
+        )
+
+
 def parse_yaml_or_json(vars_str, silent_failure=True):
     '''
     Attempt to parse a string of variables.
     First, with JSON parser, if that fails, then with PyYAML.
     If both attempts fail, return an empty dictionary if `silent_failure`
-    is True, re-raise the last error if `silent_failure` if False.
+    is True, re-raise combination error if `silent_failure` if False.
     '''
     if isinstance(vars_str, dict):
         return vars_str
@@ -596,18 +609,21 @@ def parse_yaml_or_json(vars_str, silent_failure=True):
 
     try:
         vars_dict = json.loads(vars_str)
-        assert isinstance(vars_dict, dict)
+        validate_vars_type(vars_dict)
-    except (ValueError, TypeError, AssertionError):
+    except (ValueError, TypeError, AssertionError) as json_err:
         try:
             vars_dict = yaml.safe_load(vars_str)
             # Can be None if '---'
             if vars_dict is None:
                 return {}
-            assert isinstance(vars_dict, dict)
+            validate_vars_type(vars_dict)
-        except (yaml.YAMLError, TypeError, AttributeError, AssertionError):
+        except (yaml.YAMLError, TypeError, AttributeError, AssertionError) as yaml_err:
             if silent_failure:
                 return {}
-            raise
+            raise ParseError(_(
+                'Cannot parse as JSON (error: {json_error}) or '
+                'YAML (error: {yaml_error}).').format(
+                    json_error=str(json_err), yaml_error=str(yaml_err)))
     return vars_dict
 
 

awx/main/validators.py

@@ -11,6 +11,7 @@ from django.core.exceptions import ValidationError
 
 # REST framework
 from rest_framework.serializers import ValidationError as RestValidationError
+from rest_framework.exceptions import ParseError
 
 # AWX
 from awx.main.utils import parse_yaml_or_json
@@ -183,6 +184,6 @@ def vars_validate_or_raise(vars_str):
     try:
         parse_yaml_or_json(vars_str, silent_failure=False)
         return vars_str
-    except:
+    except ParseError as e:
-        raise RestValidationError(_('Must be valid JSON or YAML.'))
+        raise RestValidationError(str(e))