Skip to content

Commit 233fcc4

Browse files
martinbonninbbakerman
martinbonnin
and
bbakerman
authored
Add support for @experimental_disableErrorPropagation (#3772)
* Add @errorHandling * cosmetics * add test for NonNullableFieldValidator * Add integration test * Add @publicapi * make some enums package-private * NULL -> ALLOW_NULL * remove wildcard imports * Add Javadoc * rename boolean getter * remove wildcard imports * restore empty lines * revert SchemaGenerator changes and add ExperimentalApi.ENABLE_CUSTOM_ERROR_PROPAGATION * remove unused imports * Also check error message * Rename to @nullonerror * add .idea/codeStyles * location -> position * Rename to @nullOnNonNullError * Revert "Rename to @nullOnNonNullError" This reverts commit e744cca. * keep in sync with graphql-js graphql/graphql-js#4348 * resolve merge * Brad Baker here Rather than ask for these changes - I decided to push to your branch as a maintainer and then get them into your PR * revert codeStyle changes * Brad Baker here Rather than ask for these changes - I decided to push to your branch as a maintainer and then get them into your PR This now adds the directive to the schema if not present * Brad Baker here Rather than ask for these changes - I decided to push to your branch as a maintainer and then get them into your PR * import * Brad Baker here Rather than ask for these changes - I decided to push to your branch as a maintainer and then get them into your PR Added the directive to common list --------- Co-authored-by: bbaker <bbaker@atlassian.com>
1 parent f1fd32b commit 233fcc4

19 files changed

Lines changed: 293 additions & 30 deletions

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
import graphql.language.StringValue;
88
import graphql.schema.GraphQLDirective;
99

10+
import java.util.concurrent.atomic.AtomicBoolean;
11+
1012
import static graphql.Scalars.GraphQLBoolean;
1113
import static graphql.Scalars.GraphQLString;
1214
import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION;
@@ -17,7 +19,10 @@
1719
import static graphql.introspection.Introspection.DirectiveLocation.INLINE_FRAGMENT;
1820
import static graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION;
1921
import static graphql.introspection.Introspection.DirectiveLocation.INPUT_OBJECT;
22+
import static graphql.introspection.Introspection.DirectiveLocation.MUTATION;
23+
import static graphql.introspection.Introspection.DirectiveLocation.QUERY;
2024
import static graphql.introspection.Introspection.DirectiveLocation.SCALAR;
25+
import static graphql.introspection.Introspection.DirectiveLocation.SUBSCRIPTION;
2126
import static graphql.language.DirectiveLocation.newDirectiveLocation;
2227
import static graphql.language.InputValueDefinition.newInputValueDefinition;
2328
import static graphql.language.NonNullType.newNonNullType;
@@ -37,6 +42,7 @@ public class Directives {
3742
private static final String SPECIFIED_BY = "specifiedBy";
3843
private static final String ONE_OF = "oneOf";
3944
private static final String DEFER = "defer";
45+
private static final String EXPERIMENTAL_DISABLE_ERROR_PROPAGATION = "experimental_disableErrorPropagation";
4046

4147
public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION;
4248
public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION;
@@ -46,6 +52,8 @@ public class Directives {
4652
public static final DirectiveDefinition ONE_OF_DIRECTIVE_DEFINITION;
4753
@ExperimentalApi
4854
public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION;
55+
@ExperimentalApi
56+
public static final DirectiveDefinition EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION;
4957

5058
public static final String BOOLEAN = "Boolean";
5159
public static final String STRING = "String";
@@ -133,6 +141,13 @@ public class Directives {
133141
.type(newTypeName().name(STRING).build())
134142
.build())
135143
.build();
144+
EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition()
145+
.name(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION)
146+
.directiveLocation(newDirectiveLocation().name(QUERY.name()).build())
147+
.directiveLocation(newDirectiveLocation().name(MUTATION.name()).build())
148+
.directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build())
149+
.description(createDescription("This directive allows returning null in non-null positions that have an associated error"))
150+
.build();
136151
}
137152

138153
/**
@@ -226,7 +241,35 @@ public class Directives {
226241
.definition(ONE_OF_DIRECTIVE_DEFINITION)
227242
.build();
228243

244+
@ExperimentalApi
245+
public static final GraphQLDirective ExperimentalDisableErrorPropagationDirective = GraphQLDirective.newDirective()
246+
.name(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION)
247+
.description("This directive disables error propagation when a non nullable field returns null for the given operation.")
248+
.validLocations(QUERY, MUTATION, SUBSCRIPTION)
249+
.definition(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION)
250+
.build();
251+
229252
private static Description createDescription(String s) {
230253
return new Description(s, null, false);
231254
}
255+
256+
private static final AtomicBoolean EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED = new AtomicBoolean(true);
257+
258+
/**
259+
* This can be used to get the state the `@experimental_disableErrorPropagation` directive support on a JVM wide basis .
260+
* @return true if the `@experimental_disableErrorPropagation` directive will be respected
261+
*/
262+
public static boolean isExperimentalDisableErrorPropagationDirectiveEnabled() {
263+
return EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED.get();
264+
}
265+
266+
/**
267+
* This can be used to disable the `@experimental_disableErrorPropagation` directive support on a JVM wide basis in case your server
268+
* implementation does NOT want to act on this directive ever.
269+
*
270+
* @param flag the desired state of the flag
271+
*/
272+
public static void setExperimentalDisableErrorPropagationEnabled(boolean flag) {
273+
EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED.set(flag);
274+
}
232275
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package graphql.execution;
22

33

4+
import graphql.Directives;
45
import graphql.ExecutionInput;
56
import graphql.ExecutionResult;
67
import graphql.ExecutionResultImpl;
@@ -20,6 +21,7 @@
2021
import graphql.extensions.ExtensionsBuilder;
2122
import graphql.incremental.DelayedIncrementalPartialResult;
2223
import graphql.incremental.IncrementalExecutionResultImpl;
24+
import graphql.language.Directive;
2325
import graphql.language.Document;
2426
import graphql.language.NodeUtil;
2527
import graphql.language.OperationDefinition;
@@ -36,6 +38,7 @@
3638
import java.util.Optional;
3739
import java.util.concurrent.CompletableFuture;
3840

41+
import static graphql.Directives.EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION;
3942
import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder;
4043
import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo;
4144
import static graphql.execution.ExecutionStrategyParameters.newParameters;
@@ -81,6 +84,8 @@ public CompletableFuture<ExecutionResult> execute(Document document, GraphQLSche
8184
throw rte;
8285
}
8386

87+
boolean propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure(getOperationResult.operationDefinition.getDirectives());
88+
8489
ExecutionContext executionContext = newExecutionContextBuilder()
8590
.instrumentation(instrumentation)
8691
.instrumentationState(instrumentationState)
@@ -101,6 +106,7 @@ public CompletableFuture<ExecutionResult> execute(Document document, GraphQLSche
101106
.locale(executionInput.getLocale())
102107
.valueUnboxer(valueUnboxer)
103108
.executionInput(executionInput)
109+
.propagapropagateErrorsOnNonNullContractFailureeErrors(propagateErrorsOnNonNullContractFailure)
104110
.build();
105111

106112
executionContext.getGraphQLContext().put(ResultNodesInfo.RESULT_NODES_INFO, executionContext.getResultNodesInfo());
@@ -263,4 +269,13 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio
263269
}
264270
return executionResult;
265271
}
272+
273+
private boolean propagateErrorsOnNonNullContractFailure(List<Directive> directives) {
274+
boolean jvmWideEnabled = Directives.isExperimentalDisableErrorPropagationDirectiveEnabled();
275+
if (! jvmWideEnabled) {
276+
return true;
277+
}
278+
Directive foundDirective = NodeUtil.findNodeByName(directives, EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION.getName());
279+
return foundDirective == null;
280+
}
266281
}

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.google.common.collect.ImmutableList;
55
import com.google.common.collect.ImmutableMap;
66
import graphql.ExecutionInput;
7+
import graphql.ExperimentalApi;
78
import graphql.GraphQLContext;
89
import graphql.GraphQLError;
910
import graphql.Internal;
@@ -59,6 +60,7 @@ public class ExecutionContext {
5960
private final ValueUnboxer valueUnboxer;
6061
private final ExecutionInput executionInput;
6162
private final Supplier<ExecutableNormalizedOperation> queryTree;
63+
private final boolean propagateErrorsOnNonNullContractFailure;
6264

6365
// this is modified after creation so it needs to be volatile to ensure visibility across Threads
6466
private volatile DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP;
@@ -88,6 +90,7 @@ public class ExecutionContext {
8890
this.executionInput = builder.executionInput;
8991
this.dataLoaderDispatcherStrategy = builder.dataLoaderDispatcherStrategy;
9092
this.queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables));
93+
this.propagateErrorsOnNonNullContractFailure = builder.propagateErrorsOnNonNullContractFailure;
9194
}
9295

