Skip to content

Commit 41863ea

Browse files
martinbonninmartinbonnin
committed
revert SchemaGenerator changes and add ExperimentalApi.ENABLE_CUSTOM_ERROR_PROPAGATION
1 parent dd9a01e commit 41863ea

File tree

6 files changed

+120
-36
lines changed

6 files changed

+120
-36
lines changed

‎src/main/java/graphql/Directives.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ public class Directives {
152152
.name("onError")
153153
.type(newNonNullType(newTypeName().name(Enums.ON_ERROR).build()).build())
154154
.defaultValue(EnumValue.newEnumValue(Enums.ON_ERROR_PROPAGATE).build())
155+
.description(createDescription("How to handle errors."))
155156
.build())
156157
.build();
157158
}
@@ -259,7 +260,7 @@ public class Directives {
259260
.value(Enums.ON_ERROR_NULL)
260261
.build()))
261262
.defaultValueProgrammatic(Enums.ON_ERROR_PROPAGATE)
262-
.description("The URL that specifies the behaviour of this scalar."))
263+
.description("How to handle errors."))
263264
.validLocations(QUERY, MUTATION, SUBSCRIPTION)
264265
.definition(ERROR_HANDLING_DIRECTIVE_DEFINITION)
265266
.build();

‎src/main/java/graphql/ExperimentalApi.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,8 @@
2424
* The key that should be associated with a boolean value which indicates whether @defer and @stream behaviour is enabled for this execution.
2525
*/
2626
String ENABLE_INCREMENTAL_SUPPORT = "ENABLE_INCREMENTAL_SUPPORT";
27+
/**
28+
* The key that should be associated with a boolean value which indicates whether @errorHandling behaviour is enabled for this execution.
29+
*/
30+
String ENABLE_CUSTOM_ERROR_HANDLING = "ENABLE_CUSTOM_ERROR_HANDLING";
2731
}

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ public CompletableFuture<ExecutionResult> execute(Document document, GraphQLSche
9292
throw rte;
9393
}
9494

