Skip to content

Commit ea4463e

Browse files
syrusakbarysyrusakbary
committed
Improved resolver catch logic
1 parent 8ca8428 commit ea4463e

File tree

4 files changed

+141
-80
lines changed

4 files changed

+141
-80
lines changed

‎graphql/execution/executor.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import logging
44
import sys
55

6-
from promise import Promise, promise_for_dict, promisify
6+
from promise import Promise, promise_for_dict
77

88
from ..error import GraphQLError, GraphQLLocatedError
99
from ..pyutils.default_ordered_dict import DefaultOrderedDict
@@ -102,7 +102,7 @@ def collect_result(resolved_result):
102102
results[response_name] = resolved_result
103103
return results
104104

105-
return promisify(result).then(collect_result, None)
105+
return result.then(collect_result, None)
106106

107107
results[response_name] = result
108108
return results
@@ -206,9 +206,9 @@ def complete_value_catching_error(exe_context, return_type, field_asts, info, re
206206
if is_promise(completed):
207207
def handle_error(error):
208208
exe_context.errors.append(error)
209-
return Promise.fulfilled(None)
209+
return None
210210

211-
return promisify(completed).then(None, handle_error)
211+
return completed.catch(handle_error)
212212

213213
return completed
214214
except Exception as e:
@@ -238,7 +238,7 @@ def complete_value(exe_context, return_type, field_asts, info, result):
238238
# If field type is NonNull, complete for inner type, and throw field error if result is null.
239239

240240
if is_promise(result):
241-
return promisify(result).then(
241+
return result.then(
242242
lambda resolved: complete_value(
243243
exe_context,
244244
return_type,

‎graphql/execution/querybuilder/resolver.py

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import sys
2+
import traceback
13
import collections
24
from functools import partial
35

6+
from ..base import default_resolve_fn
47
from ...error import GraphQLError, GraphQLLocatedError
58
from ...type import (GraphQLEnumType, GraphQLInterfaceType, GraphQLList,
69
GraphQLNonNull, GraphQLObjectType, GraphQLScalarType,
@@ -14,15 +17,26 @@ def is_promise(value):
1417
return type(value) == Promise
1518

1619

17-
def on_complete_resolver(__func, exe_context, info, __resolver, *args, **kwargs):
20+
def on_complete_resolver(catch_error, __func, exe_context, info, __resolver, *args, **kwargs):
1821
try:
1922
result = __resolver(*args, **kwargs)
2023
except Exception, e:
21-
exe_context.errors.append(e)
22-
return None
24+
# print e, __resolver, info.field_name, traceback.print_tb(sys.exc_info()[2])
25+
error = GraphQLLocatedError(info.field_asts, original_error=e)
26+
if catch_error:
27+
exe_context.errors.append(error)
28+
return None
29+
raise error
2330

2431
if is_promise(result):
25-
return result.then(__func)
32+
def on_error(e):
33+
error = GraphQLLocatedError(info.field_asts, original_error=e)
34+
if catch_error:
35+
exe_context.errors.append(error)
36+
return None
37+
raise error
38+
39+
return result.then(__func, on_error)
2640
return __func(result)
2741

2842

@@ -36,11 +50,7 @@ def complete_list_value(inner_resolver, exe_context, info, result):
3650

3751
completed_results = []
3852
for item in result:
39-
try:
40-
completed_item = inner_resolver(item)
41-
except Exception, e:
42-
completed_item = None
43-
exe_context.errors.append(e)
53+
completed_item = inner_resolver(item)
4454
completed_results.append(completed_item)
4555

4656
return completed_results
@@ -63,45 +73,60 @@ def complete_object_value(fragment_resolve, result):
6373

6474

6575
def field_resolver(field, fragment=None, exe_context=None, info=None):
66-
return type_resolver(field.type, field.resolver, fragment, exe_context, info)
76+
return type_resolver(field.type, field.resolver or default_resolve_fn, fragment, exe_context, info, catch_error=True)
77+
# def new_resolver(*args, **kwargs):
78+
# try:
79+
# result = resolver(*args, **kwargs)
80+
# except Exception, e:
81+
# exe_context.errors.append(e)
82+
83+
# return new_resolver
6784

6885

69-
def type_resolver(return_type, resolver, fragment=None, exe_context=None, info=None):
86+
def type_resolver(return_type, resolver, fragment=None, exe_context=None, info=None, catch_error=False):
7087
if isinstance(return_type, GraphQLNonNull):
7188
return type_resolver_non_null(return_type, resolver, fragment, exe_context, info)
7289

7390
if isinstance(return_type, (GraphQLScalarType, GraphQLEnumType)):
74-
return type_resolver_leaf(return_type, resolver, exe_context, info)
91+
return type_resolver_leaf(return_type, resolver, exe_context, info, catch_error)
7592

7693
if isinstance(return_type, (GraphQLList)):
77-
return type_resolver_list(return_type, resolver, fragment, exe_context, info)
94+
return type_resolver_list(return_type, resolver, fragment, exe_context, info, catch_error)
7895

7996
if isinstance(return_type, (GraphQLObjectType)):
8097
assert fragment and fragment.type == return_type, 'Fragment and return_type dont match'
81-
complete_object_value_resolve = partial(complete_object_value, fragment.resolve)
82-
return partial(on_complete_resolver, complete_object_value_resolve, exe_context, info, resolver)
98+
return type_resolver_type(return_type, resolver, fragment, exe_context, info, catch_error)
99+
# complete_object_value_resolve = partial(complete_object_value, fragment.resolve)
100+
# return partial(on_complete_resolver, complete_object_value_resolve, exe_context, info, resolver)
83101
# return partial(fragment.resolver, resolver)
84102

85103
if isinstance(return_type, (GraphQLInterfaceType, GraphQLUnionType)):
86104
assert fragment, 'You need to pass a fragment to resolve a Interface or Union'
87-
return partial(on_complete_resolver, fragment.resolve, exe_context, info, resolver)
105+
return type_resolver_type(return_type, resolver, fragment, exe_context, info, catch_error)
106+
# return partial(on_complete_resolver, fragment.resolve, exe_context, info, resolver)
88107
# return partial(fragment.resolver, resolver)
89108
# return partial(fragment.abstract_resolver, resolver, return_type)
90109

91110
raise Exception("The resolver have to be created for a fragment")
92111

93112

94-
def type_resolver_non_null(return_type, resolver, fragment, exe_context, info):
113+
def type_resolver_type(return_type, resolver, fragment, exe_context, info, catch_error):
114+
complete_object_value_resolve = partial(complete_object_value, fragment.resolve)
115+
return partial(on_complete_resolver, catch_error, complete_object_value_resolve, exe_context, info, resolver)
116+
117+
118+
def type_resolver_non_null(return_type, resolver, fragment, exe_context, info): # no catch_error
95119
resolver = type_resolver(return_type.of_type, resolver, fragment, exe_context, info)
96120
nonnull_complete = partial(complete_nonnull_value, exe_context, info)
97-
return partial(on_complete_resolver, nonnull_complete, exe_context, info, resolver)
121+
return partial(on_complete_resolver, False, nonnull_complete, exe_context, info, resolver)
98122

99123

100-
def type_resolver_leaf(return_type, resolver, exe_context, info):
101-
return partial(on_complete_resolver, return_type.serialize, exe_context, info, resolver)
124+
def type_resolver_leaf(return_type, resolver, exe_context, info, catch_error):
125+
return partial(on_complete_resolver, catch_error, return_type.serialize, exe_context, info, resolver)
102126

103127

104-
def type_resolver_list(return_type, resolver, fragment, exe_context, info):
105-
inner_resolver = type_resolver(return_type.of_type, lambda item: item, fragment, exe_context, info)
128+
def type_resolver_list(return_type, resolver, fragment, exe_context, info, catch_error):
129+
item_type = return_type.of_type
130+
inner_resolver = type_resolver(item_type, lambda item: item, fragment, exe_context, info)
106131
list_complete = partial(complete_list_value, inner_resolver, exe_context, info)
107-
return partial(on_complete_resolver, list_complete, exe_context, info, resolver)
132+
return partial(on_complete_resolver, catch_error, list_complete, exe_context, info, resolver)

‎graphql/execution/querybuilder/tests/test_nonnull.py

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -189,57 +189,57 @@ def test_nulls_a_sync_returned_object_that_contains_a_non_nullable_field_that_th
189189
# })
190190

191191

192-
# def test_nulls_a_complex_tree_of_nullable_fields_that_throw():
193-
# doc = '''
194-
# query Q {
195-
# nest {
196-
# sync
197-
# promise
198-
# nest {
199-
# sync
200-
# promise
201-
# }
202-
# promiseNest {
203-
# sync
204-
# promise
205-
# }
206-
# }
207-
# promiseNest {
208-
# sync
209-
# promise
210-
# nest {
211-
# sync
212-
# promise
213-
# }
214-
# promiseNest {
215-
# sync
216-
# promise
217-
# }
218-
# }
219-
# }
220-
# '''
221-
# check(doc, ThrowingData(), {
222-
# 'data': {'nest': {'nest': {'promise': None, 'sync': None},
223-
# 'promise': None,
224-
# 'promiseNest': {'promise': None, 'sync': None},
225-
# 'sync': None},
226-
# 'promiseNest': {'nest': {'promise': None, 'sync': None},
227-
# 'promise': None,
228-
# 'promiseNest': {'promise': None, 'sync': None},
229-
# 'sync': None}},
230-
# 'errors': [{'locations': [{'column': 11, 'line': 4}], 'message': str(sync_error)},
231-
# {'locations': [{'column': 11, 'line': 5}], 'message': str(promise_error)},
232-
# {'locations': [{'column': 13, 'line': 7}], 'message': str(sync_error)},
233-
# {'locations': [{'column': 13, 'line': 8}], 'message': str(promise_error)},
234-
# {'locations': [{'column': 13, 'line': 11}], 'message': str(sync_error)},
235-
# {'locations': [{'column': 13, 'line': 12}], 'message': str(promise_error)},
236-
# {'locations': [{'column': 11, 'line': 16}], 'message': str(sync_error)},
237-
# {'locations': [{'column': 11, 'line': 17}], 'message': str(promise_error)},
238-
# {'locations': [{'column': 13, 'line': 19}], 'message': str(sync_error)},
239-
# {'locations': [{'column': 13, 'line': 20}], 'message': str(promise_error)},
240-
# {'locations': [{'column': 13, 'line': 23}], 'message': str(sync_error)},
241-
# {'locations': [{'column': 13, 'line': 24}], 'message': str(promise_error)}]
242-
# })
192+
def test_nulls_a_complex_tree_of_nullable_fields_that_throw():
193+
doc = '''
194+
query Q {
195+
nest {
196+
sync
197+
promise
198+
nest {
199+
sync
200+
promise
201+
}
202+
promiseNest {
203+
sync
204+
promise
205+
}
206+
}
207+
promiseNest {
208+
sync
209+
promise
210+
nest {
211+
sync
212+
promise
213+
}
214+
promiseNest {
215+
sync
216+
promise
217+
}
218+
}
219+
}
220+
'''
221+
check(doc, ThrowingData(), {
222+
'data': {'nest': {'nest': {'promise': None, 'sync': None},
223+
'promise': None,
224+
'promiseNest': {'promise': None, 'sync': None},
225+
'sync': None},
226+
'promiseNest': {'nest': {'promise': None, 'sync': None},
227+
'promise': None,
228+
'promiseNest': {'promise': None, 'sync': None},
229+
'sync': None}},
230+
'errors': [{'locations': [{'column': 11, 'line': 4}], 'message': str(sync_error)},
231+
{'locations': [{'column': 11, 'line': 5}], 'message': str(promise_error)},
232+
{'locations': [{'column': 13, 'line': 7}], 'message': str(sync_error)},
233+
{'locations': [{'column': 13, 'line': 8}], 'message': str(promise_error)},
234+
{'locations': [{'column': 13, 'line': 11}], 'message': str(sync_error)},
235+
{'locations': [{'column': 13, 'line': 12}], 'message': str(promise_error)},
236+
{'locations': [{'column': 11, 'line': 16}], 'message': str(sync_error)},
237+
{'locations': [{'column': 11, 'line': 17}], 'message': str(promise_error)},
238+
{'locations': [{'column': 13, 'line': 19}], 'message': str(sync_error)},
239+
{'locations': [{'column': 13, 'line': 20}], 'message': str(promise_error)},
240+
{'locations': [{'column': 13, 'line': 23}], 'message': str(sync_error)},
241+
{'locations': [{'column': 13, 'line': 24}], 'message': str(promise_error)}]
242+
})
243243

244244

245245
# def test_nulls_the_first_nullable_object_after_a_field_throws_in_a_long_chain_of_fields_that_are_non_null():

‎graphql/execution/querybuilder/tests/test_resolver.py

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import pytest
2+
import mock
3+
4+
from ....error import GraphQLError, GraphQLLocatedError
25

36
from ....language import ast
47
from ....type import (GraphQLEnumType, GraphQLInterfaceType, GraphQLList,
58
GraphQLNonNull, GraphQLObjectType, GraphQLScalarType,
69
GraphQLSchema, GraphQLUnionType, GraphQLString, GraphQLInt, GraphQLField)
7-
from ..resolver import type_resolver
10+
from ..resolver import type_resolver, field_resolver
811

912
from promise import Promise
1013

@@ -44,3 +47,36 @@ def test_type_resolver_promise(type, value, expected):
4447
assert resolved_promise.is_fulfilled
4548
resolved = resolved_promise.get()
4649
assert resolved == expected
50+
51+
52+
def raises():
53+
raise Exception("raises")
54+
55+
56+
def test_resolver_exception():
57+
info = mock.MagicMock()
58+
with pytest.raises(GraphQLLocatedError):
59+
resolver = type_resolver(GraphQLString, raises, info=info)
60+
resolver()
61+
62+
63+
def test_field_resolver_mask_exception():
64+
info = mock.MagicMock()
65+
exe_context = mock.MagicMock()
66+
exe_context.errors = []
67+
field = GraphQLField(GraphQLString, resolver=raises)
68+
resolver = field_resolver(field, info=info, exe_context=exe_context)
69+
resolved = resolver()
70+
assert resolved == None
71+
assert len(exe_context.errors) == 1
72+
assert str(exe_context.errors[0]) == 'raises'
73+
74+
75+
# def test_nonnull_field_resolver_mask_exception():
76+
# info = mock.MagicMock()
77+
# exe_context = mock.MagicMock()
78+
# exe_context.errors = []
79+
# field = GraphQLField(GraphQLNonNull(GraphQLString), resolver=raises)
80+
# resolver = field_resolver(field, info=info, exe_context=exe_context)
81+
# with pytest.raises(GraphQLLocatedError):
82+
# resolver()

0 commit comments

Comments
 (0)