Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for @experimental_disableErrorPropagation #3772

Merged
merged 29 commits into from
Feb 28, 2025
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
3557a71
Add @errorHandling
martinbonnin Dec 10, 2024
63dcac3
cosmetics
martinbonnin Dec 10, 2024
2c6fd1c
add test for NonNullableFieldValidator
martinbonnin Dec 11, 2024
eb00f57
Add integration test
martinbonnin Dec 11, 2024
6bf581d
Add @PublicApi
martinbonnin Dec 11, 2024
1b66c6e
make some enums package-private
martinbonnin Dec 11, 2024
8f012ef
NULL -> ALLOW_NULL
martinbonnin Dec 11, 2024
ed23db4
remove wildcard imports
martinbonnin Dec 11, 2024
4d794cb
Add Javadoc
martinbonnin Dec 11, 2024
072276b
rename boolean getter
martinbonnin Dec 11, 2024
7d3097e
remove wildcard imports
martinbonnin Dec 11, 2024
dd9a01e
restore empty lines
martinbonnin Dec 11, 2024
41863ea
revert SchemaGenerator changes and add ExperimentalApi.ENABLE_CUSTOM_…
martinbonnin Dec 13, 2024
44be290
remove unused imports
martinbonnin Dec 13, 2024
160abcd
Also check error message
martinbonnin Dec 15, 2024
2c5cac1
Rename to @nullOnError
martinbonnin Dec 17, 2024
6542fe2
add .idea/codeStyles
martinbonnin Dec 17, 2024
e992d4d
location -> position
martinbonnin Dec 17, 2024
d0cae63
Merge branch 'master' into error-handling
martinbonnin Jan 2, 2025
e744cca
Rename to @nullOnNonNullError
martinbonnin Jan 9, 2025
a5f0fac
Revert "Rename to @nullOnNonNullError"
martinbonnin Feb 4, 2025
36ebb40
keep in sync with graphql-js
martinbonnin Feb 24, 2025
7288075
Merge branch 'master' into error-handling
martinbonnin Feb 24, 2025
0132374
resolve merge
martinbonnin Feb 24, 2025
e5a5342
Brad Baker here
bbakerman Feb 27, 2025
535a620
revert codeStyle changes
martinbonnin Feb 27, 2025
a37f8bd
Brad Baker here
bbakerman Feb 27, 2025
bcffe72
Brad Baker here
bbakerman Feb 27, 2025
8eb9fb5
Brad Baker here
bbakerman Feb 28, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
*.iml
*.ipr
*.iws
.idea
.idea/*
!.idea/codeStyles
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry can we get rid of these.

I dont want "outside opnions" on how we set up our IDEA in tis PR.

Make another PR if this is something we should do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing! 535a620

.gradle
build
classes
Expand Down
8 changes: 8 additions & 0 deletions .idea/codeStyles/Project.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions .idea/codeStyles/codeStyleConfig.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 43 additions & 0 deletions src/main/java/graphql/Directives.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import graphql.language.StringValue;
import graphql.schema.GraphQLDirective;

import java.util.concurrent.atomic.AtomicBoolean;

import static graphql.Scalars.GraphQLBoolean;
import static graphql.Scalars.GraphQLString;
import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION;
Expand All @@ -17,7 +19,10 @@
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;
Expand All @@ -37,6 +42,7 @@ 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 DISABLE_ERROR_PROPAGATION = "experimental_disableErrorPropagation";

public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION;
public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION;
Expand All @@ -46,6 +52,8 @@ public class Directives {
public static final DirectiveDefinition ONE_OF_DIRECTIVE_DEFINITION;
@ExperimentalApi
public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION;
@ExperimentalApi
public static final DirectiveDefinition DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION;

public static final String BOOLEAN = "Boolean";
public static final String STRING = "String";
Expand Down Expand Up @@ -133,6 +141,13 @@ public class Directives {
.type(newTypeName().name(STRING).build())
.build())
.build();
DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition()
.name(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();
}

/**
Expand Down Expand Up @@ -226,7 +241,35 @@ public class Directives {
.definition(ONE_OF_DIRECTIVE_DEFINITION)
.build();

@ExperimentalApi
public static final GraphQLDirective DisableErrorPropagationDirective = GraphQLDirective.newDirective()
.name(DISABLE_ERROR_PROPAGATION)
.description("This directive disables error propagation for the given operation.")
.validLocations(QUERY, MUTATION, SUBSCRIPTION)
.definition(DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION)
.build();

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);
}
}
15 changes: 15 additions & 0 deletions src/main/java/graphql/execution/Execution.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package graphql.execution;


import graphql.Directives;
import graphql.ExecutionInput;
import graphql.ExecutionResult;
import graphql.ExecutionResultImpl;
Expand All @@ -20,6 +21,7 @@
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;
Expand All @@ -36,6 +38,7 @@
import java.util.Optional;
import java.util.concurrent.CompletableFuture;

import static graphql.Directives.DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION;
import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder;
import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo;
import static graphql.execution.ExecutionStrategyParameters.newParameters;
Expand Down Expand Up @@ -81,6 +84,8 @@ public CompletableFuture<ExecutionResult> execute(Document document, GraphQLSche
throw rte;
}

boolean propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure(getOperationResult.operationDefinition.getDirectives());

ExecutionContext executionContext = newExecutionContextBuilder()
.instrumentation(instrumentation)
.instrumentationState(instrumentationState)
Expand All @@ -101,6 +106,7 @@ public CompletableFuture<ExecutionResult> execute(Document document, GraphQLSche
.locale(executionInput.getLocale())
.valueUnboxer(valueUnboxer)
.executionInput(executionInput)
.propagapropagateErrorsOnNonNullContractFailureeErrors(propagateErrorsOnNonNullContractFailure)
.build();

executionContext.getGraphQLContext().put(ResultNodesInfo.RESULT_NODES_INFO, executionContext.getResultNodesInfo());
Expand Down Expand Up @@ -263,4 +269,13 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio
}
return executionResult;
}

private boolean propagateErrorsOnNonNullContractFailure(List<Directive> directives) {
boolean jvmWideEnabled = Directives.isExperimentalDisableErrorPropagationDirectiveEnabled();
if (! jvmWideEnabled) {
return true;
}
Directive foundDirective = NodeUtil.findNodeByName(directives, DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION.getName());
return foundDirective == null;
}
}
28 changes: 15 additions & 13 deletions src/main/java/graphql/execution/ExecutionContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import graphql.ExecutionInput;
import graphql.GraphQLContext;
import graphql.GraphQLError;
import graphql.Internal;
import graphql.PublicApi;
import graphql.*;
import graphql.collect.ImmutableKit;
import graphql.execution.incremental.IncrementalCallState;
import graphql.execution.instrumentation.Instrumentation;
Expand All @@ -22,11 +18,7 @@
import graphql.util.LockKit;
import org.dataloader.DataLoaderRegistry;

import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.*;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import java.util.function.Supplier;
Expand Down Expand Up @@ -59,6 +51,7 @@ public class ExecutionContext {
private final ValueUnboxer valueUnboxer;
private final ExecutionInput executionInput;
private final Supplier<ExecutableNormalizedOperation> queryTree;
private final boolean propagateErrorsOnNonNullContractFailure;

// this is modified after creation so it needs to be volatile to ensure visibility across Threads
private volatile DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP;
Expand Down Expand Up @@ -88,6 +81,7 @@ public class ExecutionContext {
this.executionInput = builder.executionInput;
this.dataLoaderDispatcherStrategy = builder.dataLoaderDispatcherStrategy;
this.queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables));
this.propagateErrorsOnNonNullContractFailure = builder.propagateErrorsOnNonNullContractFailure;
}


Expand Down Expand Up @@ -129,9 +123,7 @@ public CoercedVariables getCoercedVariables() {

/**
* @param <T> for two
*
* @return the legacy context
*
* @deprecated use {@link #getGraphQLContext()} instead
*/
@Deprecated(since = "2021-07-05")
Expand Down Expand Up @@ -170,6 +162,17 @@ public ValueUnboxer getValueUnboxer() {
return valueUnboxer;
}

