Skip to content

Commit f8fb957

Browse files
odsods
committed
Separate errors to report to user from unexpected exception (issue #209)
1 parent fe703f0 commit f8fb957

15 files changed

+118
-61
lines changed

‎graphql/error/tests/test_base.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from promise import Promise
77

8+
from graphql.error import GraphQLError
89
from graphql.execution import execute
910
from graphql.language.parser import parse
1011
from graphql.type import GraphQLField, GraphQLObjectType, GraphQLSchema, GraphQLString
@@ -22,7 +23,7 @@ def test_raise():
2223

2324
def resolver(context, *_):
2425
# type: (Optional[Any], *ResolveInfo) -> None
25-
raise Exception("Failed")
26+
raise GraphQLError("Failed")
2627

2728
Type = GraphQLObjectType(
2829
"Type", {"a": GraphQLField(GraphQLString, resolver=resolver)}
@@ -38,14 +39,14 @@ def test_reraise():
3839

3940
def resolver(context, *_):
4041
# type: (Optional[Any], *ResolveInfo) -> None
41-
raise Exception("Failed")
42+
raise GraphQLError("Failed")
4243

4344
Type = GraphQLObjectType(
4445
"Type", {"a": GraphQLField(GraphQLString, resolver=resolver)}
4546
)
4647

4748
result = execute(GraphQLSchema(Type), ast)
48-
with pytest.raises(Exception) as exc_info:
49+
with pytest.raises(GraphQLError) as exc_info:
4950
result.errors[0].reraise()
5051

5152
extracted = traceback.extract_tb(exc_info.tb)
@@ -59,7 +60,7 @@ def resolver(context, *_):
5960
"return executor.execute(resolve_fn, source, info, **args)",
6061
),
6162
("execute", "return fn(*args, **kwargs)"),
62-
("resolver", 'raise Exception("Failed")'),
63+
("resolver", 'raise GraphQLError("Failed")'),
6364
]
6465

6566
assert str(exc_info.value) == "Failed"
@@ -71,7 +72,7 @@ def test_reraise_from_promise():
7172
ast = parse("query Example { a }")
7273

7374
def fail():
74-
raise Exception("Failed")
75+
raise GraphQLError("Failed")
7576

7677
def resolver(context, *_):
7778
# type: (Optional[Any], *ResolveInfo) -> None
@@ -95,7 +96,7 @@ def resolver(context, *_):
9596
("test_reraise_from_promise", "result.errors[0].reraise()"),
9697
("_resolve_from_executor", "executor(resolve, reject)"),
9798
("<lambda>", "return Promise(lambda resolve, reject: resolve(fail()))"),
98-
("fail", 'raise Exception("Failed")'),
99+
("fail", 'raise GraphQLError("Failed")'),
99100
]
100101

101102
assert str(exc_info.value) == "Failed"

‎graphql/execution/executor.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,10 @@ def promise_executor(v):
128128

129129
def on_rejected(error):
130130
# type: (Exception) -> None
131-
exe_context.errors.append(error)
132-
return None
131+
if isinstance(error, GraphQLError):
132+
exe_context.errors.append(error)
133+
return None
134+
return Promise.rejected(error)
133135

