Skip to content

Commit 81d3ce4

Browse files
bbakermanbbakerman
authored
Updated the error message for the non null exception AND create its own error type (#1925)
1 parent 7770701 commit 81d3ce4

File tree

6 files changed

+141
-9
lines changed

6 files changed

+141
-9
lines changed

‎src/main/java/graphql/ErrorType.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ public enum ErrorType implements ErrorClassification {
99
InvalidSyntax,
1010
ValidationError,
1111
DataFetchingException,
12+
NullValueInNonNullableField,
1213
OperationNotSupported,
1314
ExecutionAborted
1415
}

‎src/main/java/graphql/execution/NonNullableFieldWasNullError.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public List<SourceLocation> getLocations() {
3939

4040
@Override
4141
public ErrorType getErrorType() {
42-
return ErrorType.DataFetchingException;
42+
return ErrorType.NullValueInNonNullableField;
4343
}
4444

4545
@Override

‎src/main/java/graphql/execution/NonNullableFieldWasNullException.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,20 @@ private static String mkMessage(ExecutionStepInfo executionStepInfo, ExecutionPa
4242
GraphQLType unwrappedTyped = executionStepInfo.getUnwrappedNonNullType();
4343
if (executionStepInfo.hasParent()) {
4444
GraphQLType unwrappedParentType = executionStepInfo.getParent().getUnwrappedNonNullType();
45-
return String.format("Cannot return null for non-nullable type: '%s' within parent '%s' (%s)", simplePrint(unwrappedTyped), simplePrint(unwrappedParentType), path);
45+
return String.format(
46+
"The field at path '%s' was declared as a non null type, but the code involved in retrieving" +
47+
" data has wrongly returned a null value. The graphql specification requires that the" +
48+
" parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on." +
49+
" The non-nullable type is '%s' within parent type '%s'",
50+
path, simplePrint(unwrappedTyped), simplePrint(unwrappedParentType));
51+
} else {
52+
return String.format(
53+
"The field at path '%s' was declared as a non null type, but the code involved in retrieving" +
54+
" data has wrongly returned a null value. The graphql specification requires that the" +
55+
" parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on." +
56+
" The non-nullable type is '%s'",
57+
path, simplePrint(unwrappedTyped));
4658
}
47-
return String.format("Cannot return null for non-nullable type: '%s' (%s)", simplePrint(unwrappedTyped), path);
4859
}
4960

5061
public ExecutionStepInfo getExecutionStepInfo() {

‎src/test/groovy/graphql/GraphQLErrorTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ class GraphQLErrorTest extends Specification {
4747

4848
new NonNullableFieldWasNullError(new NonNullableFieldWasNullException(mkTypeInfo(), mkPath())) |
4949
[
50-
message: 'Cannot return null for non-nullable type: \'__Schema\' (/heroes[0]/abilities/speed[4])',
50+
message: '''The field at path '/heroes[0]/abilities/speed[4]' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value. The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is '__Schema\'''',
5151
path : ["heroes", 0, "abilities", "speed", 4],
52-
extensions:[classification:"DataFetchingException"],
52+
extensions:[classification:"NullValueInNonNullableField"],
5353
]
5454

5555
new SerializationError(mkPath(), new CoercingSerializeException("Bad coercing")) |

‎src/test/groovy/graphql/execution/NonNullableFieldWasNullExceptionTest.groovy‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package graphql.execution
2+
3+
import graphql.ExecutionInput
4+
import graphql.TestUtil
5+
import spock.lang.Specification
6+
7+
class NonNullableFieldWasNullExceptionTest extends Specification {
8+
9+
def "lower level sets a null when it should not and it bubbles all the way up"() {
10+
11+
def sdl = '''
12+
type Query {
13+
topLevelField : Type1!
14+
}
15+
16+
type Type1 {
17+
middleLevelField : Type2!
18+
}
19+
20+
type Type2 {
21+
bottomLevelField : String!
22+
}
23+
'''
24+
25+
def graphql = TestUtil.graphQL(sdl).build()
26+
27+
def query = '''
28+
{ topLevelField { middleLevelField { bottomLevelField } } }
29+
'''
30+
when:
31+
32+
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
33+
[topLevelField: [middleLevelField: [bottomLevelField: null]]]
34+
).build()
35+
36+
def er = graphql.execute(ei)
37+
38+
then:
39+
er.data == null
40+
er.errors[0].path.toList() == ["topLevelField", "middleLevelField", "bottomLevelField"]
41+
}
42+
43+
def "lower level sets a null when it should not and it bubbles up one"() {
44+
def sdl = '''
45+
type Query {
46+
topLevelField : Type1!
47+
}
48+
49+
type Type1 {
50+
middleLevelField : Type2
51+
}
52+
53+
type Type2 {
54+
bottomLevelField : String!
55+
}
56+
'''
57+
58+
def graphql = TestUtil.graphQL(sdl).build()
59+
60+
def query = '''
61+
{ topLevelField { middleLevelField { bottomLevelField } } }
62+
'''
63+
when:
64+
65+
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
66+
[topLevelField: [middleLevelField: [bottomLevelField: null]]]
67+
).build()
68+
69+
def er = graphql.execute(ei)
70+
71+
then:
72+
er.data == [topLevelField : [middleLevelField : null ]]
73+
er.errors[0].path.toList() == ["topLevelField", "middleLevelField", "bottomLevelField"]
74+
}
75+
76+
def "a top level sets a null when it should not and it bubbles up one"() {
77+
def sdl = '''
78+
type Query {
79+
topLevelField : Type1!
80+
}
81+
82+
type Type1 {
83+
middleLevelField : Type2
84+
}
85+
86+
type Type2 {
87+
bottomLevelField : String!
88+
}
89+
'''
90+
91+
def graphql = TestUtil.graphQL(sdl).build()
92+
93+
def query = '''
94+
{ topLevelField { middleLevelField { bottomLevelField } } }
95+
'''
96+
when:
97+
98+
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
99+
[topLevelField: null]
100+
).build()
101+
102+
def er = graphql.execute(ei)
103+
104+
then:
105+
er.data == null
106+
er.errors[0].path.toList() == ["topLevelField"]
107+
108+
when:
109+
110+
ei = ExecutionInput.newExecutionInput(query).root(
111+
null
112+
).build()
113+
114+
er = graphql.execute(ei)
115+
116+
then:
117+
er.data == null
118+
er.errors[0].path.toList() == ["topLevelField"]
119+
}
120+
}

‎src/test/groovy/graphql/execution/SubscriptionExecutionStrategyTest.groovy‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,11 +345,11 @@ class SubscriptionExecutionStrategyTest extends Specification {
345345
if (i == 5) {
346346
message.data == null
347347
assert message.errors.size() == 2
348-
assert message.errors[0].errorType == ErrorType.DataFetchingException
349-
assert message.errors[0].message == "Cannot return null for non-nullable type: 'String' within parent 'Message' (/newMessage/sender)"
348+
assert message.errors[0].errorType == ErrorType.NullValueInNonNullableField
349+
assert message.errors[0].message.contains("/newMessage/sender")
350350

351-
assert message.errors[1].errorType == ErrorType.DataFetchingException
352-
assert message.errors[1].message == "Cannot return null for non-nullable type: 'String' within parent 'Message' (/newMessage/text)"
351+
assert message.errors[1].errorType == ErrorType.NullValueInNonNullableField
352+
assert message.errors[1].message.contains("/newMessage/text")
353353
} else {
354354
assert message.data == ["newMessage": [sender: "sender" + i, text: "text" + i]]
355355
}

0 commit comments

Comments
 (0)