Skip to content

Commit e91ba02

Browse files
martinbonninmartinbonnin
committed
Restore @experimental_disableErrorPropagation 
1 parent 10046cf commit e91ba02

12 files changed

Lines changed: 237 additions & 19 deletions

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@
2525
import static graphql.introspection.Introspection.DirectiveLocation.INLINE_FRAGMENT;
2626
import static graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION;
2727
import static graphql.introspection.Introspection.DirectiveLocation.INPUT_OBJECT;
28+
import static graphql.introspection.Introspection.DirectiveLocation.MUTATION;
29+
import static graphql.introspection.Introspection.DirectiveLocation.QUERY;
2830
import static graphql.introspection.Introspection.DirectiveLocation.SCALAR;
31+
import static graphql.introspection.Introspection.DirectiveLocation.SUBSCRIPTION;
2932
import static graphql.language.DirectiveLocation.newDirectiveLocation;
3033
import static graphql.language.InputValueDefinition.newInputValueDefinition;
3134
import static graphql.language.NonNullType.newNonNullType;
@@ -46,6 +49,7 @@ public class Directives {
4649
private static final String SPECIFIED_BY = "specifiedBy";
4750
private static final String ONE_OF = "oneOf";
4851
private static final String DEFER = "defer";
52+
private static final String EXPERIMENTAL_DISABLE_ERROR_PROPAGATION = "experimental_disableErrorPropagation";
4953

5054
public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION;
5155
public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION;
@@ -55,6 +59,8 @@ public class Directives {
5559
public static final DirectiveDefinition ONE_OF_DIRECTIVE_DEFINITION;
5660
@ExperimentalApi
5761
public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION;
62+
@ExperimentalApi
63+
public static final DirectiveDefinition EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION;
5864

5965
public static final String BOOLEAN = "Boolean";
6066
public static final String STRING = "String";
@@ -142,6 +148,13 @@ public class Directives {
142148
.type(newTypeName().name(STRING).build())
143149
.build())
144150
.build();
151+
EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition()
152+
.name(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION)
153+
.directiveLocation(newDirectiveLocation().name(QUERY.name()).build())
154+
.directiveLocation(newDirectiveLocation().name(MUTATION.name()).build())
155+
.directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build())
156+
.description(createDescription("This directive allows returning null in non-null positions that have an associated error"))
157+
.build();
145158
}
146159

147160
/**
@@ -235,6 +248,14 @@ public class Directives {
235248
.definition(ONE_OF_DIRECTIVE_DEFINITION)
236249
.build();
237250

251+
@ExperimentalApi
252+
public static final GraphQLDirective ExperimentalDisableErrorPropagationDirective = GraphQLDirective.newDirective()
253+
.name(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION)
254+
.description("This directive disables error propagation when a non nullable field returns null for the given operation.")
255+
.validLocations(QUERY, MUTATION, SUBSCRIPTION)
256+
.definition(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION)
257+
.build();
258+
238259
/**
239260
* The set of all built-in directives that are always present in a graphql schema.
240261
* The iteration order is stable and meaningful.
@@ -254,6 +275,7 @@ public class Directives {
254275
directives.add(SpecifiedByDirective);
255276
directives.add(OneOfDirective);
256277
directives.add(DeferDirective);
278+
directives.add(ExperimentalDisableErrorPropagationDirective);
257279
BUILT_IN_DIRECTIVES = Collections.unmodifiableSet(directives);
258280

259281
LinkedHashMap<String, GraphQLDirective> map = new LinkedHashMap<>();
@@ -288,4 +310,24 @@ public static boolean isBuiltInDirective(GraphQLDirective directive) {
288310
private static Description createDescription(String s) {
289311
return new Description(s, null, false);
290312
}
313+
314+
private static final AtomicBoolean EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED = new AtomicBoolean(true);
315+
316+
/**
317+
* This can be used to get the state the `@experimental_disableErrorPropagation` directive support on a JVM wide basis .
318+
* @return true if the `@experimental_disableErrorPropagation` directive will be respected
319+
*/
320+
public static boolean isExperimentalDisableErrorPropagationDirectiveEnabled() {
321+
return EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED.get();
322+
}
323+
324+
/**
325+
* This can be used to disable the `@experimental_disableErrorPropagation` directive support on a JVM wide basis in case your server
326+
* implementation does NOT want to act on this directive ever.
327+
*
328+
* @param flag the desired state of the flag
329+
*/
330+
public static void setExperimentalDisableErrorPropagationEnabled(boolean flag) {
331+
EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED.set(flag);
332+
}
291333
}

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33

