Implement additional tests to ensure that returning exception will properly null a field and log error.

Author: jhgg

graphql/core/error.py

@@ -10,7 +10,7 @@ class GraphQLError(Error):
 
     def __init__(self, message, nodes=None, stack=None, source=None, positions=None):
         super(GraphQLError, self).__init__(message)
-        self.message = message
+        self.message = message or 'An unknown error occurred.'
         self.nodes = nodes
         self.stack = stack or message
         self._source = source

graphql/core/execution/executor.py

@@ -169,7 +169,7 @@ def _resolve_field(self, execution_context, parent_type, source, field_asts):
             execution_context
         )
 
-        result = self.run_resolve_fn(resolve_fn, source, args, info)
+        result = self.resolve_or_error(resolve_fn, source, args, info)
         return self.complete_value_catching_error(
             execution_context, return_type, field_asts, info, result
         )
@@ -223,7 +223,7 @@ def complete_value(self, ctx, return_type, field_asts, info, result):
                     info,
                     resolved
                 ),
-                lambda error: GraphQLError(str(error.value), field_asts, error)
+                lambda error: GraphQLError(error.value and str(error.value), field_asts, error)
             )
 
         if isinstance(result, Exception):
@@ -307,7 +307,7 @@ def complete_value(self, ctx, return_type, field_asts, info, result):
 
         return self._execute_fields(ctx, runtime_type, result, subfield_asts)
 
-    def run_resolve_fn(self, resolve_fn, source, args, info):
+    def resolve_or_error(self, resolve_fn, source, args, info):
         curried_resolve_fn = functools.partial(resolve_fn, source, args, info)
 
         try:

tests/core_execution/test_concurrent_executor.py

@@ -3,11 +3,9 @@
 from graphql.core.execution import Executor
 from graphql.core.execution.middlewares.sync import SynchronousExecutionMiddleware
 from graphql.core.pyutils.defer import succeed, Deferred, fail
-from graphql.core.language.parser import parse
 from graphql.core.type import (GraphQLSchema, GraphQLObjectType, GraphQLField,
                                GraphQLArgument, GraphQLList, GraphQLInt, GraphQLString)
 from graphql.core.type.definition import GraphQLNonNull
-
 from .utils import raise_callback_results
 
 
@@ -69,7 +67,6 @@ def deeper(self):
         }
     '''
 
-    ast = parse(doc)
     expected = {
         'a': 'Apple',
         'b': 'Banana',
@@ -236,6 +233,92 @@ def promise(self):
     assert not result.errors
 
 
+def test_synchronous_error_nulls_out_error_subtrees():
+    doc = '''
+    {
+        sync
+        syncError
+        syncReturnError
+        syncReturnErrorList
+        async
+        asyncReject
+        asyncEmptyReject
+        asyncReturnError
+    }
+    '''
+
+    class Data:
+        def sync(self):
+            return 'sync'
+
+        def syncError(self):
+            raise Exception('Error getting syncError')
+
+        def syncReturnError(self):
+            return Exception("Error getting syncReturnError")
+
+        def syncReturnErrorList(self):
+            return [
+                'sync0',
+                Exception('Error getting syncReturnErrorList1'),
+                'sync2',
+                Exception('Error getting syncReturnErrorList3')
+            ]
+
+        def async(self):
+            return succeed('async')
+
+        def asyncReject(self):
+            return fail(Exception('Error getting asyncReject'))
+
+        def asyncEmptyReject(self):
+            return fail()
+
+        def asyncReturnError(self):
+            return succeed(Exception('Error getting asyncReturnError'))
+
+    schema = GraphQLSchema(
+        query=GraphQLObjectType(
+            name='Type',
+            fields={
+                'sync': GraphQLField(GraphQLString),
+                'syncError': GraphQLField(GraphQLString),
+                'syncReturnError': GraphQLField(GraphQLString),
+                'syncReturnErrorList': GraphQLField(GraphQLList(GraphQLString)),
+                'async': GraphQLField(GraphQLString),
+                'asyncReject': GraphQLField(GraphQLString),
+                'asyncEmptyReject': GraphQLField(GraphQLString),
+                'asyncReturnError': GraphQLField(GraphQLString),
+            }
+        )
+    )
+
+    executor = Executor(map_type=OrderedDict)
+
+    def handle_results(result):
+        assert result.data == {
+            'async': 'async',
+            'asyncEmptyReject': None,
+            'asyncReject': None,
+            'asyncReturnError': None,
+            'sync': 'sync',
+            'syncError': None,
+            'syncReturnError': None,
+            'syncReturnErrorList': ['sync0', None, 'sync2', None]
+        }
+        assert list(map(format_error, result.errors)) == [
+            {'locations': [{'line': 4, 'column': 9}], 'message': 'Error getting syncError'},
+            {'locations': [{'line': 5, 'column': 9}], 'message': 'Error getting syncReturnError'},
+            {'locations': [{'line': 6, 'column': 9}], 'message': 'Error getting syncReturnErrorList1'},
+            {'locations': [{'line': 6, 'column': 9}], 'message': 'Error getting syncReturnErrorList3'},
+            {'locations': [{'line': 8, 'column': 9}], 'message': 'Error getting asyncReject'},
+            {'locations': [{'line': 9, 'column': 9}], 'message': 'An unknown error occurred.'},
+            {'locations': [{'line': 10, 'column': 9}], 'message': 'Error getting asyncReturnError'}
+        ]
+
+    raise_callback_results(executor.execute(schema, doc, Data()), handle_results)
+
+
 def test_executor_can_enforce_strict_ordering():
     Type = GraphQLObjectType('Type', lambda: {
         'a': GraphQLField(GraphQLString,