Skip to content

Commit 456d2f3

Browse files
odsods
committed
Separate errors to report to user from unexpected exception (issue graphql-python#209)
1 parent dd7fcde commit 456d2f3

15 files changed

+119
-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]
@@ -271,9 +273,6 @@ def subscribe_fields(
271273
# type: (...) -> Observable
272274
subscriber_exe_context = SubscriberExecutionContext(exe_context)
273275

274-
def on_error(error):
275-
subscriber_exe_context.report_error(error)
276-
277276
def map_result(data):
278277
# type: (Dict[str, Any]) -> ExecutionResult
279278
if subscriber_exe_context.errors:
@@ -482,14 +481,16 @@ def complete_value_catching_error(
482481

483482
def handle_error(error):
484483
# type: (Union[GraphQLError, GraphQLLocatedError]) -> Optional[Any]
484+
if not isinstance(error, GraphQLError):
485+
return Promise.rejected(error)
485486
traceback = completed._traceback # type: ignore
486487
exe_context.report_error(error, traceback)
487488
return None
488489

489490
return completed.catch(handle_error)
490491

491492
return completed
492-
except Exception as e:
493+
except GraphQLError as e:
493494
traceback = sys.exc_info()[2]
494495
exe_context.report_error(e, traceback)
495496
return None
@@ -531,12 +532,15 @@ def complete_value(
531532
),
532533
lambda error: Promise.rejected(
533534
GraphQLLocatedError(field_asts, original_error=error, path=path)
535+
if isinstance(error, GraphQLError) else error
534536
),
535537
)
536538

537539
# print return_type, type(result)
538-
if isinstance(result, Exception):
540+
if isinstance(result, GraphQLError):
539541
raise GraphQLLocatedError(field_asts, original_error=result, path=path)
542+
if isinstance(result, Exception):
543+
raise result
540544

541545
if isinstance(return_type, GraphQLNonNull):
542546
return complete_nonnull_value(

‎graphql/execution/tests/test_executor.py

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

264264
def error(self):
265265
# type: () -> NoReturn
266-
raise Exception("Error getting error")
266+
raise GraphQLError("Error getting error")
267267

268268
doc_ast = parse(doc)
269269

@@ -559,32 +559,58 @@ def test_fails_to_execute_a_query_containing_a_type_definition():
559559
)
560560

561561

562-
def test_exceptions_are_reraised_if_specified(mocker):
563-
# type: (MockFixture) -> None
564-
565-
logger = mocker.patch("graphql.execution.executor.logger")
562+
def test_exceptions_are_reraised():
563+
# type: () -> None
566564

567565
query = parse(
568566
"""
569567
{ foo }
570568
"""
571569
)
572570

571+
class Error(Exception):
572+
pass
573+
573574
def resolver(*_):
574575
# type: (*Any) -> NoReturn
575-
raise Exception("UH OH!")
576+
raise Error("UH OH!")
576577

577578
schema = GraphQLSchema(
578579
GraphQLObjectType(
579580
name="Query", fields={"foo": GraphQLField(GraphQLString, resolver=resolver)}
580581
)
581582
)
582583

583-
execute(schema, query)
584-
logger.exception.assert_called_with(
585-
"An error occurred while resolving field Query.foo"
584+
with raises(Error):
585+
execute(schema, query)
586+
587+
588+
def test_exceptions_are_reraised_promise():
589+
# type: () -> None
590+
591+
query = parse(
592+
"""
593+
{ foo }
594+
"""
586595
)
587596

597+
class Error(Exception):
598+
pass
599+
600+
@Promise.promisify
601+
def resolver(*_):
602+
# type: (*Any) -> NoReturn
603+
raise Error("UH OH!")
604+
605+
schema = GraphQLSchema(
606+
GraphQLObjectType(
607+
name="Query", fields={"foo": GraphQLField(GraphQLString, resolver=resolver)}
608+
)
609+
)
610+
611+
with raises(Error):
612+
execute(schema, query)
613+
588614

589615
def test_executor_properly_propogates_path_data(mocker):
590616
# 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 (
@@ -181,19 +181,19 @@ def sync(self):
181181

182182
def syncError(self):
183183
# type: () -> NoReturn
184-
raise Exception("Error getting syncError")
184+
raise GraphQLError("Error getting syncError")
185185

186186
def syncReturnError(self):
187187
# type: () -> Exception
188-
return Exception("Error getting syncReturnError")
188+
return GraphQLError("Error getting syncReturnError")
189189

190190
def syncReturnErrorList(self):
191191
# type: () -> List[Union[Exception, str]]
192192
return [
193193
"sync0",
194-
Exception("Error getting syncReturnErrorList1"),
194+
GraphQLError("Error getting syncReturnErrorList1"),
195195
"sync2",
196-
Exception("Error getting syncReturnErrorList3"),
196+
GraphQLError("Error getting syncReturnErrorList3"),
197197
]
198198

199199
def asyncBasic(self):
@@ -202,15 +202,15 @@ def asyncBasic(self):
202202

203203
def asyncReject(self):
204204
# type: () -> Promise
205-
return rejected(Exception("Error getting asyncReject"))
205+
return rejected(GraphQLError("Error getting asyncReject"))
206206

207207
def asyncEmptyReject(self):
208208
# type: () -> Promise
209-
return rejected(Exception("An unknown error occurred."))
209+
return rejected(GraphQLError("An unknown error occurred."))
210210

211211
def asyncReturnError(self):
212212
# type: () -> Promise
213-
return resolved(Exception("Error getting asyncReturnError"))
213+
return resolved(GraphQLError("Error getting asyncReturnError"))
214214

215215
schema = GraphQLSchema(
216216
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)