44
import com.google.common.collect.ImmutableList;
5+
import graphql.Directives;
56
import graphql.EngineRunningState;
67
import graphql.ExecutionInput;
78
import graphql.ExecutionResult;
@@ -28,6 +29,7 @@
2829
import graphql.extensions.ExtensionsBuilder;
2930
import graphql.incremental.DelayedIncrementalPartialResult;
3031
import graphql.incremental.IncrementalExecutionResultImpl;
32+
import graphql.language.Directive;
3133
import graphql.language.Document;
3234
import graphql.language.NodeUtil;
3335
import graphql.language.OperationDefinition;
@@ -47,6 +49,7 @@
4749
import java.util.concurrent.atomic.AtomicBoolean;
4850
import java.util.function.Supplier;
4951

52+
import static graphql.Directives.EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION;
5053
import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder;
5154
import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo;
5255
import static graphql.execution.ExecutionStrategyParameters.newParameters;
@@ -101,7 +104,13 @@ public CompletableFuture<ExecutionResult> execute(Document document, GraphQLSche
101104
return completedFuture(abortExecutionException.toExecutionResult());
102105
}
103106

104-
OnError onError = isExperimentalOnErrorEnabled()? executionInput.getOnError() : OnError.PROPAGATE;
107+
OnError onError = isExperimentalOnErrorEnabled() ? executionInput.getOnError() : OnError.PROPAGATE;
108+
109+
// The `@experimental_disableErrorPropagation` directive remains supported and, when present on the
110+
// operation, is equivalent to requesting `onError: NULL`.
111+
if (!propagateErrorsOnNonNullContractFailure(getOperationResult.operationDefinition.getDirectives())) {
112+
onError = OnError.NULL;
113+
}
105114

106115
GraphQLContext graphQLContext = executionInput.getGraphQLContext();
107116
Locale locale = executionInput.getLocale();
@@ -315,6 +324,15 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio
315324
return executionResult;
316325
}
317326

327+
private boolean propagateErrorsOnNonNullContractFailure(List<Directive> directives) {
328+
boolean jvmWideEnabled = Directives.isExperimentalDisableErrorPropagationDirectiveEnabled();
329+
if (!jvmWideEnabled) {
330+
return true;
331+
}
332+
Directive foundDirective = NodeUtil.findNodeByName(directives, EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION.getName());
333+
return foundDirective == null;
334+
}
335+
318336
private static final AtomicBoolean EXPERIMENTAL_ON_ERROR_ENABLED = new AtomicBoolean(true);
319337

