Always return unformatted errors and serialize error locations properly

Author: dittos

graphql/core/error.py

@@ -1,4 +1,3 @@
-from .defer import DeferredException
 from .language.location import get_location
 
 
@@ -39,10 +38,10 @@ def locations(self):
 
 
 def format_error(error):
-    if isinstance(error, DeferredException):
-        error = error.value
-
     return {
         'message': error.message,
-        'locations': error.locations,
+        'locations': [
+            {'line': loc.line, 'column': loc.column}
+            for loc in error.locations
+        ],
     }

graphql/core/execution/base.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-
+from ..defer import DeferredException
 from ..error import GraphQLError
 from ..language import ast
 from ..type.definition import (
@@ -68,6 +68,11 @@ class ExecutionResult(object):
 
     def __init__(self, data=None, errors=None, invalid=False):
         self.data = data
+        if errors:
+            errors = [
+                error.value if isinstance(error, DeferredException) else error
+                for error in errors
+            ]
         self.errors = errors
         if invalid:
             assert data is None

graphql/core/execution/executor.py

@@ -2,7 +2,7 @@
 import functools
 
 from ..defer import Deferred, DeferredDict, DeferredList, defer, succeed
-from ..error import GraphQLError, format_error
+from ..error import GraphQLError
 from ..language import ast
 from ..language.parser import parse
 from ..language.source import Source
@@ -71,7 +71,7 @@ def _execute_graphql_query(self, root, ast, operation_name, args, request_contex
             lambda error: ctx.errors.append(error)
         ) \
             .add_callback(
-            lambda data: ExecutionResult(data, list(map(format_error, ctx.errors))),
+            lambda data: ExecutionResult(data, ctx.errors),
         )
 
     def _execute_operation(self, ctx, root, operation, execute_serially):

graphql/core/language/error.py

@@ -1,25 +1,23 @@
-from ..compat import native_str
-from ..error import Error
+from ..error import GraphQLError
 from .location import get_location
 
 __all__ = ['LanguageError']
 
 
-class LanguageError(Error):
+class LanguageError(GraphQLError):
     def __init__(self, source, position, description):
-        self.source = source
-        self.position = position
-        self.description = description
-
-    def __str__(self):
-        location = get_location(self.source, self.position)
-        return native_str(u'Syntax Error {} ({}:{}) {}\n\n{}'.format(
-            self.source.name,
-            location.line,
-            location.column,
-            self.description,
-            highlight_source_at_location(self.source, location),
-        ), errors='replace')
+        location = get_location(source, position)
+        super(LanguageError, self).__init__(
+            message=u'Syntax Error {} ({}:{}) {}\n\n{}'.format(
+                source.name,
+                location.line,
+                location.column,
+                description,
+                highlight_source_at_location(source, location),
+            ),
+            source=source,
+            positions=[position],
+        )
 
 
 def highlight_source_at_location(source, location):

tests/core_execution/test_concurrent_executor.py

@@ -1,3 +1,4 @@
+from graphql.core.error import format_error
 from graphql.core.execution import Executor
 from graphql.core.execution.middlewares.sync import SynchronousExecutionMiddleware
 from graphql.core.defer import succeed, Deferred, fail
@@ -147,9 +148,10 @@ def notPromise(self):
     result = executor.execute(doc, Data(), operation_name='Example')
     assert not isinstance(result, Deferred)
     assert result.data == {"promise": None, 'notPromise': 'i should work'}
-    assert result.errors == [{'locations': [SourceLocation(line=3, column=9)],
-                              'message': 'You cannot return a Deferred from a resolver '
-                                         'when using SynchronousExecutionMiddleware'}]
+    formatted_errors = list(map(format_error, result.errors))
+    assert formatted_errors == [{'locations': [dict(line=3, column=9)],
+                                 'message': 'You cannot return a Deferred from a resolver '
+                                            'when using SynchronousExecutionMiddleware'}]
 
 
 def test_synchronous_executor_doesnt_support_defers():
@@ -176,9 +178,10 @@ def notPromise(self):
     result = executor.execute(doc, Data(), operation_name='Example')
     assert not isinstance(result, Deferred)
     assert result.data is None
-    assert result.errors == [{'locations': [SourceLocation(line=3, column=9)],
-                              'message': 'You cannot return a Deferred from a resolver '
-                                         'when using SynchronousExecutionMiddleware'}]
+    formatted_errors = list(map(format_error, result.errors))
+    assert formatted_errors == [{'locations': [dict(line=3, column=9)],
+                                 'message': 'You cannot return a Deferred from a resolver '
+                                            'when using SynchronousExecutionMiddleware'}]
 
 
 def test_executor_defer_failure():
@@ -206,8 +209,9 @@ def notPromise(self):
     assert result.called
     result = result.result
     assert result.data is None
-    assert result.errors == [{'locations': [SourceLocation(line=3, column=9)],
-                              'message': "Something bad happened! Sucks :("}]
+    formatted_errors = list(map(format_error, result.errors))
+    assert formatted_errors == [{'locations': [dict(line=3, column=9)],
+                                 'message': "Something bad happened! Sucks :("}]
 
 
 def test_synchronous_executor_will_synchronously_resolve():

tests/core_execution/test_executor.py

@@ -231,7 +231,7 @@ def error(self):
     result = execute(GraphQLSchema(Type), Data(), doc_ast)
     assert result.data == {'ok': 'ok', 'error': None}
     assert len(result.errors) == 1
-    assert result.errors[0]['message'] == 'Error getting error'
+    assert result.errors[0].message == 'Error getting error'
     # TODO: check error location
 
 

tests/core_execution/test_gevent.py

@@ -1,5 +1,5 @@
 # flake8: noqa
-
+from graphql.core.error import format_error
 from graphql.core.execution import Executor
 from graphql.core.execution.middlewares.gevent import GeventExecutionMiddleware, run_in_greenlet
 from graphql.core.language.location import SourceLocation
@@ -57,5 +57,6 @@ def resolver_2(context, *_):
 
     executor = Executor(GraphQLSchema(Type), [GeventExecutionMiddleware()])
     result = executor.execute(doc)
-    assert result.errors == [{'locations': [SourceLocation(1, 20)], 'message': 'resolver_2 failed!'}]
+    formatted_errors = list(map(format_error, result.errors))
+    assert formatted_errors == [{'locations': [{'line': 1, 'column': 20}], 'message': 'resolver_2 failed!'}]
     assert result.data == {'a': 'hey', 'b': None}

tests/core_execution/test_lists.py

@@ -50,7 +50,7 @@ def test_non_nullable_list_of_nullables():
     # Returns null
     result = run(type, None)
     assert len(result.errors) == 1
-    assert result.errors[0]['message'] == 'Cannot return null for non-nullable field DataType.test.'
+    assert result.errors[0].message == 'Cannot return null for non-nullable field DataType.test.'
     # TODO: check error location
     assert result.data == {'nest': None}
 
@@ -62,7 +62,7 @@ def test_nullable_list_of_non_nullables():
     # Contains null
     result = run(type, [1, None, 2])
     assert len(result.errors) == 1
-    assert result.errors[0]['message'] == 'Cannot return null for non-nullable field DataType.test.'
+    assert result.errors[0].message == 'Cannot return null for non-nullable field DataType.test.'
     # TODO: check error location
     assert result.data == {'nest': {'test': None}}
     # Returns null
@@ -76,12 +76,12 @@ def test_non_nullable_list_of_non_nullables():
     # Contains null
     result = run(type, [1, None, 2])
     assert len(result.errors) == 1
-    assert result.errors[0]['message'] == 'Cannot return null for non-nullable field DataType.test.'
+    assert result.errors[0].message == 'Cannot return null for non-nullable field DataType.test.'
     # TODO: check error location
     assert result.data == {'nest': None}
     # Returns null
     result = run(type, None)
     assert len(result.errors) == 1
-    assert result.errors[0]['message'] == 'Cannot return null for non-nullable field DataType.test.'
+    assert result.errors[0].message == 'Cannot return null for non-nullable field DataType.test.'
     # TODO: check error location
     assert result.data == {'nest': None}

tests/core_execution/test_mutations.py

@@ -128,5 +128,5 @@ def test_evaluates_mutations_correctly_in_the_presense_of_a_failed_mutation():
         }
     assert len(result.errors) == 2
     # TODO: check error location