/**
* @return true if the current operation should propagate errors in non-null positions
* Propagating errors is the default. Error aware clients may opt in returning null in non-null positions
* by using the `@experimental_disableErrorPropagation` directive.
* @see graphql.Directives#setExperimentalDisableErrorPropagationEnabled(boolean) to change the JVM wide default
*/
@ExperimentalApi
public boolean propagateErrorsOnNonNullContractFailure() {
return propagateErrorsOnNonNullContractFailure;
}

/**
* @return true if the current operation is a Query
*/
Expand Down Expand Up @@ -317,7 +320,6 @@ public DataLoaderDispatchStrategy getDataLoaderDispatcherStrategy() {
* the current values and allows you to transform it how you want.
*
* @param builderConsumer the consumer code that will be given a builder to transform
*
* @return a new ExecutionContext object based on calling build on that builder
*/
public ExecutionContext transform(Consumer<ExecutionContextBuilder> builderConsumer) {
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/graphql/execution/ExecutionContextBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import graphql.ExecutionInput;
import graphql.ExperimentalApi;
import graphql.GraphQLContext;
import graphql.GraphQLError;
import graphql.Internal;
Expand Down Expand Up @@ -46,6 +47,7 @@ public class ExecutionContextBuilder {
Object localContext;
ExecutionInput executionInput;
DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP;
boolean propagateErrorsOnNonNullContractFailure = true;

/**
* @return a new builder of {@link graphql.execution.ExecutionContext}s
Expand Down Expand Up @@ -92,6 +94,7 @@ public ExecutionContextBuilder() {
valueUnboxer = other.getValueUnboxer();
executionInput = other.getExecutionInput();
dataLoaderDispatcherStrategy = other.getDataLoaderDispatcherStrategy();
propagateErrorsOnNonNullContractFailure = other.propagateErrorsOnNonNullContractFailure();
}

public ExecutionContextBuilder instrumentation(Instrumentation instrumentation) {
Expand Down Expand Up @@ -216,6 +219,13 @@ public ExecutionContextBuilder resetErrors() {
return this;
}

@ExperimentalApi
public ExecutionContextBuilder propagapropagateErrorsOnNonNullContractFailureeErrors(boolean propagateErrorsOnNonNullContractFailure) {
this.propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure;
return this;
}


public ExecutionContext build() {
// preconditions
assertNotNull(executionId, () -> "You must provide a query identifier");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ public <T> T validate(ExecutionStrategyParameters parameters, T result) throws N
} else {
executionContext.addError(error, path);
}
throw nonNullException;
if (executionContext.propagateErrorsOnNonNullContractFailure()) {
throw nonNullException;
}
}
}
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import static java.util.stream.Collectors.toMap;

/**
* This contains helper code to build out appliedm directives on schema element
* This contains helper code to build out applied directives on schema element
*/
@Internal
class SchemaGeneratorAppliedDirectiveHelper {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
import graphql.schema.GraphQLTypeReference;
import graphql.schema.GraphQLUnionType;
import graphql.schema.GraphqlTypeComparatorRegistry;
import graphql.schema.PropertyDataFetcher;
import graphql.schema.SingletonPropertyDataFetcher;
import graphql.schema.TypeResolver;
import graphql.schema.TypeResolverProxy;
Expand Down
Loading
Loading