Implement various error message improvements.

jhgg · jhgg · commit cf10db643abe · 2015-11-20T17:57:31.000-05:00
Closes #39.
diff --git a/graphql/core/execution/values.py b/graphql/core/execution/values.py
@@ -79,7 +79,8 @@ def get_variable_value(schema, definition_ast, input):
             [definition_ast]
         )
 
-    if is_valid_value(type, input):
+    errors = is_valid_value(input, type)
+    if not errors:
         if input is None:
             default_value = definition_ast.default_value
             if default_value:
@@ -96,11 +97,12 @@ def get_variable_value(schema, definition_ast, input):
             [definition_ast]
         )
 
+    message = (u'\n' + u'\n'.join(errors)) if errors else u''
     raise GraphQLError(
-        'Variable "${}" expected value of type "{}" but got: {}.'.format(
+        'Variable "${}" got invalid value {}.{}'.format(
             variable.name.value,
-            print_ast(definition_ast.type),
-            json.dumps(input, sort_keys=True)
+            json.dumps(input, sort_keys=True),
+            message
         ),
         [definition_ast]
     )
diff --git a/graphql/core/utils/is_valid_literal_value.py b/graphql/core/utils/is_valid_literal_value.py
@@ -1,4 +1,5 @@
 from ..language import ast
+from ..language.printer import print_ast
 from ..type.definition import (
     GraphQLEnumType,
     GraphQLInputObjectType,
@@ -7,45 +8,66 @@
     GraphQLScalarType,
 )
 
+_empty_list = []
+
 
 def is_valid_literal_value(type, value_ast):
     if isinstance(type, GraphQLNonNull):
+        of_type = type.of_type
         if not value_ast:
-            return False
+            if of_type.name:
+                return [u'Expected "{}", found null.'.format(type)]
+
+            return [u'Expected non-null value, found null.']
 
-        of_type = type.of_type
         return is_valid_literal_value(of_type, value_ast)
 
     if not value_ast:
-        return True
+        return _empty_list
 
     if isinstance(value_ast, ast.Variable):
-        return True
+        return _empty_list
 
     if isinstance(type, GraphQLList):
         item_type = type.of_type
         if isinstance(value_ast, ast.ListValue):
-            return all(is_valid_literal_value(item_type, item_ast) for item_ast in value_ast.values)
+            errors = []
+
+            for i, item_ast in enumerate(value_ast.values):
+                item_errors = is_valid_literal_value(item_type, item_ast)
+                for error in item_errors:
+                    errors.append(u'In element #{}: {}'.format(i, error))
+
+            return errors
 
         return is_valid_literal_value(item_type, value_ast)
 
     if isinstance(type, GraphQLInputObjectType):
         if not isinstance(value_ast, ast.ObjectValue):
-            return False
+            return [u'Expected "{}", found not an object.'.format(type)]
 
         fields = type.get_fields()
         field_asts = value_ast.fields
 
-        if any(not fields.get(field_ast.name.value, None) for field_ast in field_asts):
-            return False
+        errors = []
+        for provided_field_ast in field_asts:
+            if provided_field_ast.name.value not in fields:
+                errors.append(u'In field "{}": Unknown field.'.format(provided_field_ast.name.value))
 
         field_ast_map = {field_ast.name.value: field_ast for field_ast in field_asts}
-        get_field_ast_value = lambda field_name: field_ast_map[
-            field_name].value if field_name in field_ast_map else None
+        get_field_ast_value = lambda field_name: field_ast_map[field_name].value \
+            if field_name in field_ast_map else None
 
-        return all(is_valid_literal_value(field.type, get_field_ast_value(field_name))
-                   for field_name, field in fields.items())
+        for field_name, field in fields.items():
+            subfield_errors = is_valid_literal_value(field.type, get_field_ast_value(field_name))
+            errors.extend(u'In field "{}": {}'.format(field_name, e) for e in subfield_errors)
+
+        return errors
 
     assert isinstance(type, (GraphQLScalarType, GraphQLEnumType)), 'Must be input type'
 
-    return type.parse_literal(value_ast) is not None
+    parse_result = type.parse_literal(value_ast)
+    if parse_result is None:
+        return [u'Expected type "{}", found {}.'.format(type.name, print_ast(value_ast))]
+
+    return _empty_list
diff --git a/graphql/core/utils/is_valid_value.py b/graphql/core/utils/is_valid_value.py
@@ -3,6 +3,7 @@
 """
 
 import collections
+import json
 from six import string_types
 from ..type import (
     GraphQLEnumType,
@@ -12,45 +13,62 @@
     GraphQLScalarType,
 )
 
+_empty_list = []
 
-def is_valid_value(type, value):
+
+def is_valid_value(value, type):
     """Given a type and any value, return True if that value is valid."""
     if isinstance(type, GraphQLNonNull):
+        of_type = type.of_type
         if value is None:
-            return False
+            if hasattr(of_type, 'name'):
+                return [u'Expected "{}", found null.'.format(type)]
+
+            return [u'Expected non-null value, found null.']
 
-        return is_valid_value(type.of_type, value)
+        return is_valid_value(value, of_type)
 
     if value is None:
-        return True
+        return _empty_list
 
     if isinstance(type, GraphQLList):
         item_type = type.of_type
-        if not isinstance(value, string_types) and \
-                isinstance(value, collections.Iterable):
-            return all(is_valid_value(item_type, item) for item in value)
+        if not isinstance(value, string_types) and isinstance(value, collections.Iterable):
+            errors = []
+            for i, item in enumerate(value):
+                item_errors = is_valid_value(item, item_type)
+                for error in item_errors:
+                    errors.append(u'In element #{}: {}'.format(i, error))
+
+            return errors
+
         else:
-            return is_valid_value(item_type, value)
+            return is_valid_value(value, item_type)
 
     if isinstance(type, GraphQLInputObjectType):
         if not isinstance(value, collections.Mapping):
-            return False
+            return [u'Expected "{}", found not an object.'.format(type)]
 
         fields = type.get_fields()
+        errors = []
+
+        for provided_field in value.keys():
+            if provided_field not in fields:
+                errors.append(u'In field "{}": Unknown field.'.format(provided_field))
 
-        # Ensure every provided field is defined.
-        if any(field_name not in fields for field_name in value.keys()):
-            return False
+        for field_name, field in fields.items():
+            subfield_errors = is_valid_value(value.get(field_name), field.type)
+            errors.extend(u'In field "{}": {}'.format(field_name, e) for e in subfield_errors)
 
-        # Ensure every defined field is valid.
-        return all(
-            is_valid_value(field.type, value.get(field_name))
-            for field_name, field in fields.items()
-        )
+        return errors
 
     assert isinstance(type, (GraphQLScalarType, GraphQLEnumType)), \
         'Must be input type'
 
     # Scalar/Enum input checks to ensure the type can parse the value to
     # a non-null value.
-    return type.parse_value(value) is not None
+    parse_result = type.parse_value(value)
+    if parse_result is None:
+        return [u'Expected type "{}", found {}.'.format(type, json.dumps(value))]
+
+    return _empty_list
diff --git a/graphql/core/validation/rules/arguments_of_correct_type.py b/graphql/core/validation/rules/arguments_of_correct_type.py
@@ -7,13 +7,16 @@
 class ArgumentsOfCorrectType(ValidationRule):
     def enter_Argument(self, node, key, parent, path, ancestors):
         arg_def = self.context.get_argument()
-        if arg_def and not is_valid_literal_value(arg_def.type, node.value):
-            return GraphQLError(
-                self.bad_value_message(node.name.value, arg_def.type,
-                                       print_ast(node.value)),
-                [node.value]
-            )
+        if arg_def:
+            errors = is_valid_literal_value(arg_def.type, node.value)
+            if errors:
+                return GraphQLError(
+                    self.bad_value_message(node.name.value, arg_def.type,
+                                           print_ast(node.value), errors),
+                    [node.value]
+                )
 
     @staticmethod
-    def bad_value_message(arg_name, type, value):
-        return 'Argument "{}" expected type "{}" but got: {}.'.format(arg_name, type, value)
+    def bad_value_message(arg_name, type, value, verbose_errors):
+        message = (u'\n' + u'\n'.join(verbose_errors)) if verbose_errors else ''
+        return 'Argument "{}" has invalid value {}.{}'.format(arg_name, value, message)
diff --git a/graphql/core/validation/rules/default_values_of_correct_type.py b/graphql/core/validation/rules/default_values_of_correct_type.py
@@ -17,17 +17,20 @@ def enter_VariableDefinition(self, node, key, parent, path, ancestors):
                 [default_value]
             )
 
-        if type and default_value and not is_valid_literal_value(type, default_value):
-            return GraphQLError(
-                self.bad_value_for_default_arg_message(name, type, print_ast(default_value)),
-                [default_value]
-            )
+        if type and default_value:
+            errors = is_valid_literal_value(type, default_value)
+            if errors:
+                return GraphQLError(
+                    self.bad_value_for_default_arg_message(name, type, print_ast(default_value), errors),
+                    [default_value]
+                )
 
     @staticmethod
     def default_for_non_null_arg_message(var_name, type, guess_type):
-        return 'Variable "${}" of type "{}" is required and will not use the default value. ' \
-               'Perhaps you meant to use type "{}".'.format(var_name, type, guess_type)
+        return u'Variable "${}" of type "{}" is required and will not use the default value. ' \
+               u'Perhaps you meant to use type "{}".'.format(var_name, type, guess_type)
 
     @staticmethod
-    def bad_value_for_default_arg_message(var_name, type, value):
-        return 'Variable "${}" of type "{}" has invalid default value: {}.'.format(var_name, type, value)
+    def bad_value_for_default_arg_message(var_name, type, value, verbose_errors):
+        message = (u'\n' + u'\n'.join(verbose_errors)) if verbose_errors else u''
+        return u'Variable "${}" of type "{}" has invalid default value: {}.{}'.format(var_name, type, value, message)
diff --git a/tests/core_execution/test_variables.py b/tests/core_execution/test_variables.py
@@ -39,6 +39,14 @@ def input_to_json(obj, args, info):
         return stringify(input)
 
 
+TestNestedInputObject = GraphQLInputObjectType(
+    name='TestNestedInputObject',
+    fields={
+        'na': GraphQLInputObjectField(GraphQLNonNull(TestInputObject)),
+        'nb': GraphQLInputObjectField(GraphQLNonNull(GraphQLString))
+    }
+)
+
 TestType = GraphQLObjectType('TestType', {
     'fieldWithObjectInput': GraphQLField(
         GraphQLString,
@@ -56,6 +64,10 @@ def input_to_json(obj, args, info):
         GraphQLString,
         args={'input': GraphQLArgument(GraphQLString, 'Hello World')},
         resolver=input_to_json),
+    'fieldWithNestedInputObject': GraphQLField(
+        GraphQLString,
+        args={'input': GraphQLArgument(TestNestedInputObject, 'Hello World')},
+        resolver=input_to_json),
     'list': GraphQLField(
         GraphQLString,
         args={'input': GraphQLArgument(GraphQLList(GraphQLString))},
@@ -135,6 +147,7 @@ def test_does_not_use_incorrect_value():
     })
 
 
+# noinspection PyMethodMayBeStatic
 class TestUsingVariables:
     doc = '''
     query q($input: TestInputObject) {
@@ -179,8 +192,8 @@ def test_errors_on_null_for_nested_non_null(self):
 
         assert format_error(excinfo.value) == {
             'locations': [{'column': 13, 'line': 2}],
-            'message': 'Variable "$input" expected value of type "TestInputObject" but got: '
-                       '{}.'.format(stringify(params['input']))
+            'message': 'Variable "$input" got invalid value {}.\n'
+                       'In field "c": Expected "String!", found null.'.format(stringify(params['input']))
         }
 
     def test_errors_on_incorrect_type(self):
@@ -191,8 +204,8 @@ def test_errors_on_incorrect_type(self):
 
         assert format_error(excinfo.value) == {
             'locations': [{'column': 13, 'line': 2}],
-            'message': 'Variable "$input" expected value of type "TestInputObject" but got: '
-                       '{}.'.format(stringify(params['input']))
+            'message': 'Variable "$input" got invalid value {}.\n'
+                       'Expected "TestInputObject", found not an object.'.format(stringify(params['input']))
         }
 
     def test_errors_on_omission_of_nested_non_null(self):
@@ -203,20 +216,50 @@ def test_errors_on_omission_of_nested_non_null(self):
 
         assert format_error(excinfo.value) == {
             'locations': [{'column': 13, 'line': 2}],
-            'message': 'Variable "$input" expected value of type "TestInputObject" but got: '
-                       '{}.'.format(stringify(params['input']))
+            'message': 'Variable "$input" got invalid value {}.\n'
+                       'In field "c": Expected "String!", found null.'.format(stringify(params['input']))
         }
 
-    def test_errors_on_addition_of_unknown_input_field(self):
+    def test_errors_on_deep_nested_errors_and_with_many_errors(self):
+        nested_doc = '''
+          query q($input: TestNestedInputObject) {
+            fieldWithNestedObjectInput(input: $input)
+          }
+        '''
+
+        params = {'input': {'na': {'a': 'foo'}}}
+        with raises(GraphQLError) as excinfo:
+            check(nested_doc, {}, params)
+
+        assert format_error(excinfo.value) == {
+            'locations': [{'column': 19, 'line': 2}],
+            'message': 'Variable "$input" got invalid value {}.\n'
+                       'In field "na": In field "c": Expected "String!", found null.\n'
+                       'In field "nb": Expected "String!", found null.'.format(stringify(params['input']))
+        }
+
+    def test_errors_on_addition_of_input_field_of_incorrect_type(self):
         params = {'input': {'a': 'foo', 'b': 'bar', 'c': 'baz', 'd': 'dog'}}
 
         with raises(GraphQLError) as excinfo:
             check(self.doc, {}, params)
 
         assert format_error(excinfo.value) == {
             'locations': [{'column': 13, 'line': 2}],
-            'message': 'Variable "$input" expected value of type "TestInputObject" but got: '
-                       '{}.'.format(stringify(params['input']))
+            'message': 'Variable "$input" got invalid value {}.\n'
+                       'In field "d": Expected type "ComplexScalar", found "dog".'.format(stringify(params['input']))
+        }
+
+    def test_errors_on_addition_of_unknown_input_field(self):
+        params = {'input': {'a': 'foo', 'b': 'bar', 'c': 'baz', 'f': 'dog'}}
+
+        with raises(GraphQLError) as excinfo:
+            check(self.doc, {}, params)
+
+        assert format_error(excinfo.value) == {
+            'locations': [{'column': 13, 'line': 2}],
+            'message': 'Variable "$input" got invalid value {}.\n'
+                       'In field "f": Unknown field.'.format(stringify(params['input']))
         }
 
 
@@ -496,8 +539,8 @@ def test_does_not_allow_lists_of_non_nulls_to_contain_null():
 
     assert format_error(excinfo.value) == {
         'locations': [{'column': 13, 'line': 2}],
-        'message': 'Variable "$input" expected value of type "[String!]" but got: '
-                   '{}.'.format(stringify(params['input']))
+        'message': 'Variable "$input" got invalid value {}.\n'
+                   'In element #1: Expected "String!", found null.'.format(stringify(params['input']))
     }
 
 
@@ -544,8 +587,8 @@ def test_does_not_allow_non_null_lists_of_non_nulls_to_contain_null():
 
     assert format_error(excinfo.value) == {
         'locations': [{'column': 13, 'line': 2}],
-        'message': 'Variable "$input" expected value of type "[String!]!" but got: '
-                   '{}.'.format(stringify(params['input']))
+        'message': 'Variable "$input" got invalid value {}.\n'
+                   'In element #1: Expected "String!", found null.'.format(stringify(params['input']))
     }
 
 
diff --git a/tests/core_type/test_enum_type.py b/tests/core_type/test_enum_type.py
diff --git a/tests/core_validation/test_arguments_of_correct_type.py b/tests/core_validation/test_arguments_of_correct_type.py
diff --git a/tests/core_validation/test_default_values_of_correct_type.py b/tests/core_validation/test_default_values_of_correct_type.py