9396

@@ -129,9 +132,7 @@ public CoercedVariables getCoercedVariables() {
129132

130133
/**
131134
* @param <T> for two
132-
*
133135
* @return the legacy context
134-
*
135136
* @deprecated use {@link #getGraphQLContext()} instead
136137
*/
137138
@Deprecated(since = "2021-07-05")
@@ -170,6 +171,17 @@ public ValueUnboxer getValueUnboxer() {
170171
return valueUnboxer;
171172
}
172173

174+
/**
175+
* @return true if the current operation should propagate errors in non-null positions
176+
* Propagating errors is the default. Error aware clients may opt in returning null in non-null positions
177+
* by using the `@experimental_disableErrorPropagation` directive.
178+
* @see graphql.Directives#setExperimentalDisableErrorPropagationEnabled(boolean) to change the JVM wide default
179+
*/
180+
@ExperimentalApi
181+
public boolean propagateErrorsOnNonNullContractFailure() {
182+
return propagateErrorsOnNonNullContractFailure;
183+
}
184+
173185
/**
174186
* @return true if the current operation is a Query
175187
*/
@@ -317,7 +329,6 @@ public DataLoaderDispatchStrategy getDataLoaderDispatcherStrategy() {
317329
* the current values and allows you to transform it how you want.
318330
*
319331
* @param builderConsumer the consumer code that will be given a builder to transform
320-
*
321332
* @return a new ExecutionContext object based on calling build on that builder
322333
*/
323334
public ExecutionContext transform(Consumer<ExecutionContextBuilder> builderConsumer) {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.google.common.collect.ImmutableList;
44
import com.google.common.collect.ImmutableMap;
55
import graphql.ExecutionInput;
6+
import graphql.ExperimentalApi;
67
import graphql.GraphQLContext;
78
import graphql.GraphQLError;
89
import graphql.Internal;
@@ -46,6 +47,7 @@ public class ExecutionContextBuilder {
4647
Object localContext;
4748
ExecutionInput executionInput;
4849
DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP;
50+
boolean propagateErrorsOnNonNullContractFailure = true;
4951

5052
/**
5153
* @return a new builder of {@link graphql.execution.ExecutionContext}s
@@ -92,6 +94,7 @@ public ExecutionContextBuilder() {
9294
valueUnboxer = other.getValueUnboxer();
9395
executionInput = other.getExecutionInput();
9496
dataLoaderDispatcherStrategy = other.getDataLoaderDispatcherStrategy();
97+
propagateErrorsOnNonNullContractFailure = other.propagateErrorsOnNonNullContractFailure();
9598
}
9699

97100
public ExecutionContextBuilder instrumentation(Instrumentation instrumentation) {
@@ -216,6 +219,13 @@ public ExecutionContextBuilder resetErrors() {
216219
return this;
217220
}
218221

222+
@ExperimentalApi
223+
public ExecutionContextBuilder propagapropagateErrorsOnNonNullContractFailureeErrors(boolean propagateErrorsOnNonNullContractFailure) {
224+
this.propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure;
225+
return this;
226+
}
227+
228+
219229
public ExecutionContext build() {
220230
// preconditions
221231
assertNotNull(executionId, () -> "You must provide a query identifier");

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ public <T> T validate(ExecutionStrategyParameters parameters, T result) throws N
5656
} else {
5757
executionContext.addError(error, path);
5858
}
59-
throw nonNullException;
59+
if (executionContext.propagateErrorsOnNonNullContractFailure()) {
60+
throw nonNullException;
61+
}
6062
}
6163
}
6264
return result;

‎src/main/java/graphql/schema/GraphQLSchema.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -847,16 +847,10 @@ private GraphQLSchema buildImpl() {
847847

848848
// schemas built via the schema generator have the deprecated directive BUT we want it present for hand built
849849
// schemas - it's inherently part of the spec!
850-
if (additionalDirectives.stream().noneMatch(d -> d.getName().equals(Directives.DeprecatedDirective.getName()))) {
851-
additionalDirectives.add(Directives.DeprecatedDirective);
852-
}
853-
854-
if (additionalDirectives.stream().noneMatch(d -> d.getName().equals(Directives.SpecifiedByDirective.getName()))) {
855-
additionalDirectives.add(Directives.SpecifiedByDirective);
856-
}
857-
if (additionalDirectives.stream().noneMatch(d -> d.getName().equals(Directives.OneOfDirective.getName()))) {
858-
additionalDirectives.add(Directives.OneOfDirective);
859-
}
850+
addBuiltInDirective(Directives.DeprecatedDirective, additionalDirectives);
851+
addBuiltInDirective(Directives.SpecifiedByDirective, additionalDirectives);
852+
addBuiltInDirective(Directives.OneOfDirective, additionalDirectives);
853+
addBuiltInDirective(Directives.ExperimentalDisableErrorPropagationDirective, additionalDirectives);
860854

861855
// quick build - no traversing
862856
final GraphQLSchema partiallyBuiltSchema = new GraphQLSchema(this);
@@ -879,6 +873,12 @@ private GraphQLSchema buildImpl() {
879873
return validateSchema(finalSchema);
880874
}
881875

876+
private void addBuiltInDirective(GraphQLDirective qlDirective, Set<GraphQLDirective> additionalDirectives1) {
877+
if (additionalDirectives1.stream().noneMatch(d -> d.getName().equals(qlDirective.getName()))) {
878+
additionalDirectives1.add(qlDirective);
879+
}
880+
}
881+
882882
private GraphQLSchema validateSchema(GraphQLSchema graphQLSchema) {
883883
Collection<SchemaValidationError> errors = new SchemaValidator().validateSchema(graphQLSchema);
884884
if (!errors.isEmpty()) {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,10 @@ public class DirectiveInfo {
3434
Directives.SkipDirective.getName(), Directives.SkipDirective,
3535
Directives.DeprecatedDirective.getName(), Directives.DeprecatedDirective,
3636
Directives.SpecifiedByDirective.getName(), Directives.SpecifiedByDirective,
37-
Directives.OneOfDirective.getName(), Directives.OneOfDirective
37+
Directives.OneOfDirective.getName(), Directives.OneOfDirective,
38+
// technically this is NOT yet in spec - but it is added by default by graphql-java so we include it
39+
// we should also do @defer at some future time soon
40+
Directives.ExperimentalDisableErrorPropagationDirective.getName(), Directives.ExperimentalDisableErrorPropagationDirective
3841
);
3942

4043
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import static java.util.stream.Collectors.toMap;
3636

3737
/**
38-
* This contains helper code to build out appliedm directives on schema element
38+
* This contains helper code to build out applied directives on schema element
3939
*/
4040
@Internal
4141
class SchemaGeneratorAppliedDirectiveHelper {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@
5656
import graphql.schema.GraphQLTypeReference;
5757
import graphql.schema.GraphQLUnionType;
5858
import graphql.schema.GraphqlTypeComparatorRegistry;
59-
import graphql.schema.PropertyDataFetcher;
6059
import graphql.schema.SingletonPropertyDataFetcher;
6160
import graphql.schema.TypeResolver;
6261
import graphql.schema.TypeResolverProxy;

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ directive @deprecated(
3030
reason: String! = "No longer supported"
3131
) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION
3232
33+
"This directive disables error propagation when a non nullable field returns null for the given operation."
34+
directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION
35+
3336
"Directs the executor to include this field or fragment only when the `if` argument is true"
3437
directive @include(
3538
"Included when true."

0 commit comments

Comments
 (0)