134136
def on_resolve(data):
135137
# type: (Union[None, Dict, Observable]) -> Union[ExecutionResult, Observable]
@@ -272,9 +274,6 @@ def subscribe_fields(
272274
# type: (...) -> Observable
273275
subscriber_exe_context = SubscriberExecutionContext(exe_context)
274276

275-
def on_error(error):
276-
subscriber_exe_context.report_error(error)
277-
278277
def map_result(data):
279278
# type: (Dict[str, Any]) -> ExecutionResult
280279
if subscriber_exe_context.errors:
@@ -484,14 +483,16 @@ def complete_value_catching_error(
484483

485484
def handle_error(error):
486485
# type: (Union[GraphQLError, GraphQLLocatedError]) -> Optional[Any]
486+
if not isinstance(error, GraphQLError):
487+
return Promise.rejected(error)
487488
traceback = completed._traceback # type: ignore
488489
exe_context.report_error(error, traceback)
489490
return None
490491

491492
return completed.catch(handle_error)
492493

493494
return completed
494-
except Exception as e:
495+
except GraphQLError as e:
495496
traceback = sys.exc_info()[2]
496497
exe_context.report_error(e, traceback)
497498
return None
@@ -535,12 +536,15 @@ def complete_value(
535536
GraphQLLocatedError( # type: ignore
536537
field_asts, original_error=error, path=path
537538
)
539+
if isinstance(error, GraphQLError) else error
538540
),
539541
)
540542

541543
# print return_type, type(result)
542-
if isinstance(result, Exception):
544+
if isinstance(result, GraphQLError):
543545
raise GraphQLLocatedError(field_asts, original_error=result, path=path)
546+
if isinstance(result, Exception):
547+
raise result
544548

545549
if isinstance(return_type, GraphQLNonNull):
546550
return complete_nonnull_value(

‎graphql/execution/tests/test_executor.py

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ def ok(self):
264264

265265
def error(self):
266266
# type: () -> NoReturn
267-
raise Exception("Error getting error")
267+
raise GraphQLError("Error getting error")
268268

269269
doc_ast = parse(doc)
270270

@@ -567,32 +567,58 @@ def test_fails_to_execute_a_query_containing_a_type_definition():
567567
assert isinstance(nodes[0], ObjectTypeDefinition)
568568

569569

570-
def test_exceptions_are_reraised_if_specified(mocker):
571-
# type: (MockFixture) -> None
572-
573-
logger = mocker.patch("graphql.execution.executor.logger")
570+
def test_exceptions_are_reraised():
571+
# type: () -> None
574572

575573
query = parse(
576574
"""
577575
{ foo }
578576
"""
579577
)
580578

579+
class Error(Exception):
580+
pass
581+
581582
def resolver(*_):
582583
# type: (*Any) -> NoReturn
583-
raise Exception("UH OH!")
584+
raise Error("UH OH!")
584585

585586
schema = GraphQLSchema(
586587
GraphQLObjectType(
587588
name="Query", fields={"foo": GraphQLField(GraphQLString, resolver=resolver)}
588589
)
589590
)
590591

591-
execute(schema, query)
592-
logger.exception.assert_called_with(
593-
"An error occurred while resolving field Query.foo"
592+
with raises(Error):
593+
execute(schema, query)
594+
595+
596+
def test_exceptions_are_reraised_promise():
597+
# type: () -> None
598+
599+
query = parse(
600+
"""
601+
{ foo }
602+
"""
594603
)
595604

605+
class Error(Exception):
606+
pass
607+
608+
@Promise.promisify
609+
def resolver(*_):
610+
# type: (*Any) -> NoReturn
611+
raise Error("UH OH!")
612+
613+
schema = GraphQLSchema(
614+
GraphQLObjectType(
615+
name="Query", fields={"foo": GraphQLField(GraphQLString, resolver=resolver)}
616+
)
617+
)
618+
619+
with raises(Error):
620+
execute(schema, query)
621+
596622

597623
def test_executor_properly_propogates_path_data(mocker):
598624
# type: (MockFixture) -> None

‎graphql/execution/tests/test_executor_asyncio.py

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
asyncio = pytest.importorskip("asyncio")
1010

11-
from graphql.error import format_error
11+
from graphql.error import format_error, GraphQLError
1212
from graphql.execution import execute
1313
from graphql.language.parser import parse
1414
from graphql.type import GraphQLField, GraphQLObjectType, GraphQLSchema, GraphQLString
@@ -95,7 +95,7 @@ def resolver(context, *_):
9595
def resolver_2(context, *_):
9696
# type: (Optional[Any], *ResolveInfo) -> NoReturn
9797
asyncio.sleep(0.003)
98-
raise Exception("resolver_2 failed!")
98+
raise GraphQLError("resolver_2 failed!")
9999

100100
Type = GraphQLObjectType(
101101
"Type",
@@ -117,6 +117,28 @@ def resolver_2(context, *_):
117117
assert result.data == {"a": "hey", "b": None}
118118

119119

120+
def test_asyncio_executor_exceptions_reraised():
121+
# type: () -> None
122+
ast = parse("query Example { a }")
123+
124+
class Error(Exception):
125+
pass
126+
127+
def resolver(context, *_):
128+
# type: (Optional[Any], *ResolveInfo) -> str
129+
raise Error("UH OH!")
130+
131+
Type = GraphQLObjectType(
132+
"Type",
133+
{
134+
"a": GraphQLField(GraphQLString, resolver=resolver),
135+
},
136+
)
137+
138+
with pytest.raises(Error):
139+
execute(GraphQLSchema(Type), ast, executor=AsyncioExecutor())
140+
141+
120142
def test_evaluates_mutations_serially():
121143
# type: () -> None
122144
assert_evaluate_mutations_serially(executor=AsyncioExecutor())

‎graphql/execution/tests/test_executor_gevent.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
gevent = pytest.importorskip("gevent")
1010

11-
from graphql.error import format_error
11+
from graphql.error import format_error, GraphQLError
1212
from graphql.execution import execute
1313
from graphql.language.location import SourceLocation
1414
from graphql.language.parser import parse
@@ -54,7 +54,7 @@ def resolver(context, *_):
5454

5555
def resolver_2(context, *_):
5656
gevent.sleep(0.003)
57-
raise Exception("resolver_2 failed!")
57+
raise GraphQLError("resolver_2 failed!")
5858

5959
Type = GraphQLObjectType(
6060
"Type",

‎graphql/execution/tests/test_executor_thread.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# type: ignore
2-
from graphql.error import format_error
2+
from graphql.error import format_error, GraphQLError
33
from graphql.execution import execute
44
from graphql.language.parser import parse
55
from graphql.type import (
@@ -191,19 +191,19 @@ def sync(self):
191191

192192
def syncError(self):
193193
# type: () -> NoReturn
194-
raise Exception("Error getting syncError")
194+
raise GraphQLError("Error getting syncError")
195195

196196
def syncReturnError(self):
197197
# type: () -> Exception
198-
return Exception("Error getting syncReturnError")
198+
return GraphQLError("Error getting syncReturnError")
199199

200200
def syncReturnErrorList(self):
201201
# type: () -> List[Union[Exception, str]]
202202
return [
203203
"sync0",
204-
Exception("Error getting syncReturnErrorList1"),
204+
GraphQLError("Error getting syncReturnErrorList1"),
205205
"sync2",
206-
Exception("Error getting syncReturnErrorList3"),
206+
GraphQLError("Error getting syncReturnErrorList3"),
207207
]
208208

209209
def asyncBasic(self):
@@ -212,15 +212,15 @@ def asyncBasic(self):
212212

213213
def asyncReject(self):
214214
# type: () -> Promise
215-
return rejected(Exception("Error getting asyncReject"))
215+
return rejected(GraphQLError("Error getting asyncReject"))
216216

217217
def asyncEmptyReject(self):
218218
# type: () -> Promise
219-
return rejected(Exception("An unknown error occurred."))
219+
return rejected(GraphQLError("An unknown error occurred."))
220220

221221
def asyncReturnError(self):
222222
# type: () -> Promise
223-
return resolved(Exception("Error getting asyncReturnError"))
223+
return resolved(GraphQLError("Error getting asyncReturnError"))
224224

225225
schema = GraphQLSchema(
226226
query=GraphQLObjectType(

‎graphql/execution/tests/test_lists.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# type: ignore
22
from collections import namedtuple
33

4-
from graphql.error import format_error
4+
from graphql.error import format_error, GraphQLError
55
from graphql.execution import execute
66
from graphql.language.parser import parse
77
from graphql.type import (
@@ -66,7 +66,7 @@ class Test_ListOfT_Promise_Array_T: # [T] Promise<Array<T>>
6666
)
6767
test_returns_null = check(resolved(None), {"data": {"nest": {"test": None}}})
6868
test_rejected = check(
69-
lambda: rejected(Exception("bad")),
69+
lambda: rejected(GraphQLError("bad")),
7070
{
7171
"data": {"nest": {"test": None}},
7272
"errors": [
@@ -91,7 +91,7 @@ class Test_ListOfT_Array_Promise_T: # [T] Array<Promise<T>>
9191
{"data": {"nest": {"test": [1, None, 2]}}},
9292
)
9393
test_contains_reject = check(
94-
lambda: [resolved(1), rejected(Exception("bad")), resolved(2)],
94+
lambda: [resolved(1), rejected(GraphQLError("bad")), resolved(2)],
9595
{
9696
"data": {"nest": {"test": [1, None, 2]}},
9797
"errors": [
@@ -149,7 +149,7 @@ class Test_NotNullListOfT_Promise_Array_T: # [T]! Promise<Array<T>>>
149149
)
150150

151151
test_rejected = check(
152-
lambda: rejected(Exception("bad")),
152+
lambda: rejected(GraphQLError("bad")),
153153
{
154154
"data": {"nest": None},
155155
"errors": [
@@ -173,7 +173,7 @@ class Test_NotNullListOfT_Array_Promise_T: # [T]! Promise<Array<T>>>
173173
{"data": {"nest": {"test": [1, None, 2]}}},
174174
)
175175
test_contains_reject = check(
176-
lambda: [resolved(1), rejected(Exception("bad")), resolved(2)],
176+
lambda: [resolved(1), rejected(GraphQLError("bad")), resolved(2)],
177177
{
178178
"data": {"nest": {"test": [1, None, 2]}},
179179
"errors": [
@@ -228,7 +228,7 @@ class TestListOfNotNullT_Promise_Array_T: # [T!] Promise<Array<T>>
228228
test_returns_null = check(resolved(None), {"data": {"nest": {"test": None}}})
229229

230230
test_rejected = check(
231-
lambda: rejected(Exception("bad")),
231+
lambda: rejected(GraphQLError("bad")),
232232
{
233233
"data": {"nest": {"test": None}},
234234
"errors": [
@@ -262,7 +262,7 @@ class TestListOfNotNullT_Array_Promise_T: # [T!] Array<Promise<T>>
262262
},
263263
)
264264
test_contains_reject = check(
265-
lambda: [resolved(1), rejected(Exception("bad")), resolved(2)],
265+
lambda: [resolved(1), rejected(GraphQLError("bad")), resolved(2)],
266266
{
267267
"data": {"nest": {"test": None}},
268268
"errors": [
@@ -341,7 +341,7 @@ class TestNotNullListOfNotNullT_Promise_Array_T: # [T!]! Promise<Array<T>>
341341
)
342342

343343
test_rejected = check(
344-
lambda: rejected(Exception("bad")),
344+
lambda: rejected(GraphQLError("bad")),
345345
{
346346
"data": {"nest": None},
347347
"errors": [
@@ -375,7 +375,7 @@ class TestNotNullListOfNotNullT_Array_Promise_T: # [T!]! Array<Promise<T>>
375375
},
376376
)
377377
test_contains_reject = check(
378-
lambda: [resolved(1), rejected(Exception("bad")), resolved(2)],
378+
lambda: [resolved(1), rejected(GraphQLError("bad")), resolved(2)],
379379
{
380380
"data": {"nest": None},
381381
"errors": [

0 commit comments

Comments
 (0)