95+
boolean customErrorPropagationEnabled = Optional.ofNullable(executionInput.getGraphQLContext())
96+
.map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING))
97+
.orElse(false);
98+
boolean propagateErrors = !customErrorPropagationEnabled || propagateErrors(coercedVariables, operationDefinition.getDirectives(), true);
99+
95100
ExecutionContext executionContext = newExecutionContextBuilder()
96101
.instrumentation(instrumentation)
97102
.instrumentationState(instrumentationState)
@@ -112,7 +117,7 @@ public CompletableFuture<ExecutionResult> execute(Document document, GraphQLSche
112117
.locale(executionInput.getLocale())
113118
.valueUnboxer(valueUnboxer)
114119
.executionInput(executionInput)
115-
.propagateErrors(propagateErrors(coercedVariables, operationDefinition.getDirectives(), true))
120+
.propagateErrors(propagateErrors)
116121
.build();
117122

118123
executionContext.getGraphQLContext().put(ResultNodesInfo.RESULT_NODES_INFO, executionContext.getResultNodesInfo());

‎src/main/java/graphql/schema/idl/SchemaGenerator.java

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public GraphQLSchema makeExecutableSchema(Options options, TypeDefinitionRegistr
101101
TypeDefinitionRegistry typeRegistryCopy = new TypeDefinitionRegistry();
102102
typeRegistryCopy.merge(typeRegistry);
103103

104-
schemaGeneratorHelper.addDirectivesIncludedByDefault(typeRegistryCopy, options.addOnErrorDirective);
104+
schemaGeneratorHelper.addDirectivesIncludedByDefault(typeRegistryCopy);
105105

106106
List<GraphQLError> errors = typeChecker.checkTypeRegistry(typeRegistryCopy, wiring);
107107
if (!errors.isEmpty()) {
@@ -164,13 +164,11 @@ public static class Options {
164164
private final boolean useCommentsAsDescription;
165165
private final boolean captureAstDefinitions;
166166
private final boolean useAppliedDirectivesOnly;
167-
private final boolean addOnErrorDirective;
168167

169-
Options(boolean useCommentsAsDescription, boolean captureAstDefinitions, boolean useAppliedDirectivesOnly, boolean addOnErrorDirective) {
168+
Options(boolean useCommentsAsDescription, boolean captureAstDefinitions, boolean useAppliedDirectivesOnly) {
170169
this.useCommentsAsDescription = useCommentsAsDescription;
171170
this.captureAstDefinitions = captureAstDefinitions;
172171
this.useAppliedDirectivesOnly = useAppliedDirectivesOnly;
173-
this.addOnErrorDirective = addOnErrorDirective;
174172
}
175173

176174
public boolean isUseCommentsAsDescription() {
@@ -185,12 +183,8 @@ public boolean isUseAppliedDirectivesOnly() {
185183
return useAppliedDirectivesOnly;
186184
}
187185

188-
public boolean isAddOnErrorDirective() {
189-
return addOnErrorDirective;
190-
}
191-
192186
public static Options defaultOptions() {
193-
return new Options(true, true, false, false);
187+
return new Options(true, true, false);
194188
}
195189

196190
/**
@@ -203,7 +197,7 @@ public static Options defaultOptions() {
203197
* @return a new Options object
204198
*/
205199
public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) {
206-
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective);
200+
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly);
207201
}
208202

209203
/**
@@ -215,7 +209,7 @@ public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) {
215209
* @return a new Options object
216210
*/
217211
public Options captureAstDefinitions(boolean captureAstDefinitions) {
218-
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective);
212+
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly);
219213
}
220214

221215
/**
@@ -228,11 +222,7 @@ public Options captureAstDefinitions(boolean captureAstDefinitions) {
228222
* @return a new Options object
229223
*/
230224
public Options useAppliedDirectivesOnly(boolean useAppliedDirectivesOnly) {
231-
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective);
232-
}
233-
234-
public Options addOnErrorDirective(boolean addOnErrorDirective) {
235-
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective);
225+
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly);
236226
}
237227
}
238228
}

‎src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,21 +1085,12 @@ Set<GraphQLDirective> buildAdditionalDirectiveDefinitions(BuildContext buildCtx)
10851085
}
10861086

10871087
void addDirectivesIncludedByDefault(TypeDefinitionRegistry typeRegistry) {
1088-
addDirectivesIncludedByDefault(typeRegistry, false);
1089-
}
1090-
1091-
void addDirectivesIncludedByDefault(TypeDefinitionRegistry typeRegistry, boolean addOnErrorDirective) {
10921088
// we synthesize this into the type registry - no need for them to add it
10931089
typeRegistry.add(DEPRECATED_DIRECTIVE_DEFINITION);
10941090
typeRegistry.add(SPECIFIED_BY_DIRECTIVE_DEFINITION);
10951091
typeRegistry.add(ONE_OF_DIRECTIVE_DEFINITION);
1096-
if (addOnErrorDirective) {
1097-
typeRegistry.add(ERROR_HANDLING_DIRECTIVE_DEFINITION);
1098-
typeRegistry.add(ON_ERROR_TYPE_DEFINITION);
1099-
}
11001092
}
11011093

1102-
11031094
private Optional<OperationTypeDefinition> getOperationNamed(String name, Map<String, OperationTypeDefinition> operationTypeDefs) {
11041095
return Optional.ofNullable(operationTypeDefs.get(name));
11051096
}

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

Lines changed: 102 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,26 @@
11
package graphql.execution
22

33
import graphql.ExecutionInput
4+
import graphql.ExperimentalApi
45
import graphql.TestUtil
5-
import graphql.schema.idl.RuntimeWiring
6-
import graphql.schema.idl.SchemaGenerator
7-
import graphql.GraphQL
86
import spock.lang.Specification
97

