diff --git a/src/main/java/graphql/ExecutionInput.java b/src/main/java/graphql/ExecutionInput.java index d65920fcb..3dd9ce6f1 100644 --- a/src/main/java/graphql/ExecutionInput.java +++ b/src/main/java/graphql/ExecutionInput.java @@ -2,6 +2,7 @@ import graphql.collect.ImmutableKit; import graphql.execution.ExecutionId; +import graphql.execution.OnError; import graphql.execution.RawVariables; import graphql.execution.preparsed.persisted.PersistedQuerySupport; import org.dataloader.DataLoaderRegistry; @@ -36,6 +37,7 @@ public class ExecutionInput { private final Locale locale; private final AtomicBoolean cancelled; private final boolean profileExecution; + private final OnError onError; /** * In order for {@link #getQuery()} to never be null, use this to mark @@ -62,6 +64,7 @@ private ExecutionInput(Builder builder) { this.extensions = builder.extensions; this.cancelled = builder.cancelled; this.profileExecution = builder.profileExecution; + this.onError = builder.onError; } private static String assertQuery(Builder builder) { @@ -227,6 +230,10 @@ public boolean isProfileExecution() { return profileExecution; } + public OnError getOnError() { + return onError; + } + /** * This helps you transform the current ExecutionInput object into another one by starting a builder with all * the current values and allows you to transform it how you want. @@ -248,7 +255,8 @@ public ExecutionInput transform(Consumer builderConsumer) { .variables(this.rawVariables.toMap()) .extensions(this.extensions) .executionId(this.executionId) - .locale(this.locale); + .locale(this.locale) + .onError(this.onError); builderConsumer.accept(builder); @@ -267,6 +275,7 @@ public String toString() { ", dataLoaderRegistry=" + dataLoaderRegistry + ", executionId= " + executionId + ", locale= " + locale + + ", onError= " + onError + '}'; } @@ -308,6 +317,7 @@ public static class Builder { private ExecutionId executionId; private AtomicBoolean cancelled = new AtomicBoolean(false); private boolean profileExecution; + private OnError onError = OnError.PROPAGATE; /** * Package level access to the graphql context @@ -461,6 +471,11 @@ public Builder profileExecution(boolean profileExecution) { return this; } + public Builder onError(OnError onError) { + this.onError = onError; + return this; + } + public ExecutionInput build() { return new ExecutionInput(this); } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 448d1c738..14d59dfa7 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -46,6 +46,7 @@ import java.util.Locale; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import static graphql.Directives.EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION; @@ -103,7 +104,13 @@ public CompletableFuture execute(Document document, GraphQLSche return completedFuture(abortExecutionException.toExecutionResult()); } - boolean propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure(getOperationResult.operationDefinition.getDirectives()); + OnError onError = isExperimentalOnErrorEnabled() ? executionInput.getOnError() : OnError.PROPAGATE; + + // The `@experimental_disableErrorPropagation` directive remains supported and, when present on the + // operation, is equivalent to requesting `onError: NULL`. + if (!propagateErrorsOnNonNullContractFailure(getOperationResult.operationDefinition.getDirectives())) { + onError = OnError.NULL; + } GraphQLContext graphQLContext = executionInput.getGraphQLContext(); Locale locale = executionInput.getLocale(); @@ -137,7 +144,7 @@ public CompletableFuture execute(Document document, GraphQLSche .valueUnboxer(valueUnboxer) .responseMapFactory(responseMapFactory) .executionInput(executionInput) - .propagapropagateErrorsOnNonNullContractFailureeErrors(propagateErrorsOnNonNullContractFailure) + .onError(onError) .engineRunningState(engineRunningState) .profiler(profiler) .build(); @@ -325,4 +332,25 @@ private boolean propagateErrorsOnNonNullContractFailure(List directiv Directive foundDirective = NodeUtil.findNodeByName(directives, EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION.getName()); return foundDirective == null; } + + private static final AtomicBoolean EXPERIMENTAL_ON_ERROR_ENABLED = new AtomicBoolean(true); + + /** + * This can be used to get the state the `onError` request parameter support on a JVM-wide basis. + * @return true if the `onError` request parameter will be respected + */ + public static boolean isExperimentalOnErrorEnabled() { + return EXPERIMENTAL_ON_ERROR_ENABLED.get(); + } + + /** + * This can be used to disable the `onError` request parameter support on a JVM-wide basis in case your server + * implementation does NOT want to act on the request parameter ever. + * + * @param flag the desired state of the flag + */ + public static void setExperimentalOnErrorEnabled(boolean flag) { + EXPERIMENTAL_ON_ERROR_ENABLED.set(flag); + } + } diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 702d47520..c6cb5c890 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -72,7 +72,7 @@ public class ExecutionContext { private final ExecutionInput executionInput; private final Supplier queryTree; - private final boolean propagateErrorsOnNonNullContractFailure; + private final OnError onError; // this is modified after creation so it needs to be volatile to ensure visibility across Threads private volatile DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP; @@ -108,7 +108,7 @@ public class ExecutionContext { this.localContext = builder.localContext; this.executionInput = builder.executionInput; this.dataLoaderDispatcherStrategy = builder.dataLoaderDispatcherStrategy; - this.propagateErrorsOnNonNullContractFailure = builder.propagateErrorsOnNonNullContractFailure; + this.onError = builder.onError; this.engineRunningState = builder.engineRunningState; this.profiler = builder.profiler; // lazy loading for performance @@ -233,15 +233,15 @@ public ValueUnboxer getValueUnboxer() { } /** - * @return true if the current operation should propagate errors in non-null positions + * @return the [OnError] behavior requested by the client. * Propagating errors is the default. Error aware clients may opt in returning null in non-null positions - * by using the `@experimental_disableErrorPropagation` directive. + * or halting the execution. * - * @see graphql.Directives#setExperimentalDisableErrorPropagationEnabled(boolean) to change the JVM wide default + * @see Execution#setExperimentalOnErrorEnabled(boolean) (boolean) to change the JVM wide default */ @ExperimentalApi - public boolean propagateErrorsOnNonNullContractFailure() { - return propagateErrorsOnNonNullContractFailure; + public OnError getOnError() { + return onError; } /** diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index 4077b8def..9cfe9e951 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -53,7 +53,7 @@ public class ExecutionContextBuilder { Object localContext; ExecutionInput executionInput; DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP; - boolean propagateErrorsOnNonNullContractFailure = true; + OnError onError = OnError.PROPAGATE; EngineRunningState engineRunningState; ResponseMapFactory responseMapFactory = ResponseMapFactory.DEFAULT; Profiler profiler; @@ -104,7 +104,7 @@ public ExecutionContextBuilder() { valueUnboxer = other.getValueUnboxer(); executionInput = other.getExecutionInput(); dataLoaderDispatcherStrategy = other.getDataLoaderDispatcherStrategy(); - propagateErrorsOnNonNullContractFailure = other.propagateErrorsOnNonNullContractFailure(); + onError = other.getOnError(); engineRunningState = other.getEngineRunningState(); responseMapFactory = other.getResponseMapFactory(); profiler = other.getProfiler(); @@ -245,8 +245,8 @@ public ExecutionContextBuilder resetErrors() { } @ExperimentalApi - public ExecutionContextBuilder propagapropagateErrorsOnNonNullContractFailureeErrors(boolean propagateErrorsOnNonNullContractFailure) { - this.propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure; + public ExecutionContextBuilder onError(OnError onError) { + this.onError = onError; return this; } diff --git a/src/main/java/graphql/execution/NonNullableFieldValidator.java b/src/main/java/graphql/execution/NonNullableFieldValidator.java index 4680d9a7b..3242befe9 100644 --- a/src/main/java/graphql/execution/NonNullableFieldValidator.java +++ b/src/main/java/graphql/execution/NonNullableFieldValidator.java @@ -7,8 +7,8 @@ /** * This will check that a value is non-null when the type definition says it must be and, it will throw {@link NonNullableFieldWasNullException} * if this is not the case. - * - * See: https://spec.graphql.org/October2021/#sec-Errors-and-Non-Nullability + *

+ * See: https://spec.graphql.org/October2021/#sec-Errors-and-Non-Nullability */ @Internal public class NonNullableFieldValidator { @@ -20,7 +20,7 @@ public NonNullableFieldValidator(ExecutionContext executionContext) { } /** - * Called to check that a value is non-null if the type requires it to be non null + * Called to check that a value is non-null if the type requires it to be non-null * * @param parameters the execution strategy parameters * @param result the result to check @@ -55,8 +55,13 @@ public T validate(ExecutionStrategyParameters parameters, T result) throws N } else { executionContext.addError(error, path); } - if (executionContext.propagateErrorsOnNonNullContractFailure()) { + + OnError onError = executionContext.getOnError(); + if (onError == OnError.PROPAGATE) { + throw nonNullException; + } else if (onError == OnError.HALT) { + throw new AbortExecutionException(executionContext.getErrors()); } } } diff --git a/src/main/java/graphql/execution/OnError.java b/src/main/java/graphql/execution/OnError.java new file mode 100644 index 000000000..845c022e8 --- /dev/null +++ b/src/main/java/graphql/execution/OnError.java @@ -0,0 +1,15 @@ +package graphql.execution; + +import graphql.ExperimentalApi; +import org.jspecify.annotations.NullMarked; + +/** + * Controls how errors are handled during execution + */ +@ExperimentalApi +@NullMarked +public enum OnError { + NULL, + PROPAGATE, + HALT +} diff --git a/src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy b/src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy new file mode 100644 index 000000000..6bc6227c6 --- /dev/null +++ b/src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy @@ -0,0 +1,123 @@ +package graphql.execution + +import graphql.Directives +import graphql.ExecutionInput +import graphql.TestUtil +import spock.lang.Specification + +class ExperimentalOnErrorTest extends Specification { + + void setup() { + Execution.setExperimentalOnErrorEnabled(true) + } + + def "with onError: NULL, null is returned"() { + + def sdl = ''' + type Query { + foo : Int! + } + ''' + + def graphql = TestUtil.graphQL(sdl).build() + + def query = ''' + query GetFoo { foo } + ''' + when: + + ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( + [foo: null] + ).onError(OnError.NULL).build() + + def er = graphql.execute(ei) + + then: + er.data != null + er.data.foo == null + er.errors[0].path.toList() == ["foo"] + } + + def "with onError: HALT, execution stops and a request error is returned"() { + + def sdl = ''' + type Query { + foo : Int! + bar : Int + } + ''' + + def graphql = TestUtil.graphQL(sdl).build() + + def query = ''' + query GetFoo { foo bar } + ''' + when: + + ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( + [foo: null, bar: 42] + ).onError(OnError.HALT).build() + + def er = graphql.execute(ei) + + then: + er.data == null + er.errors.size() == 1 + er.errors[0].path.toList() == ["foo"] + } + + def "with onError: PROPAGATE, error is propagated"() { + + def sdl = ''' + type Query { + foo : Int! + } + ''' + + def graphql = TestUtil.graphQL(sdl).build() + + def query = ''' + query GetFoo { foo } + ''' + when: + + ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( + [foo: null] + ).build() + + def er = graphql.execute(ei) + + then: + er.data == null + 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'" + er.errors[0].path.toList() == ["foo"] + } + + def "With onError JVM disabled, error is propagated"() { + def sdl = ''' + type Query { + foo : Int! + } + ''' + + def graphql = TestUtil.graphQL(sdl).build() + + def query = ''' + query GetFoo { foo } + ''' + when: + + Execution.setExperimentalOnErrorEnabled(false) // JVM wide + + ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( + [foo: null] + ).onError(OnError.NULL).build() + + def er = graphql.execute(ei) + + then: + er.data == null + 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'" + er.errors[0].path.toList() == ["foo"] + } +} diff --git a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy index 33977d515..9d00ba75e 100644 --- a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy +++ b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy @@ -9,7 +9,7 @@ class NonNullableFieldValidatorTest extends Specification { def "non nullable field throws exception"() { ExecutionContext context = Mock(ExecutionContext) { - propagateErrorsOnNonNullContractFailure() >> true + getOnError() >> OnError.PROPAGATE } ExecutionStepInfo typeInfo = ExecutionStepInfo.newExecutionStepInfo().type(nonNull(GraphQLString)).build() @@ -31,7 +31,7 @@ class NonNullableFieldValidatorTest extends Specification { def "nullable field does not throw exception"() { ExecutionContext context = Mock(ExecutionContext) { - propagateErrorsOnNonNullContractFailure() >> true + getOnError() >> OnError.PROPAGATE } ExecutionStepInfo typeInfo = ExecutionStepInfo.newExecutionStepInfo().type(GraphQLString).build() @@ -52,7 +52,7 @@ class NonNullableFieldValidatorTest extends Specification { def "non nullable field returns null if errors are not propagated"() { ExecutionContext context = Mock(ExecutionContext) { - propagateErrorsOnNonNullContractFailure() >> false + getOnError() >> OnError.NULL } ExecutionStepInfo typeInfo = ExecutionStepInfo.newExecutionStepInfo().type(nonNull(GraphQLString)).build()