320338
/**

‎src/test/groovy/graphql/Issue2141.groovy‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ directive @deprecated(
3838
reason: String! = "No longer supported"
3939
) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION
4040
41+
"This directive disables error propagation when a non nullable field returns null for the given operation."
42+
directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION
43+
4144
"Directs the executor to include this field or fragment only when the `if` argument is true"
4245
directive @include(
4346
"Included when true."

‎src/test/groovy/graphql/StarWarsIntrospectionTests.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,6 @@ class StarWarsIntrospectionTests extends Specification {
430430
schemaParts.get('mutationType').size() == 1
431431
schemaParts.get('subscriptionType') == null
432432
schemaParts.get('types').size() == 17
433-
schemaParts.get('directives').size() == 6
433+
schemaParts.get('directives').size() == 7
434434
}
435435
}

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

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package graphql.execution
2+
3+
import graphql.Directives
4+
import graphql.ExecutionInput
5+
import graphql.TestUtil
6+
import spock.lang.Specification
7+
8+
class ExperimentalDisableErrorPropagationTest extends Specification {
9+
10+
void setup() {
11+
Directives.setExperimentalDisableErrorPropagationEnabled(true)
12+
}
13+
14+
def "with experimental_disableErrorPropagation, null is returned"() {
15+
16+
def sdl = '''
17+
type Query {
18+
foo : Int!
19+
}
20+
directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION
21+
'''
22+
23+
def graphql = TestUtil.graphQL(sdl).build()
24+
25+
def query = '''
26+
query GetFoo @experimental_disableErrorPropagation { foo }
27+
'''
28+
when:
29+
30+
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
31+
[foo: null]
32+
).build()
33+
34+
def er = graphql.execute(ei)
35+
36+
then:
37+
er.data != null
38+
er.data.foo == null
39+
er.errors[0].path.toList() == ["foo"]
40+
}
41+
42+
def "without experimental_disableErrorPropagation, error is propagated"() {
43+
44+
def sdl = '''
45+
type Query {
46+
foo : Int!
47+
}
48+
directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION
49+
'''
50+
51+
def graphql = TestUtil.graphQL(sdl).build()
52+
53+
def query = '''
54+
query GetFoo { foo }
55+
'''
56+
when:
57+
58+
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
59+
[foo: null]
60+
).build()
61+
62+
def er = graphql.execute(ei)
63+
64+
then:
65+
er.data == null
66+
er.errors[0].message == "The field at path '/foo' 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 'Int' within parent type 'Query'"
67+
er.errors[0].path.toList() == ["foo"]
68+
}
69+
70+
def "With experimental_disableErrorPropagation JVM disabled, error is propagated"() {
71+
def sdl = '''
72+
type Query {
73+
foo : Int!
74+
}
75+
directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION
76+
'''
77+
78+
def graphql = TestUtil.graphQL(sdl).build()
79+
80+
def query = '''
81+
query GetFoo @experimental_disableErrorPropagation { foo }
82+
'''
83+
when:
84+
85+
Directives.setExperimentalDisableErrorPropagationEnabled(false) // JVM wide
86+
87+
ExecutionInput ei = ExecutionInput.newExecutionInput(query).root(
88+
[foo: null]
89+
).build()
90+
91+
def er = graphql.execute(ei)
92+
93+
then:
94+
er.data == null
95+
er.errors[0].message == "The field at path '/foo' 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 'Int' within parent type 'Query'"
96+
er.errors[0].path.toList() == ["foo"]
97+
}
98+
99+
def "when @experimental_disableErrorPropagation is not added to the schema operation is gets added by schema code"() {
100+
101+
def sdl = '''
102+
type Query {
103+
foo : Int!
104+
}
105+
'''
106+
107+
when:
108+
def graphql = TestUtil.graphQL(sdl).build()
109+
110+
then:
111+
graphql.getGraphQLSchema().getDirective(Directives.ExperimentalDisableErrorPropagationDirective.getName()) === Directives.ExperimentalDisableErrorPropagationDirective
112+
}
113+
114+
}

‎src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ class IntrospectionWithDirectivesSupportTest extends Specification {
9090
def schemaType = er.data["__schema"]
9191

9292
schemaType["directives"] == [
93-
[name: "include"], [name: "skip"], [name: "defer"],
93+
[name: "include"], [name: "skip"], [name: "defer"], [name: "experimental_disableErrorPropagation"],
9494
[name: "example"], [name: "secret"], [name: "noDefault"],
9595
[name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"]
9696
]
@@ -174,7 +174,7 @@ class IntrospectionWithDirectivesSupportTest extends Specification {
174174

175175
def definedDirectives = er.data["__schema"]["directives"]
176176
// secret is filter out
177-
definedDirectives == [[name: "include"], [name: "skip"], [name: "defer"],
177+
definedDirectives == [[name: "include"], [name: "skip"], [name: "defer"], [name: "experimental_disableErrorPropagation"],
178178
[name: "example"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"]
179179
]
180180
}

‎src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -155,24 +155,25 @@ class GraphQLSchemaTest extends Specification {
155155

156156
when: "a schema is built"
157157
def schema = schemaBuilder.build()
158-
then: "all 6 built-in directives are present"
159-
schema.directives.size() == 6
158+
then: "all 7 built-in directives are present"
159+
schema.directives.size() == 7
160160
schema.getDirective("include") != null
161161
schema.getDirective("skip") != null
162162
schema.getDirective("deprecated") != null
163163
schema.getDirective("specifiedBy") != null
164164
schema.getDirective("oneOf") != null
165165
schema.getDirective("defer") != null
166+
schema.getDirective("experimental_disableErrorPropagation") != null
166167

167168
when: "the schema is transformed, things are copied"
168169
schema = schema.transform({ builder -> builder })
169-
then: "all 6 built-in directives are still present"
170-
schema.directives.size() == 6
170+
then: "all 7 built-in directives are still present"
171+
schema.directives.size() == 7
171172

172173
when: "clearDirectives is called"
173174
schema = basicSchemaBuilder().clearDirectives().build()
174-
then: "all 6 built-in directives are still present because ensureBuiltInDirectives re-adds them"
175-
schema.directives.size() == 6
175+
then: "all 7 built-in directives are still present because ensureBuiltInDirectives re-adds them"
176+
schema.directives.size() == 7
176177

177178
when: "clearDirectives is called and additional directives are added"
178179
schema = basicSchemaBuilder().clearDirectives()
@@ -181,8 +182,8 @@ class GraphQLSchemaTest extends Specification {
181182
.validLocations(DirectiveLocation.FIELD)
182183
.build())
183184
.build()
184-
then: "all 6 built-in directives are present plus the additional one"
185-
schema.directives.size() == 7
185+
then: "all 7 built-in directives are present plus the additional one"
186+
schema.directives.size() == 8
186187
schema.getDirective("custom") != null
187188
}
188189

@@ -274,7 +275,7 @@ class GraphQLSchemaTest extends Specification {
274275
def schema = basicSchemaBuilder()
275276
.additionalDirective(originalDirective)
276277
.build()
277-
assert schema.directives.size() == 7
278+
assert schema.directives.size() == 8
278279

279280
when: "the schema is transformed to replace the custom directive"
280281
def replacementDirective = GraphQLDirective.newDirective()
@@ -290,7 +291,7 @@ class GraphQLSchemaTest extends Specification {
290291
})
291292

292293
then: "all 7 built-in directives are still present"
293-
newSchema.directives.size() == 7
294+
newSchema.directives.size() == 8
294295
newSchema.getDirective("include") != null
295296
newSchema.getDirective("skip") != null
296297
newSchema.getDirective("deprecated") != null
@@ -318,7 +319,8 @@ class GraphQLSchemaTest extends Specification {
318319

319320
then: "unoverridden built-ins come first (in BUILT_IN_DIRECTIVES order, skip excluded), then user-supplied in insertion order"
320321
def names = schema.directives.collect { it.name }
321-
names == ["include", "deprecated", "specifiedBy", "oneOf", "defer", "custom", "skip"]
322+
names == ["include", "deprecated", "specifiedBy", "oneOf", "defer",
323+
"experimental_disableErrorPropagation", "custom", "skip"]
322324

323325
and: "the customized skip directive retains its custom description"
324326
schema.getDirective("skip").description == "custom skip description"

‎src/test/groovy/graphql/schema/SchemaTransformerTest.groovy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1510,6 +1510,7 @@ type Rental {
15101510
newSchema.getDirective("specifiedBy") != null
15111511
newSchema.getDirective("oneOf") != null
15121512
newSchema.getDirective("defer") != null
1513+
newSchema.getDirective("experimental_disableErrorPropagation") != null
15131514
newSchema.getDirectives().size() == schema.getDirectives().size()
15141515
}
15151516
}

‎src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ class SchemaDiffingTest extends Specification {
3838
schemaGraph.getVerticesByType(SchemaGraph.ARGUMENT).size() == 11
3939
schemaGraph.getVerticesByType(SchemaGraph.INPUT_FIELD).size() == 0
4040
schemaGraph.getVerticesByType(SchemaGraph.INPUT_OBJECT).size() == 0
41-
schemaGraph.getVerticesByType(SchemaGraph.DIRECTIVE).size() == 6
41+
schemaGraph.getVerticesByType(SchemaGraph.DIRECTIVE).size() == 7
4242
schemaGraph.getVerticesByType(SchemaGraph.APPLIED_ARGUMENT).size() == 0
4343
schemaGraph.getVerticesByType(SchemaGraph.APPLIED_DIRECTIVE).size() == 0
44-
schemaGraph.size() == 96
44+
schemaGraph.size() == 97
4545

4646
}
4747

0 commit comments

Comments
 (0)