Skip to content

Commit

Permalink
Implement various error message improvements.
Browse files Browse the repository at this point in the history
Closes #39.
  • Loading branch information
jhgg committed Nov 20, 2015
1 parent 436e13f commit cf10db6
Show file tree
Hide file tree
Showing 9 changed files with 203 additions and 86 deletions.
10 changes: 6 additions & 4 deletions graphql/core/execution/values.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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]
)
Expand Down
48 changes: 35 additions & 13 deletions graphql/core/utils/is_valid_literal_value.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from ..language import ast
from ..language.printer import print_ast
from ..type.definition import (
GraphQLEnumType,
GraphQLInputObjectType,
Expand All @@ -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
54 changes: 36 additions & 18 deletions graphql/core/utils/is_valid_value.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""

import collections
import json
from six import string_types
from ..type import (
GraphQLEnumType,
Expand All @@ -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
19 changes: 11 additions & 8 deletions graphql/core/validation/rules/arguments_of_correct_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
21 changes: 12 additions & 9 deletions graphql/core/validation/rules/default_values_of_correct_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
69 changes: 56 additions & 13 deletions tests/core_execution/test_variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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))},
Expand Down Expand Up @@ -135,6 +147,7 @@ def test_does_not_use_incorrect_value():
})


# noinspection PyMethodMayBeStatic
class TestUsingVariables:
doc = '''
query q($input: TestInputObject) {
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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']))
}


Expand Down Expand Up @@ -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']))
}


Expand Down Expand Up @@ -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']))
}


Expand Down
Loading

0 comments on commit cf10db6

Please sign in to comment.