108
class NoErrorPropagationTest extends Specification {
119

12-
def "when error propagation is disabled null is returned"() {
10+
def "when onError is ALLOW_NULL null is returned"() {
1311

1412
def sdl = '''
1513
type Query {
1614
foo : Int!
1715
}
16+
enum OnError {
17+
ALLOW_NULL
18+
PROPAGATE
19+
}
20+
directive @errorHandling(onError: OnError) on QUERY | MUTATION | SUBSCRIPTION
1821
'''
1922

20-
def options = SchemaGenerator.Options.defaultOptions().addOnErrorDirective(true)
21-
22-
def schema = TestUtil.schema(options, sdl, RuntimeWiring.MOCKED_WIRING)
23-
def graphql = GraphQL.newGraphQL(schema).build()
23+
def graphql = TestUtil.graphQL(sdl).build()
2424

2525
def query = '''
2626
query GetFoo @errorHandling(onError: ALLOW_NULL) { foo }
@@ -29,7 +29,8 @@ class NoErrorPropagationTest extends Specification {
2929

3030
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
3131
[foo: null]
32-
).build()
32+
).graphQLContext([(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING): true])
33+
.build()
3334

3435
def er = graphql.execute(ei)
3536

@@ -38,4 +39,96 @@ class NoErrorPropagationTest extends Specification {
3839
er.data.foo == null
3940
er.errors[0].path.toList() == ["foo"]
4041
}
42+
43+
def "when onError is PROPAGATE error is propagated"() {
44+
45+
def sdl = '''
46+
type Query {
47+
foo : Int!
48+
}
49+
enum OnError {
50+
ALLOW_NULL
51+
PROPAGATE
52+
}
53+
directive @errorHandling(onError: OnError) on QUERY | MUTATION | SUBSCRIPTION
54+
'''
55+
56+
def graphql = TestUtil.graphQL(sdl).build()
57+
58+
def query = '''
59+
query GetFoo @errorHandling(onError: PROPAGATE) { foo }
60+
'''
61+
when:
62+
63+
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
64+
[foo: null]
65+
).graphQLContext([(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING): true])
66+
.build()
67+
68+
def er = graphql.execute(ei)
69+
70+
then:
71+
er.data == null
72+
er.errors[0].path.toList() == ["foo"]
73+
}
74+
75+
76+
def "when custom error propagation is disabled error is propagated"() {
77+
78+
def sdl = '''
79+
type Query {
80+
foo : Int!
81+
}
82+
enum OnError {
83+
ALLOW_NULL
84+
PROPAGATE
85+
}
86+
directive @errorHandling(onError: OnError) on QUERY | MUTATION | SUBSCRIPTION
87+
'''
88+
89+
def graphql = TestUtil.graphQL(sdl).build()
90+
91+
def query = '''
92+
query GetFoo @errorHandling(onError: ALLOW_NULL) { foo }
93+
'''
94+
when:
95+
96+
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
97+
[foo: null]
98+
).build()
99+
100+
def er = graphql.execute(ei)
101+
102+
then:
103+
er.data == null
104+
er.errors[0].path.toList() == ["foo"]
105+
}
106+
107+
def "when @errorHandling is not added to the schema operation does not validate"() {
108+
109+
def sdl = '''
110+
type Query {
111+
foo : Int!
112+
}
113+
'''
114+
115+
def graphql = TestUtil.graphQL(sdl).build()
116+
117+
def query = '''
118+
query GetFoo @errorHandling(onError: PROPAGATE) { foo }
119+
'''
120+
when:
121+
122+
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
123+
[foo: null]
124+
).graphQLContext([(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING): true])
125+
.build()
126+
127+
def er = graphql.execute(ei)
128+
129+
then:
130+
er.data == null
131+
er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'errorHandling'")
132+
}
133+
41134
}

0 commit comments

Comments
 (0)