From 148ddcc42bf2e6908a2e7b8275ae5e5eb654d0d7 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Mon, 11 May 2026 10:31:44 +0200 Subject: [PATCH 1/2] Replace `@experimental_disableErrorPropagation` with `onError` parameter --- src/main/java/graphql/Directives.java | 42 ------------------- src/main/java/graphql/ExecutionInput.java | 17 +++++++- .../java/graphql/execution/Execution.java | 34 +++++++++------ .../graphql/execution/ExecutionContext.java | 14 +++---- .../execution/ExecutionContextBuilder.java | 8 ++-- .../execution/NonNullableFieldValidator.java | 11 +++-- src/main/java/graphql/execution/OnError.java | 14 +++++++ src/test/groovy/graphql/Issue2141.groovy | 3 -- .../graphql/StarWarsIntrospectionTests.groovy | 2 +- ....groovy => ExperimentalOnErrorTest.groovy} | 39 +++++------------ .../NonNullableFieldValidatorTest.groovy | 6 +-- ...rospectionWithDirectivesSupportTest.groovy | 4 +- .../graphql/schema/GraphQLSchemaTest.groovy | 24 +++++------ .../schema/SchemaTransformerTest.groovy | 1 - .../schema/diffing/SchemaDiffingTest.groovy | 4 +- ...GeneratorAppliedDirectiveHelperTest.groovy | 2 - .../schema/idl/SchemaGeneratorTest.groovy | 4 +- .../schema/idl/SchemaPrinterTest.groovy | 36 ---------------- 18 files changed, 101 insertions(+), 164 deletions(-) create mode 100644 src/main/java/graphql/execution/OnError.java rename src/test/groovy/graphql/execution/{ExperimentalDisableErrorPropagationTest.groovy => ExperimentalOnErrorTest.groovy} (61%) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 2835867e05..a392834eb8 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -25,10 +25,7 @@ import static graphql.introspection.Introspection.DirectiveLocation.INLINE_FRAGMENT; import static graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION; import static graphql.introspection.Introspection.DirectiveLocation.INPUT_OBJECT; -import static graphql.introspection.Introspection.DirectiveLocation.MUTATION; -import static graphql.introspection.Introspection.DirectiveLocation.QUERY; import static graphql.introspection.Introspection.DirectiveLocation.SCALAR; -import static graphql.introspection.Introspection.DirectiveLocation.SUBSCRIPTION; import static graphql.language.DirectiveLocation.newDirectiveLocation; import static graphql.language.InputValueDefinition.newInputValueDefinition; import static graphql.language.NonNullType.newNonNullType; @@ -49,7 +46,6 @@ public class Directives { private static final String SPECIFIED_BY = "specifiedBy"; private static final String ONE_OF = "oneOf"; private static final String DEFER = "defer"; - private static final String EXPERIMENTAL_DISABLE_ERROR_PROPAGATION = "experimental_disableErrorPropagation"; public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION; public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION; @@ -59,8 +55,6 @@ public class Directives { public static final DirectiveDefinition ONE_OF_DIRECTIVE_DEFINITION; @ExperimentalApi public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION; - @ExperimentalApi - public static final DirectiveDefinition EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION; public static final String BOOLEAN = "Boolean"; public static final String STRING = "String"; @@ -148,13 +142,6 @@ public class Directives { .type(newTypeName().name(STRING).build()) .build()) .build(); - EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() - .name(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION) - .directiveLocation(newDirectiveLocation().name(QUERY.name()).build()) - .directiveLocation(newDirectiveLocation().name(MUTATION.name()).build()) - .directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build()) - .description(createDescription("This directive allows returning null in non-null positions that have an associated error")) - .build(); } /** @@ -248,14 +235,6 @@ public class Directives { .definition(ONE_OF_DIRECTIVE_DEFINITION) .build(); - @ExperimentalApi - public static final GraphQLDirective ExperimentalDisableErrorPropagationDirective = GraphQLDirective.newDirective() - .name(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION) - .description("This directive disables error propagation when a non nullable field returns null for the given operation.") - .validLocations(QUERY, MUTATION, SUBSCRIPTION) - .definition(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION) - .build(); - /** * The set of all built-in directives that are always present in a graphql schema. * The iteration order is stable and meaningful. @@ -275,7 +254,6 @@ public class Directives { directives.add(SpecifiedByDirective); directives.add(OneOfDirective); directives.add(DeferDirective); - directives.add(ExperimentalDisableErrorPropagationDirective); BUILT_IN_DIRECTIVES = Collections.unmodifiableSet(directives); LinkedHashMap map = new LinkedHashMap<>(); @@ -310,24 +288,4 @@ public static boolean isBuiltInDirective(GraphQLDirective directive) { private static Description createDescription(String s) { return new Description(s, null, false); } - - private static final AtomicBoolean EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED = new AtomicBoolean(true); - - /** - * This can be used to get the state the `@experimental_disableErrorPropagation` directive support on a JVM wide basis . - * @return true if the `@experimental_disableErrorPropagation` directive will be respected - */ - public static boolean isExperimentalDisableErrorPropagationDirectiveEnabled() { - return EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED.get(); - } - - /** - * This can be used to disable the `@experimental_disableErrorPropagation` directive support on a JVM wide basis in case your server - * implementation does NOT want to act on this directive ever. - * - * @param flag the desired state of the flag - */ - public static void setExperimentalDisableErrorPropagationEnabled(boolean flag) { - EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED.set(flag); - } } diff --git a/src/main/java/graphql/ExecutionInput.java b/src/main/java/graphql/ExecutionInput.java index d65920fcb3..3dd9ce6f19 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 448d1c7388..cd82c80da1 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -2,7 +2,6 @@ import com.google.common.collect.ImmutableList; -import graphql.Directives; import graphql.EngineRunningState; import graphql.ExecutionInput; import graphql.ExecutionResult; @@ -29,7 +28,6 @@ import graphql.extensions.ExtensionsBuilder; import graphql.incremental.DelayedIncrementalPartialResult; import graphql.incremental.IncrementalExecutionResultImpl; -import graphql.language.Directive; import graphql.language.Document; import graphql.language.NodeUtil; import graphql.language.OperationDefinition; @@ -46,9 +44,9 @@ 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; import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; import static graphql.execution.ExecutionStrategyParameters.newParameters; @@ -103,7 +101,7 @@ public CompletableFuture execute(Document document, GraphQLSche return completedFuture(abortExecutionException.toExecutionResult()); } - boolean propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure(getOperationResult.operationDefinition.getDirectives()); + OnError onError = isExperimentalOnErrorEnabled()? executionInput.getOnError() : OnError.PROPAGATE; GraphQLContext graphQLContext = executionInput.getGraphQLContext(); Locale locale = executionInput.getLocale(); @@ -137,7 +135,7 @@ public CompletableFuture execute(Document document, GraphQLSche .valueUnboxer(valueUnboxer) .responseMapFactory(responseMapFactory) .executionInput(executionInput) - .propagapropagateErrorsOnNonNullContractFailureeErrors(propagateErrorsOnNonNullContractFailure) + .onError(onError) .engineRunningState(engineRunningState) .profiler(profiler) .build(); @@ -317,12 +315,24 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio return executionResult; } - private boolean propagateErrorsOnNonNullContractFailure(List directives) { - boolean jvmWideEnabled = Directives.isExperimentalDisableErrorPropagationDirectiveEnabled(); - if (!jvmWideEnabled) { - return true; - } - 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 702d475209..c6cb5c8908 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 4077b8def3..9cfe9e9516 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 b59f633bac..f7a4ea6b32 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,7 +55,10 @@ 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; } } diff --git a/src/main/java/graphql/execution/OnError.java b/src/main/java/graphql/execution/OnError.java new file mode 100644 index 0000000000..4b4127fcf7 --- /dev/null +++ b/src/main/java/graphql/execution/OnError.java @@ -0,0 +1,14 @@ +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 +} diff --git a/src/test/groovy/graphql/Issue2141.groovy b/src/test/groovy/graphql/Issue2141.groovy index c3b00df8d6..225cb8486c 100644 --- a/src/test/groovy/graphql/Issue2141.groovy +++ b/src/test/groovy/graphql/Issue2141.groovy @@ -38,9 +38,6 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." diff --git a/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy b/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy index 75411f6af0..89fc781138 100644 --- a/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy +++ b/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy @@ -430,6 +430,6 @@ class StarWarsIntrospectionTests extends Specification { schemaParts.get('mutationType').size() == 1 schemaParts.get('subscriptionType') == null schemaParts.get('types').size() == 17 - schemaParts.get('directives').size() == 7 + schemaParts.get('directives').size() == 6 } } diff --git a/src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy similarity index 61% rename from src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy rename to src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy index 366c95426e..1ae17eb502 100644 --- a/src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy @@ -5,31 +5,30 @@ import graphql.ExecutionInput import graphql.TestUtil import spock.lang.Specification -class ExperimentalDisableErrorPropagationTest extends Specification { +class ExperimentalOnErrorTest extends Specification { void setup() { - Directives.setExperimentalDisableErrorPropagationEnabled(true) + Execution.setExperimentalOnErrorEnabled(true) } - def "with experimental_disableErrorPropagation, null is returned"() { + def "with onError: NULL, null is returned"() { def sdl = ''' type Query { foo : Int! } - directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @experimental_disableErrorPropagation { foo } + query GetFoo { foo } ''' when: ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).build() + ).onError(OnError.NULL).build() def er = graphql.execute(ei) @@ -39,13 +38,12 @@ class ExperimentalDisableErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "without experimental_disableErrorPropagation, error is propagated"() { + def "with onError: PROPAGATE, error is propagated"() { def sdl = ''' type Query { foo : Int! } - directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() @@ -67,26 +65,25 @@ class ExperimentalDisableErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "With experimental_disableErrorPropagation JVM disabled, error is propagated"() { + def "With onError JVM disabled, error is propagated"() { def sdl = ''' type Query { foo : Int! } - directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @experimental_disableErrorPropagation { foo } + query GetFoo { foo } ''' when: - Directives.setExperimentalDisableErrorPropagationEnabled(false) // JVM wide + Execution.setExperimentalOnErrorEnabled(false) // JVM wide ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).build() + ).onError(OnError.NULL).build() def er = graphql.execute(ei) @@ -95,20 +92,4 @@ class ExperimentalDisableErrorPropagationTest extends Specification { 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 "when @experimental_disableErrorPropagation is not added to the schema operation is gets added by schema code"() { - - def sdl = ''' - type Query { - foo : Int! - } - ''' - - when: - def graphql = TestUtil.graphQL(sdl).build() - - then: - graphql.getGraphQLSchema().getDirective(Directives.ExperimentalDisableErrorPropagationDirective.getName()) === Directives.ExperimentalDisableErrorPropagationDirective - } - } diff --git a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy index 33977d515c..9d00ba75e7 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() diff --git a/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy b/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy index d78bb30952..cfd43b601f 100644 --- a/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy +++ b/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy @@ -90,7 +90,7 @@ class IntrospectionWithDirectivesSupportTest extends Specification { def schemaType = er.data["__schema"] schemaType["directives"] == [ - [name: "include"], [name: "skip"], [name: "defer"], [name: "experimental_disableErrorPropagation"], + [name: "include"], [name: "skip"], [name: "defer"], [name: "example"], [name: "secret"], [name: "noDefault"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"] ] @@ -174,7 +174,7 @@ class IntrospectionWithDirectivesSupportTest extends Specification { def definedDirectives = er.data["__schema"]["directives"] // secret is filter out - definedDirectives == [[name: "include"], [name: "skip"], [name: "defer"], [name: "experimental_disableErrorPropagation"], + definedDirectives == [[name: "include"], [name: "skip"], [name: "defer"], [name: "example"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"] ] } diff --git a/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy b/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy index f3cafb9abb..2aad363660 100644 --- a/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy +++ b/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy @@ -155,25 +155,24 @@ class GraphQLSchemaTest extends Specification { when: "a schema is built" def schema = schemaBuilder.build() - then: "all 7 built-in directives are present" - schema.directives.size() == 7 + then: "all 6 built-in directives are present" + schema.directives.size() == 6 schema.getDirective("include") != null schema.getDirective("skip") != null schema.getDirective("deprecated") != null schema.getDirective("specifiedBy") != null schema.getDirective("oneOf") != null schema.getDirective("defer") != null - schema.getDirective("experimental_disableErrorPropagation") != null when: "the schema is transformed, things are copied" schema = schema.transform({ builder -> builder }) - then: "all 7 built-in directives are still present" - schema.directives.size() == 7 + then: "all 6 built-in directives are still present" + schema.directives.size() == 6 when: "clearDirectives is called" schema = basicSchemaBuilder().clearDirectives().build() - then: "all 7 built-in directives are still present because ensureBuiltInDirectives re-adds them" - schema.directives.size() == 7 + then: "all 6 built-in directives are still present because ensureBuiltInDirectives re-adds them" + schema.directives.size() == 6 when: "clearDirectives is called and additional directives are added" schema = basicSchemaBuilder().clearDirectives() @@ -182,8 +181,8 @@ class GraphQLSchemaTest extends Specification { .validLocations(DirectiveLocation.FIELD) .build()) .build() - then: "all 7 built-in directives are present plus the additional one" - schema.directives.size() == 8 + then: "all 6 built-in directives are present plus the additional one" + schema.directives.size() == 7 schema.getDirective("custom") != null } @@ -197,7 +196,7 @@ class GraphQLSchemaTest extends Specification { def schema = basicSchemaBuilder() .additionalDirective(originalDirective) .build() - assert schema.directives.size() == 8 + assert schema.directives.size() == 7 when: "the schema is transformed to replace the custom directive" def replacementDirective = GraphQLDirective.newDirective() @@ -213,7 +212,7 @@ class GraphQLSchemaTest extends Specification { }) then: "all 7 built-in directives are still present" - newSchema.directives.size() == 8 + newSchema.directives.size() == 7 newSchema.getDirective("include") != null newSchema.getDirective("skip") != null newSchema.getDirective("deprecated") != null @@ -241,8 +240,7 @@ class GraphQLSchemaTest extends Specification { then: "unoverridden built-ins come first (in BUILT_IN_DIRECTIVES order, skip excluded), then user-supplied in insertion order" def names = schema.directives.collect { it.name } - names == ["include", "deprecated", "specifiedBy", "oneOf", "defer", - "experimental_disableErrorPropagation", "custom", "skip"] + names == ["include", "deprecated", "specifiedBy", "oneOf", "defer", "custom", "skip"] and: "the customized skip directive retains its custom description" schema.getDirective("skip").description == "custom skip description" diff --git a/src/test/groovy/graphql/schema/SchemaTransformerTest.groovy b/src/test/groovy/graphql/schema/SchemaTransformerTest.groovy index bf0279755c..aad2254b55 100644 --- a/src/test/groovy/graphql/schema/SchemaTransformerTest.groovy +++ b/src/test/groovy/graphql/schema/SchemaTransformerTest.groovy @@ -1510,7 +1510,6 @@ type Rental { newSchema.getDirective("specifiedBy") != null newSchema.getDirective("oneOf") != null newSchema.getDirective("defer") != null - newSchema.getDirective("experimental_disableErrorPropagation") != null newSchema.getDirectives().size() == schema.getDirectives().size() } } diff --git a/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy b/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy index 46481fa7a5..ca941433ec 100644 --- a/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy +++ b/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy @@ -38,10 +38,10 @@ class SchemaDiffingTest extends Specification { schemaGraph.getVerticesByType(SchemaGraph.ARGUMENT).size() == 11 schemaGraph.getVerticesByType(SchemaGraph.INPUT_FIELD).size() == 0 schemaGraph.getVerticesByType(SchemaGraph.INPUT_OBJECT).size() == 0 - schemaGraph.getVerticesByType(SchemaGraph.DIRECTIVE).size() == 7 + schemaGraph.getVerticesByType(SchemaGraph.DIRECTIVE).size() == 6 schemaGraph.getVerticesByType(SchemaGraph.APPLIED_ARGUMENT).size() == 0 schemaGraph.getVerticesByType(SchemaGraph.APPLIED_DIRECTIVE).size() == 0 - schemaGraph.size() == 97 + schemaGraph.size() == 96 } diff --git a/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy index f42cc49058..35adb8cbfa 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy @@ -57,7 +57,6 @@ class SchemaGeneratorAppliedDirectiveHelperTest extends Specification { "complex", "defer", "deprecated", - "experimental_disableErrorPropagation", "foo", "include", "oneOf", @@ -109,7 +108,6 @@ class SchemaGeneratorAppliedDirectiveHelperTest extends Specification { "complex", "defer", "deprecated", - "experimental_disableErrorPropagation", "foo", "include", "oneOf", diff --git a/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy index 5c268ebd0f..14e32acf66 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy @@ -2025,7 +2025,7 @@ class SchemaGeneratorTest extends Specification { directives = schema.getDirectives() then: - directives.size() == 10 // built in ones : include / skip and deprecated + directives.size() == 9 // built in ones : include / skip and deprecated def directiveNames = directives.collect { it.name } directiveNames.contains("include") directiveNames.contains("skip") @@ -2041,7 +2041,7 @@ class SchemaGeneratorTest extends Specification { directivesMap = schema.getDirectivesByName() then: - directivesMap.size() == 10 // built in ones + directivesMap.size() == 9 // built in ones directivesMap.containsKey("include") directivesMap.containsKey("skip") directivesMap.containsKey("defer") diff --git a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy index ea4268043f..1e290b6630 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy @@ -984,9 +984,6 @@ directive @enumTypeDirective on ENUM directive @enumValueDirective on ENUM_VALUE -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - directive @fieldDirective1 on FIELD_DEFINITION directive @fieldDirective2(argBool: Boolean, argFloat: Float, argInt: Int, argStr: String) on FIELD_DEFINITION @@ -1167,9 +1164,6 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1276,9 +1270,6 @@ directive @deprecated( directive @example on FIELD_DEFINITION -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1355,9 +1346,6 @@ directive @deprecated( directive @example on FIELD_DEFINITION -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1461,9 +1449,6 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1558,9 +1543,6 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1711,9 +1693,6 @@ directive @deprecated( directive @directive1 on SCALAR -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -2256,9 +2235,6 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - directive @foo on SCHEMA "Directs the executor to include this field or fragment only when the `if` argument is true" @@ -2485,9 +2461,6 @@ directive @include( if: Boolean! ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Marks the field, argument, input field or enum value as deprecated" directive @deprecated( "The reason for the deprecation" @@ -2630,9 +2603,6 @@ directive @deprecated( # custom directive 'example' comment 1 directive @example on ENUM_VALUE -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -2877,9 +2847,6 @@ directive @deprecated( " custom directive 'example' description 1" directive @example on ENUM_VALUE -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -3073,9 +3040,6 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION -"This directive disables error propagation when a non nullable field returns null for the given operation." -directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION - "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." From f42cc932c1c4fba6ef2accf89e77ebd2732f0fc6 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Mon, 11 May 2026 11:00:44 +0200 Subject: [PATCH 2/2] Add support for `HALT` --- .../execution/NonNullableFieldValidator.java | 2 ++ src/main/java/graphql/execution/OnError.java | 3 +- .../execution/ExperimentalOnErrorTest.groovy | 28 +++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/main/java/graphql/execution/NonNullableFieldValidator.java b/src/main/java/graphql/execution/NonNullableFieldValidator.java index f7a4ea6b32..6b3669514c 100644 --- a/src/main/java/graphql/execution/NonNullableFieldValidator.java +++ b/src/main/java/graphql/execution/NonNullableFieldValidator.java @@ -60,6 +60,8 @@ public T validate(ExecutionStrategyParameters parameters, T result) throws N 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 index 4b4127fcf7..845c022e85 100644 --- a/src/main/java/graphql/execution/OnError.java +++ b/src/main/java/graphql/execution/OnError.java @@ -10,5 +10,6 @@ @NullMarked public enum OnError { NULL, - PROPAGATE + PROPAGATE, + HALT } diff --git a/src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy b/src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy index 1ae17eb502..6bc6227c6f 100644 --- a/src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy +++ b/src/test/groovy/graphql/execution/ExperimentalOnErrorTest.groovy @@ -38,6 +38,34 @@ class ExperimentalOnErrorTest extends Specification { 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 = '''