-    assert result.errors[0]['message'] == 'Cannot change the number'
-    assert result.errors[1]['message'] == 'Cannot change the number'
+    assert result.errors[0].message == 'Cannot change the number'
+    assert result.errors[1].message == 'Cannot change the number'

tests/core_execution/test_nonnull.py

@@ -54,7 +54,7 @@ def test_nulls_a_nullable_field_that_throws_sync():
     result = execute(schema, ThrowingData(), ast, 'Q', {})
     assert len(result.errors) == 1
     # TODO: check error location
-    assert result.errors[0]['message'] == str(sync_error)
+    assert result.errors[0].message == str(sync_error)
     assert result.data == {
         'sync': None
     }
@@ -72,7 +72,7 @@ def test_nulls_a_sync_returned_object_that_contains_a_non_nullable_field_that_th
     result = execute(schema, ThrowingData(), ast, 'Q', {})
     assert len(result.errors) == 1
     # TODO: check error location
-    assert result.errors[0]['message'] == str(non_null_sync_error)
+    assert result.errors[0].message == str(non_null_sync_error)
     assert result.data == {
         'nest': None
     }
@@ -111,8 +111,8 @@ def test_nulls_a_complex_tree_of_nullable_fields_that_throw():
     result = execute(schema, ThrowingData(), ast, 'Q', {})
     assert len(result.errors) == 2
     # TODO: check error location
-    assert result.errors[0]['message'] == str(sync_error)
-    assert result.errors[1]['message'] == str(sync_error)
+    assert result.errors[0].message == str(sync_error)
+    assert result.errors[1].message == str(sync_error)
     assert result.data == {
         'nest': {
             'sync': None,
@@ -149,7 +149,7 @@ def test_nulls_a_sync_returned_object_that_contains_a_non_nullable_field_that_re
     result = execute(schema, NullingData(), ast, 'Q', {})
     assert len(result.errors) == 1
     # TODO: check error location
-    assert result.errors[0]['message'] == 'Cannot return null for non-nullable field DataType.nonNullSync.'
+    assert result.errors[0].message == 'Cannot return null for non-nullable field DataType.nonNullSync.'
     assert result.data == {
         'nest': None
     }
@@ -206,7 +206,7 @@ def test_nulls_the_top_level_if_sync_non_nullable_field_throws():
     assert result.data is None
     assert len(result.errors) == 1
     # TODO: check error location
-    assert result.errors[0]['message'] == str(non_null_sync_error)
+    assert result.errors[0].message == str(non_null_sync_error)
 
 
 def test_nulls_the_top_level_if_sync_non_nullable_field_returns_null():
@@ -218,4 +218,4 @@ def test_nulls_the_top_level_if_sync_non_nullable_field_returns_null():
     assert result.data is None
     assert len(result.errors) == 1
     # TODO: check error location
-    assert result.errors[0]['message'] == 'Cannot return null for non-nullable field DataType.nonNullSync.'
+    assert result.errors[0].message == 'Cannot return null for non-nullable field DataType.nonNullSync.'