Skip to content

Commit f81ca08

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

15 files changed

+118
-60
lines changed

‎graphql/error/tests/test_base.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import pytest
22
import traceback
33

4+
from graphql.error import GraphQLError
45
from graphql.execution import execute
56
from graphql.language.parser import parse
67
from graphql.type import GraphQLField, GraphQLObjectType, GraphQLSchema, GraphQLString
@@ -18,7 +19,7 @@ def test_raise():
1819

1920
def resolver(context, *_):
2021
# type: (Optional[Any], *ResolveInfo) -> None
21-
raise Exception("Failed")
22+
raise GraphQLError("Failed")
2223

2324
Type = GraphQLObjectType(
2425
"Type", {"a": GraphQLField(GraphQLString, resolver=resolver)}
@@ -34,14 +35,14 @@ def test_reraise():
3435

3536
def resolver(context, *_):
3637
# type: (Optional[Any], *ResolveInfo) -> None
37-
raise Exception("Failed")
38+
raise GraphQLError("Failed")
3839

3940
Type = GraphQLObjectType(
4041
"Type", {"a": GraphQLField(GraphQLString, resolver=resolver)}
4142
)
4243

4344
result = execute(GraphQLSchema(Type), ast)
44-
with pytest.raises(Exception) as exc_info:
45+
with pytest.raises(GraphQLError) as exc_info:
4546
result.errors[0].reraise()
4647

4748
extracted = traceback.extract_tb(exc_info.tb)
@@ -58,7 +59,7 @@ def resolver(context, *_):
5859
"return executor.execute(resolve_fn, source, info, **args)",
5960
),
6061
("execute", "return fn(*args, **kwargs)"),
61-
("resolver", 'raise Exception("Failed")'),
62+
("resolver", 'raise GraphQLError("Failed")'),
6263
]
6364
# assert formatted_tb == [
6465
# ('test_reraise', 'result.errors[0].reraise()'),
@@ -67,7 +68,7 @@ def resolver(context, *_):
6768
# # ('reraise', 'raise value.with_traceback(tb)'),
6869
# # ('resolve_or_error', 'return executor.execute(resolve_fn, source, info, **args)'),
6970
# # ('execute', 'return fn(*args, **kwargs)'),
70-
# ('resolver', "raise Exception('Failed')")
71+
# ('resolver', "raise GraphQLError('Failed')")
7172
# ]
7273

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

‎graphql/execution/executor.py

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

126126
def on_rejected(error):
127127
# type: (Exception) -> None
128-
exe_context.errors.append(error)
129-
return None
128+
if isinstance(error, GraphQLError):
129+
exe_context.errors.append(error)
130+
return None
131+
return Promise.rejected(error)
130132

131133
def on_resolve(data):
132134
# type: (Union[None, Dict, Observable]) -> Union[ExecutionResult, Observable]
@@ -268,9 +270,6 @@ def subscribe_fields(
268270
# type: (...) -> Observable
269271
subscriber_exe_context = SubscriberExecutionContext(exe_context)
270272

271-
def on_error(error):
272-
subscriber_exe_context.report_error(error)
273-
274273
def map_result(data):
275274
# type: (Dict[str, Any]) -> ExecutionResult
276275
if subscriber_exe_context.errors:
@@ -479,14 +478,16 @@ def complete_value_catching_error(
479478

480479
def handle_error(error):
481480
# type: (Union[GraphQLError, GraphQLLocatedError]) -> Optional[Any]
481+
if not isinstance(error, GraphQLError):
482+
return Promise.rejected(error)
482483
traceback = completed._traceback # type: ignore
483484
exe_context.report_error(error, traceback)
484485
return None
485486

486487
return completed.catch(handle_error)
487488

488489
return completed
489-
except Exception as e:
490+
except GraphQLError as e:
490491
traceback = sys.exc_info()[2]
491492
exe_context.report_error(e, traceback)
492493
return None
@@ -528,12 +529,15 @@ def complete_value(
528529
),
529530
lambda error: Promise.rejected(
530531
GraphQLLocatedError(field_asts, original_error=error, path=path)
532+
if isinstance(error, GraphQLError) else error
531533
),
532534
)
533535

534536
# print return_type, type(result)
535-
if isinstance(result, Exception):
537+
if isinstance(result, GraphQLError):
536538
raise GraphQLLocatedError(field_asts, original_error=result, path=path)
539+
if isinstance(result, Exception):
540+
raise result
537541

538542
if isinstance(return_type, GraphQLNonNull):
539543
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": [

‎graphql/execution/tests/test_located_error.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
execute,
1010
parse,
1111
)
12-
from graphql.error import GraphQLLocatedError
12+
from graphql.error import GraphQLError, GraphQLLocatedError
1313

1414

1515
def test_unicode_error_message():
@@ -18,7 +18,7 @@ def test_unicode_error_message():
1818

1919
def resolver(context, *_):
2020
# type: (Optional[Any], *ResolveInfo) -> NoReturn
21-
raise Exception(u"UNIÇODÉ!")
21+
raise GraphQLError(u"UNIÇODÉ!")
2222

2323
Type = GraphQLObjectType(
2424
"Type", {"unicode": GraphQLField(GraphQLString, resolver=resolver)}

0 commit comments

Comments
 (0)