From c31f2745a6c9bada3cae47d7dd0bb0e020433cb9 Mon Sep 17 00:00:00 2001 From: Alexandre Carlton Date: Sun, 24 May 2026 21:42:21 +1000 Subject: [PATCH 1/8] Capture partial results on cancellation Web clients can often impose request timeouts. If a GraphQL server takes too long to respond, then nothing is returned. While we can call `ExecutionInput.cancel()` in order to slide a web client's timeout, there are no usable results for the client to use. This change improves on this - if we set `#capturePartialResultsOnCancel`, then calling `.cancel` will save all the evaluated `DataFetcher` results so that something potentially usable can be returned to the client. Full disclosure: almost all of this was generated with Rovo, with a preliminary review from Brad before submitting here. --- src/main/java/graphql/ExecutionInput.java | 31 +++- .../graphql/GraphQLUnusualConfiguration.java | 57 ++++++ .../AbstractAsyncExecutionStrategy.java | 46 ++++- src/main/java/graphql/execution/Async.java | 120 ++++++++++++ .../execution/AsyncExecutionStrategy.java | 36 +++- .../java/graphql/execution/Execution.java | 8 + .../graphql/execution/ExecutionStrategy.java | 25 ++- .../groovy/graphql/ExecutionInputTest.groovy | 172 ++++++++++++++++++ 8 files changed, 476 insertions(+), 19 deletions(-) diff --git a/src/main/java/graphql/ExecutionInput.java b/src/main/java/graphql/ExecutionInput.java index d65920fcb3..a83c741e7f 100644 --- a/src/main/java/graphql/ExecutionInput.java +++ b/src/main/java/graphql/ExecutionInput.java @@ -11,7 +11,8 @@ import java.util.Locale; import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.CompletableFuture; + import java.util.function.Consumer; import static graphql.Assert.assertNotNull; @@ -34,7 +35,7 @@ public class ExecutionInput { private final DataLoaderRegistry dataLoaderRegistry; private final ExecutionId executionId; private final Locale locale; - private final AtomicBoolean cancelled; + private final CompletableFuture cancellationFuture; private final boolean profileExecution; /** @@ -60,7 +61,7 @@ private ExecutionInput(Builder builder) { this.locale = builder.locale != null ? builder.locale : Locale.getDefault(); // always have a locale in place this.localContext = builder.localContext; this.extensions = builder.extensions; - this.cancelled = builder.cancelled; + this.cancellationFuture = builder.cancellationFuture; this.profileExecution = builder.profileExecution; } @@ -211,7 +212,7 @@ public Map getExtensions() { * @return true if the execution should be cancelled */ public boolean isCancelled() { - return cancelled.get(); + return cancellationFuture.isDone(); } /** @@ -219,7 +220,18 @@ public boolean isCancelled() { * and the graphql engine needs to be running on a thread to allow is to respect this flag. */ public void cancel() { - cancelled.set(true); + cancellationFuture.complete(null); + } + + /** + * Returns a {@link CompletableFuture} that completes when {@link #cancel()} is called. + * This allows async code to race against cancellation without polling. + * + * @return a future that completes (with null) when this execution is cancelled + */ + @Internal + public CompletableFuture getCancellationFuture() { + return cancellationFuture; } @@ -241,7 +253,7 @@ public ExecutionInput transform(Consumer builderConsumer) { .operationName(this.operationName) .context(this.context) .internalTransferContext(this.graphQLContext) - .internalTransferCancelBoolean(this.cancelled) + .internalTransferCancellationFuture(this.cancellationFuture) .localContext(this.localContext) .root(this.root) .dataLoaderRegistry(this.dataLoaderRegistry) @@ -306,7 +318,7 @@ public static class Builder { private DataLoaderRegistry dataLoaderRegistry = EMPTY_DATALOADER_REGISTRY; private Locale locale = Locale.getDefault(); private ExecutionId executionId; - private AtomicBoolean cancelled = new AtomicBoolean(false); + private CompletableFuture cancellationFuture = new CompletableFuture<>(); private boolean profileExecution; /** @@ -412,9 +424,8 @@ private Builder internalTransferContext(GraphQLContext graphQLContext) { return this; } - // hidden on purpose - private Builder internalTransferCancelBoolean(AtomicBoolean cancelled) { - this.cancelled = cancelled; + private Builder internalTransferCancellationFuture(CompletableFuture cancellationFuture) { + this.cancellationFuture = cancellationFuture; return this; } diff --git a/src/main/java/graphql/GraphQLUnusualConfiguration.java b/src/main/java/graphql/GraphQLUnusualConfiguration.java index e96b0b7380..852d8b4bb1 100644 --- a/src/main/java/graphql/GraphQLUnusualConfiguration.java +++ b/src/main/java/graphql/GraphQLUnusualConfiguration.java @@ -277,6 +277,14 @@ public ResponseMapFactoryConfig responseMapFactory() { return new ResponseMapFactoryConfig(this); } + /** + * @return an element that allows you to control cancellation behavior + */ + @ExperimentalApi + public CancellationConfig cancellation() { + return new CancellationConfig(this); + } + private void put(String named, Object value) { if (graphQLContext != null) { graphQLContext.put(named, value); @@ -410,4 +418,53 @@ public ResponseMapFactoryConfig setFactory(ResponseMapFactory factory) { return this; } } + + public static class CancellationConfig extends BaseContextConfig { + + /** + * The context key used to enable capturing partial results when an execution is cancelled. + */ + @ExperimentalApi + public static final String CAPTURE_PARTIAL_RESULTS_ON_CANCEL = "graphql.capturePartialResultsOnCancel"; + + /** + * The context key used to store the cancellation {@link java.util.concurrent.CompletableFuture} + * that completes when {@link ExecutionInput#cancel()} is called. + * This is only set when {@link #CAPTURE_PARTIAL_RESULTS_ON_CANCEL} is enabled. + */ + @Internal + public static final String CANCELLATION_FUTURE_KEY = CAPTURE_PARTIAL_RESULTS_ON_CANCEL + ".cancelFuture"; + + private CancellationConfig(GraphQLContextConfiguration contextConfig) { + super(contextConfig); + } + + /** + * Returns true if partial results should be captured when the execution is cancelled via + * {@link ExecutionInput#cancel()}. + * + * @return true if partial results capture on cancel is enabled + */ + @ExperimentalApi + public boolean isCapturePartialResultsOnCancelEnabled() { + return contextConfig.getBoolean(CAPTURE_PARTIAL_RESULTS_ON_CANCEL); + } + + /** + * When enabled, if {@link ExecutionInput#cancel()} is called during execution, the engine will + * return the partial results of any fields that have already completed, along with an error + * indicating the execution was cancelled. + *

+ * By default this is false and cancellation returns only the cancellation error with null data. + * + * @param enable true to enable capturing partial results on cancel + * + * @return this config object for chaining + */ + @ExperimentalApi + public CancellationConfig capturePartialResultsOnCancel(boolean enable) { + contextConfig.put(CAPTURE_PARTIAL_RESULTS_ON_CANCEL, enable); + return this; + } + } } diff --git a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java index 25f2036cbf..682e8b583a 100644 --- a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java @@ -2,8 +2,11 @@ import graphql.ExecutionResult; import graphql.ExecutionResultImpl; +import graphql.GraphQLContext; import graphql.PublicSpi; +import java.util.concurrent.CompletionException; + import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; @@ -29,8 +32,47 @@ protected BiConsumer, Throwable> handleResults(ExecutionContext exe return; } - Map resolvedValuesByField = executionContext.getResponseMapFactory().createInsertionOrdered(fieldNames, results); - overallResult.complete(new ExecutionResultImpl(resolvedValuesByField, executionContext.getErrors())); + completeResultFuture(overallResult, executionContext, fieldNames, results); }; } + + protected BiConsumer, Throwable> handleResultsWithPartialData(ExecutionContext executionContext, List fieldNames, CompletableFuture overallResult) { + return (List results, Throwable exception) -> { + // when partial results on cancel is enabled the results list will already have partial data + // (already-completed fields) so we can build a partial response even if exception is set + if (exception != null) { + Throwable cause = exception instanceof CompletionException ? exception.getCause() : exception; + if (cause instanceof AbortExecutionException && results != null + && capturePartialResults(executionContext)) { + executionContext.addError((AbortExecutionException) cause); + completeResultFuture(overallResult, executionContext, fieldNames, results); + return; + } + handleNonNullException(executionContext, overallResult, exception); + return; + } + + // check if cancel fired while results were being gathered (no exception, but cancelled) + Throwable cancelException = executionContext.possibleCancellation(null); + if (cancelException != null) { + Throwable cancelCause = cancelException instanceof CompletionException ? cancelException.getCause() : cancelException; + if (cancelCause instanceof AbortExecutionException && results != null + && capturePartialResults(executionContext)) { + // we have partial data from already-completed CFs — use it + executionContext.addError((AbortExecutionException) cancelCause); + completeResultFuture(overallResult, executionContext, fieldNames, results); + } else { + handleNonNullException(executionContext, overallResult, cancelException); + } + return; + } + + completeResultFuture(overallResult, executionContext, fieldNames, results); + }; + } + + protected void completeResultFuture(CompletableFuture overallResult, ExecutionContext executionContext, List fieldNames, List results) { + Map resolvedValuesByField = executionContext.getResponseMapFactory().createInsertionOrdered(fieldNames, results); + overallResult.complete(new ExecutionResultImpl(resolvedValuesByField, executionContext.getErrors())); + } } diff --git a/src/main/java/graphql/execution/Async.java b/src/main/java/graphql/execution/Async.java index 8fc6ec0cf7..aff242a261 100644 --- a/src/main/java/graphql/execution/Async.java +++ b/src/main/java/graphql/execution/Async.java @@ -1,7 +1,9 @@ package graphql.execution; import graphql.Assert; +import graphql.GraphQLContext; import graphql.Internal; +import graphql.GraphQLUnusualConfiguration; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; @@ -21,6 +23,8 @@ import java.util.stream.Collectors; import static graphql.Assert.assertTrue; +import static graphql.GraphQLUnusualConfiguration.CancellationConfig.CANCELLATION_FUTURE_KEY; +import static graphql.GraphQLUnusualConfiguration.CancellationConfig.CAPTURE_PARTIAL_RESULTS_ON_CANCEL; import static java.util.stream.Collectors.toList; @Internal @@ -57,6 +61,17 @@ public interface CombinedBuilder { */ CompletableFuture> await(); + /** + * Like {@link #await()} but when {@link GraphQLUnusualConfiguration.CancellationConfig#CAPTURE_PARTIAL_RESULTS_ON_CANCEL} + * is enabled in the context and an {@link AbortExecutionException} causes a CF to fail, the already-completed + * CFs will have their values harvested and returned as partial results rather than completing exceptionally. + * + * @param graphQLContext the context to check for the partial results flag + * + * @return a CompletableFuture to a List of values (possibly partial on cancellation) + */ + CompletableFuture> await(GraphQLContext graphQLContext); + /** * This will return a {@code CompletableFuture>} if ANY of the input values are async * otherwise it just return a materialised {@code List} @@ -104,6 +119,11 @@ public CompletableFuture> await() { return typedEmpty(); } + @Override + public CompletableFuture> await(GraphQLContext graphQLContext) { + return await(); + } + @Override public Object awaitPolymorphic() { Assert.assertTrue(ix == 0, () -> "expected size was " + 0 + " got " + ix); @@ -149,6 +169,11 @@ public CompletableFuture> await() { return CompletableFuture.completedFuture(Collections.singletonList((T) value)); } + @Override + public CompletableFuture> await(GraphQLContext graphQLContext) { + return await(); + } + @Override public Object awaitPolymorphic() { commonSizeAssert(); @@ -232,6 +257,101 @@ public CompletableFuture> await() { return overallResult; } + @SuppressWarnings("unchecked") + @Override + public CompletableFuture> await(GraphQLContext graphQLContext) { + commonSizeAssert(); + if (cfCount == 0) { + return CompletableFuture.completedFuture(materialisedList(array)); + } + if (!graphQLContext.getBoolean(CAPTURE_PARTIAL_RESULTS_ON_CANCEL)) { + return await(); + } + + CompletableFuture cancellationFuture = graphQLContext.get(CANCELLATION_FUTURE_KEY); + if (cancellationFuture == null) { + return await(); + } + + CompletableFuture> overallResult = new CompletableFuture<>(); + CompletableFuture[] cfsArr = copyOnlyCFsToArray(); + CompletableFuture allOf = CompletableFuture.allOf(cfsArr); + + // race: either all CFs complete normally, or cancellation fires first + CompletableFuture.anyOf(allOf, cancellationFuture) + .whenComplete((ignored, exception) -> { + if (overallResult.isDone()) { + return; + } + if (exception != null) { + // a CF failed — not our abort path, propagate + overallResult.completeExceptionally(exception); + return; + } + if (!allOf.isDone()) { + // cancellation fired before allOf: harvest already-completed CFs + List partialResults = harvestPartialResults(array); + overallResult.complete(partialResults); + return; + } + // allOf finished normally: collect all results + overallResult.complete(collectAllResults(array, cfsArr)); + }); + + // also handle when allOf completes (either normally or exceptionally) after the race + allOf.whenComplete((ignored, exception) -> { + if (overallResult.isDone()) { + return; + } + if (exception != null) { + // a CF in allOf failed — propagate + overallResult.completeExceptionally(exception); + return; + } + overallResult.complete(collectAllResults(array, cfsArr)); + }); + + return overallResult; + } + + @SuppressWarnings("unchecked") + private List harvestPartialResults(Object[] array) { + List partialResults = new ArrayList<>(array.length); + for (Object object : array) { + if (object instanceof CompletableFuture) { + CompletableFuture cf = (CompletableFuture) object; + if (cf.isDone() && !cf.isCompletedExceptionally()) { + partialResults.add(cf.join()); + } else { + partialResults.add(null); + } + } else { + partialResults.add((T) object); + } + } + return partialResults; + } + + @SuppressWarnings("unchecked") + private List collectAllResults(Object[] array, CompletableFuture[] cfsArr) { + List results = new ArrayList<>(array.length); + if (cfsArr.length == array.length) { + for (CompletableFuture cf : cfsArr) { + results.add(cf.join()); + } + } else { + for (Object object : array) { + if (object instanceof CompletableFuture) { + CompletableFuture cf = (CompletableFuture) object; + results.add(cf.join()); + } else { + results.add((T) object); + } + } + } + return results; + } + @SuppressWarnings("unchecked") @NonNull private CompletableFuture[] copyOnlyCFsToArray() { diff --git a/src/main/java/graphql/execution/AsyncExecutionStrategy.java b/src/main/java/graphql/execution/AsyncExecutionStrategy.java index 325a5829a8..0d4c94d20d 100644 --- a/src/main/java/graphql/execution/AsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncExecutionStrategy.java @@ -1,6 +1,7 @@ package graphql.execution; import graphql.ExecutionResult; +import graphql.GraphQLContext; import graphql.PublicApi; import graphql.execution.incremental.DeferredExecutionSupport; import org.jspecify.annotations.NullMarked; @@ -10,9 +11,12 @@ import graphql.introspection.Introspection; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.function.BiConsumer; +import java.util.stream.Collectors; /** * The standard graphql execution strategy that runs fields asynchronously non-blocking. @@ -64,14 +68,36 @@ public CompletableFuture execute(ExecutionContext executionCont CompletableFuture overallResult = new CompletableFuture<>(); executionStrategyCtx.onDispatched(); - futures.await().whenComplete((completeValueInfos, throwable) -> { + GraphQLContext graphQLContext = executionContext.getGraphQLContext(); + futures.await(graphQLContext).whenComplete((completeValueInfos, throwable) -> { List fieldsExecutedOnInitialResult = deferredExecutionSupport.getNonDeferredFieldNames(fieldNames); - BiConsumer, Throwable> handleResultsConsumer = handleResults(executionContext, fieldsExecutedOnInitialResult, overallResult); throwable = executionContext.possibleCancellation(throwable); if (throwable != null) { - handleResultsConsumer.accept(null, throwable.getCause()); + if (capturePartialResults(executionContext) && completeValueInfos != null) { + // partial results: some FieldValueInfos completed before cancel - build with those + // null entries mean that FieldValueInfo CF wasn't done yet (field was cancelled) + Async.CombinedBuilder fieldValuesFutures = Async.ofExpectedSize(completeValueInfos.size()); + for (FieldValueInfo completeValueInfo : completeValueInfos) { + if (completeValueInfo != null) { + fieldValuesFutures.addObject(completeValueInfo.getFieldValueObject()); + } else { + fieldValuesFutures.addObject((Object) null); + } + } + List nonNullValueInfos = completeValueInfos.stream() + .filter(Objects::nonNull) + .collect(Collectors.toList()); + dataLoaderDispatcherStrategy.executionStrategyOnFieldValuesInfo(nonNullValueInfos, parameters); + executionStrategyCtx.onFieldValuesInfo(nonNullValueInfos); + // Let handleResultsWithPartialData add the error to avoid duplication + BiConsumer, Throwable> fieldValuesConsumer = handleResultsWithPartialData(executionContext, fieldsExecutedOnInitialResult, overallResult); + fieldValuesFutures.await(graphQLContext).whenComplete(fieldValuesConsumer); + return; + } + Throwable cause = throwable instanceof CompletionException ? throwable.getCause() : throwable; + handleResults(executionContext, fieldsExecutedOnInitialResult, overallResult).accept(null, cause); return; } @@ -81,7 +107,9 @@ public CompletableFuture execute(ExecutionContext executionCont } dataLoaderDispatcherStrategy.executionStrategyOnFieldValuesInfo(completeValueInfos, parameters); executionStrategyCtx.onFieldValuesInfo(completeValueInfos); - fieldValuesFutures.await().whenComplete(handleResultsConsumer); + + BiConsumer, Throwable> fieldValuesConsumer = handleResultsWithPartialData(executionContext, fieldsExecutedOnInitialResult, overallResult); + fieldValuesFutures.await(graphQLContext).whenComplete(fieldValuesConsumer); }).exceptionally((ex) -> { // if there are any issues with combining/handling the field results, // complete the future at all costs and bubble up any thrown exception so diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 448d1c7388..df7a0f6aeb 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -10,6 +10,7 @@ import graphql.GraphQL; import graphql.GraphQLContext; import graphql.GraphQLError; +import graphql.GraphQLUnusualConfiguration; import graphql.GraphQLException; import graphql.Internal; import graphql.Profiler; @@ -144,6 +145,13 @@ public CompletableFuture execute(Document document, GraphQLSche executionContext.getGraphQLContext().put(ResultNodesInfo.RESULT_NODES_INFO, executionContext.getResultNodesInfo()); + // When partial results on cancel is enabled, store the cancellation future in the context + // so that Async.Many#await(GraphQLContext) can race against it + if (graphQLContext.getBoolean(GraphQLUnusualConfiguration.CancellationConfig.CAPTURE_PARTIAL_RESULTS_ON_CANCEL)) { + graphQLContext.put(GraphQLUnusualConfiguration.CancellationConfig.CANCELLATION_FUTURE_KEY, + executionInput.getCancellationFuture()); + } + InstrumentationExecutionParameters parameters = new InstrumentationExecutionParameters( executionInput, graphQLSchema ); diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 713da688d0..fe51143a0f 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -5,6 +5,7 @@ import graphql.EngineRunningState; import graphql.ExecutionResult; import graphql.ExecutionResultImpl; +import graphql.GraphQLContext; import graphql.GraphQLError; import graphql.Internal; import graphql.PublicSpi; @@ -58,6 +59,7 @@ import java.util.function.Function; import java.util.function.Supplier; +import static graphql.GraphQLUnusualConfiguration.CancellationConfig.CAPTURE_PARTIAL_RESULTS_ON_CANCEL; import static graphql.execution.Async.exceptionallyCompletedFuture; import static graphql.execution.FieldCollectorParameters.newParameters; import static graphql.execution.FieldValueInfo.CompleteValueType.ENUM; @@ -218,6 +220,7 @@ protected Object executeObject(ExecutionContext executionContext, ExecutionStrat resolveObjectCtx.onDispatched(); + GraphQLContext graphQLContext = executionContext.getGraphQLContext(); Object fieldValueInfosResult = resolvedFieldFutures.awaitPolymorphic(); if (fieldValueInfosResult instanceof CompletableFuture) { CompletableFuture> fieldValueInfos = (CompletableFuture>) fieldValueInfosResult; @@ -232,7 +235,7 @@ protected Object executeObject(ExecutionContext executionContext, ExecutionStrat Async.CombinedBuilder resultFutures = fieldValuesCombinedBuilder(completeValueInfos); dataLoaderDispatcherStrategy.executeObjectOnFieldValuesInfo(completeValueInfos, parameters); resolveObjectCtx.onFieldValuesInfo(completeValueInfos); - resultFutures.await().whenComplete(handleResultsConsumer); + resultFutures.await(graphQLContext).whenComplete(handleResultsConsumer); }).exceptionally((ex) -> { // if there are any issues with combining/handling the field results, // complete the future at all costs and bubble up any thrown exception so @@ -273,19 +276,35 @@ protected Object executeObject(ExecutionContext executionContext, ExecutionStrat return resultFutures; } + protected boolean capturePartialResults(ExecutionContext executionContext) { + return executionContext.getGraphQLContext().getBoolean(CAPTURE_PARTIAL_RESULTS_ON_CANCEL); + } + private BiConsumer, Throwable> buildFieldValueMap(List fieldNames, CompletableFuture> overallResult, ExecutionContext executionContext) { return (List results, Throwable exception) -> { exception = executionContext.possibleCancellation(exception); if (exception != null) { + Throwable cause = exception instanceof CompletionException ? exception.getCause() : exception; + if (cause instanceof AbortExecutionException && results != null + && capturePartialResults(executionContext)) { + // partial results mode: results already has harvested values from completed CFs + executionContext.addError((AbortExecutionException) cause); + completeFieldValueMap(overallResult, executionContext, fieldNames, results); + return; + } handleValueException(overallResult, exception, executionContext); return; } - Map resolvedValuesByField = executionContext.getResponseMapFactory().createInsertionOrdered(fieldNames, results); - overallResult.complete(resolvedValuesByField); + completeFieldValueMap(overallResult, executionContext, fieldNames, results); }; } + protected void completeFieldValueMap(CompletableFuture> overallResult, ExecutionContext executionContext, List fieldNames, List results) { + Map resolvedValuesByField = executionContext.getResponseMapFactory().createInsertionOrdered(fieldNames, results); + overallResult.complete(resolvedValuesByField); + } + DeferredExecutionSupport createDeferredExecutionSupport(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { MergedSelectionSet fields = parameters.getFields(); diff --git a/src/test/groovy/graphql/ExecutionInputTest.groovy b/src/test/groovy/graphql/ExecutionInputTest.groovy index 23c06ea9c7..84819950b4 100644 --- a/src/test/groovy/graphql/ExecutionInputTest.groovy +++ b/src/test/groovy/graphql/ExecutionInputTest.groovy @@ -451,6 +451,178 @@ class ExecutionInputTest extends Specification { } + def "capturePartialResultsOnCancel configuration is accessible via unusualConfiguration"() { + // Smoke test: verify the configuration API works correctly + + def sdl = ''' + type Query { + field1 : String + field2 : String + } + ''' + + DataFetcher df = { DataFetchingEnvironment env -> "value" } + + def fetcherMap = ["Query": ["field1": df, "field2": df]] + def schema = TestUtil.schema(sdl, fetcherMap) + def graphQL = GraphQL.newGraphQL(schema).build() + + when: "capturePartialResultsOnCancel is enabled and execution completes normally" + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query("{ field1 field2 }") + .graphQLContext({ c -> + GraphQL.unusualConfiguration(c).cancellation().capturePartialResultsOnCancel(true) + }) + .build() + + def er = graphQL.execute(executionInput) + + then: "normal results are returned unchanged" + er.errors.isEmpty() + er.data == [field1: "value", field2: "value"] + + when: "capturePartialResultsOnCancel is enabled but cancel is called before execution" + ExecutionInput cancelledInput = ExecutionInput.newExecutionInput() + .query("{ field1 field2 }") + .graphQLContext({ c -> + GraphQL.unusualConfiguration(c).cancellation().capturePartialResultsOnCancel(true) + }) + .build() + cancelledInput.cancel() + + er = graphQL.execute(cancelledInput) + + then: "cancel error is returned" + !er.errors.isEmpty() + er.errors.any { it["message"].contains("Execution has been asked to be cancelled") } + } + + def "capturePartialResultsOnCancel returns fast field value when slow field is cancelled"() { + // The partial-results harvest happens at the fieldValuesFutures.await(graphQLContext) level. + // FieldValueInfo.getFieldValueObject() for async object fields is a CompletableFuture. + // When the slow object's CF hasn't completed yet and cancel fires, await(graphQLContext) + // harvests the already-done fast CF and returns partial data. + def sdl = ''' + type Query { + fast : Inner + slow : Inner + } + type Inner { + value : String + } + ''' + + CountDownLatch slowStarted = new CountDownLatch(1) + CountDownLatch slowRelease = new CountDownLatch(1) + + DataFetcher fastInnerDf = { DataFetchingEnvironment env -> + return CompletableFuture.completedFuture([value: "fast-value"]) + } + + DataFetcher slowInnerDf = { DataFetchingEnvironment env -> + return CompletableFuture.supplyAsync { + slowStarted.countDown() + slowRelease.await() + return [value: "slow-value"] + } + } + + DataFetcher innerValueDf = { DataFetchingEnvironment env -> + return env.source["value"] + } + + def fetcherMap = [ + "Query": ["fast": fastInnerDf, "slow": slowInnerDf], + "Inner": ["value": innerValueDf] + ] + def schema = TestUtil.schema(sdl, fetcherMap) + def graphQL = GraphQL.newGraphQL(schema).build() + + when: + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query("{ fast { value } slow { value } }") + .graphQLContext({ c -> + GraphQL.unusualConfiguration(c).cancellation().capturePartialResultsOnCancel(true) + }) + .build() + + def cf = graphQL.executeAsync(executionInput) + + // wait for slow DF to have started; by this point fast has already completed + slowStarted.await() + + // cancel while slow is still blocked; fast's fieldValue CF is already done + executionInput.cancel() + + // unblock slow so it doesn't hang forever + slowRelease.countDown() + + await().atMost(Duration.ofSeconds(10)).until({ -> cf.isDone() }) + def er = cf.join() + + then: + !cf.isCompletedExceptionally() + !er.errors.isEmpty() + er.errors.any { it["message"].contains("Execution has been asked to be cancelled") } + // fast field completed before cancel — its data should be present + er.data != null + er.data["fast"] != null + er.data["fast"]["value"] == "fast-value" + // slow field was cancelled — should be null + er.data["slow"] == null + } + + def "without capturePartialResultsOnCancel only the cancel error is returned"() { + def sdl = ''' + type Query { + fast : String + slow : String + } + ''' + + CountDownLatch fastDone = new CountDownLatch(1) + CountDownLatch slowLatch = new CountDownLatch(1) + + DataFetcher fastDf = { DataFetchingEnvironment env -> + return CompletableFuture.supplyAsync { + fastDone.countDown() + return "fast-value" + } + } + + DataFetcher slowDf = { DataFetchingEnvironment env -> + return CompletableFuture.supplyAsync { + slowLatch.await() + return "slow-value" + } + } + + def fetcherMap = ["Query": ["fast": fastDf, "slow": slowDf]] + def schema = TestUtil.schema(sdl, fetcherMap) + def graphQL = GraphQL.newGraphQL(schema).build() + + when: + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query("{ fast slow }") + .build() + + def cf = graphQL.executeAsync(executionInput) + + fastDone.await() + executionInput.cancel() + slowLatch.countDown() + + await().atMost(Duration.ofSeconds(10)).until({ -> cf.isDone() }) + def er = cf.join() + + then: + !cf.isCompletedExceptionally() + !er.errors.isEmpty() + er.errors[0]["message"] == "Execution has been asked to be cancelled" + // no partial data - old behaviour + er.data == null + } + private static ExecutionResult awaitAsync(GraphQL graphQL, ExecutionInput executionInput) { def cf = graphQL.executeAsync(executionInput) await().atMost(Duration.ofSeconds(10)).until({ -> cf.isDone() }) From 6247698d4cae6a5911989fada96290415405640b Mon Sep 17 00:00:00 2001 From: Alexandre Carlton Date: Tue, 26 May 2026 21:14:57 +1000 Subject: [PATCH 2/8] Lift out GraphQLContext from Async `Async` is intended to be a simple utility class - we thus just pass in the `CompletableFuture`s that are necessary to have this function. --- .../AbstractAsyncExecutionStrategy.java | 1 - src/main/java/graphql/execution/Async.java | 28 ++-- .../execution/AsyncExecutionStrategy.java | 9 +- .../graphql/execution/ExecutionStrategy.java | 23 ++- .../groovy/graphql/execution/AsyncTest.groovy | 141 ++++++++++++++++++ 5 files changed, 177 insertions(+), 25 deletions(-) diff --git a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java index 682e8b583a..fc97f6071b 100644 --- a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java @@ -2,7 +2,6 @@ import graphql.ExecutionResult; import graphql.ExecutionResultImpl; -import graphql.GraphQLContext; import graphql.PublicSpi; import java.util.concurrent.CompletionException; diff --git a/src/main/java/graphql/execution/Async.java b/src/main/java/graphql/execution/Async.java index aff242a261..2439ca03ed 100644 --- a/src/main/java/graphql/execution/Async.java +++ b/src/main/java/graphql/execution/Async.java @@ -1,9 +1,7 @@ package graphql.execution; import graphql.Assert; -import graphql.GraphQLContext; import graphql.Internal; -import graphql.GraphQLUnusualConfiguration; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; @@ -23,8 +21,6 @@ import java.util.stream.Collectors; import static graphql.Assert.assertTrue; -import static graphql.GraphQLUnusualConfiguration.CancellationConfig.CANCELLATION_FUTURE_KEY; -import static graphql.GraphQLUnusualConfiguration.CancellationConfig.CAPTURE_PARTIAL_RESULTS_ON_CANCEL; import static java.util.stream.Collectors.toList; @Internal @@ -62,15 +58,18 @@ public interface CombinedBuilder { CompletableFuture> await(); /** - * Like {@link #await()} but when {@link GraphQLUnusualConfiguration.CancellationConfig#CAPTURE_PARTIAL_RESULTS_ON_CANCEL} - * is enabled in the context and an {@link AbortExecutionException} causes a CF to fail, the already-completed - * CFs will have their values harvested and returned as partial results rather than completing exceptionally. + * Like {@link #await()} but races against the given cancellation future. If the cancellation future + * completes before all the tracked futures complete, the already-completed futures will have their + * values harvested and returned as partial results (with {@code null} for incomplete entries) + * rather than completing exceptionally. * - * @param graphQLContext the context to check for the partial results flag + *

If {@code cancellationFuture} is {@code null}, this behaves identically to {@link #await()}. + * + * @param cancellationFuture a future that, when completed, signals cancellation; may be {@code null} * * @return a CompletableFuture to a List of values (possibly partial on cancellation) */ - CompletableFuture> await(GraphQLContext graphQLContext); + CompletableFuture> await(@Nullable CompletableFuture cancellationFuture); /** * This will return a {@code CompletableFuture>} if ANY of the input values are async @@ -120,7 +119,7 @@ public CompletableFuture> await() { } @Override - public CompletableFuture> await(GraphQLContext graphQLContext) { + public CompletableFuture> await(@Nullable CompletableFuture cancellationFuture) { return await(); } @@ -170,7 +169,7 @@ public CompletableFuture> await() { } @Override - public CompletableFuture> await(GraphQLContext graphQLContext) { + public CompletableFuture> await(@Nullable CompletableFuture cancellationFuture) { return await(); } @@ -259,16 +258,11 @@ public CompletableFuture> await() { @SuppressWarnings("unchecked") @Override - public CompletableFuture> await(GraphQLContext graphQLContext) { + public CompletableFuture> await(@Nullable CompletableFuture cancellationFuture) { commonSizeAssert(); if (cfCount == 0) { return CompletableFuture.completedFuture(materialisedList(array)); } - if (!graphQLContext.getBoolean(CAPTURE_PARTIAL_RESULTS_ON_CANCEL)) { - return await(); - } - - CompletableFuture cancellationFuture = graphQLContext.get(CANCELLATION_FUTURE_KEY); if (cancellationFuture == null) { return await(); } diff --git a/src/main/java/graphql/execution/AsyncExecutionStrategy.java b/src/main/java/graphql/execution/AsyncExecutionStrategy.java index 0d4c94d20d..cd06d31c2f 100644 --- a/src/main/java/graphql/execution/AsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncExecutionStrategy.java @@ -1,7 +1,6 @@ package graphql.execution; import graphql.ExecutionResult; -import graphql.GraphQLContext; import graphql.PublicApi; import graphql.execution.incremental.DeferredExecutionSupport; import org.jspecify.annotations.NullMarked; @@ -68,8 +67,8 @@ public CompletableFuture execute(ExecutionContext executionCont CompletableFuture overallResult = new CompletableFuture<>(); executionStrategyCtx.onDispatched(); - GraphQLContext graphQLContext = executionContext.getGraphQLContext(); - futures.await(graphQLContext).whenComplete((completeValueInfos, throwable) -> { + CompletableFuture cancelCF = getCancellationFuture(executionContext); + futures.await(cancelCF).whenComplete((completeValueInfos, throwable) -> { List fieldsExecutedOnInitialResult = deferredExecutionSupport.getNonDeferredFieldNames(fieldNames); throwable = executionContext.possibleCancellation(throwable); @@ -93,7 +92,7 @@ public CompletableFuture execute(ExecutionContext executionCont executionStrategyCtx.onFieldValuesInfo(nonNullValueInfos); // Let handleResultsWithPartialData add the error to avoid duplication BiConsumer, Throwable> fieldValuesConsumer = handleResultsWithPartialData(executionContext, fieldsExecutedOnInitialResult, overallResult); - fieldValuesFutures.await(graphQLContext).whenComplete(fieldValuesConsumer); + fieldValuesFutures.await(cancelCF).whenComplete(fieldValuesConsumer); return; } Throwable cause = throwable instanceof CompletionException ? throwable.getCause() : throwable; @@ -109,7 +108,7 @@ public CompletableFuture execute(ExecutionContext executionCont executionStrategyCtx.onFieldValuesInfo(completeValueInfos); BiConsumer, Throwable> fieldValuesConsumer = handleResultsWithPartialData(executionContext, fieldsExecutedOnInitialResult, overallResult); - fieldValuesFutures.await(graphQLContext).whenComplete(fieldValuesConsumer); + fieldValuesFutures.await(cancelCF).whenComplete(fieldValuesConsumer); }).exceptionally((ex) -> { // if there are any issues with combining/handling the field results, // complete the future at all costs and bubble up any thrown exception so diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index fe51143a0f..b7bad18b63 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -47,6 +47,7 @@ import graphql.schema.LightDataFetcher; import graphql.util.FpKit; import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; import java.util.ArrayList; import java.util.Collections; @@ -59,6 +60,7 @@ import java.util.function.Function; import java.util.function.Supplier; +import static graphql.GraphQLUnusualConfiguration.CancellationConfig.CANCELLATION_FUTURE_KEY; import static graphql.GraphQLUnusualConfiguration.CancellationConfig.CAPTURE_PARTIAL_RESULTS_ON_CANCEL; import static graphql.execution.Async.exceptionallyCompletedFuture; import static graphql.execution.FieldCollectorParameters.newParameters; @@ -220,7 +222,7 @@ protected Object executeObject(ExecutionContext executionContext, ExecutionStrat resolveObjectCtx.onDispatched(); - GraphQLContext graphQLContext = executionContext.getGraphQLContext(); + CompletableFuture cancelCF = getCancellationFuture(executionContext); Object fieldValueInfosResult = resolvedFieldFutures.awaitPolymorphic(); if (fieldValueInfosResult instanceof CompletableFuture) { CompletableFuture> fieldValueInfos = (CompletableFuture>) fieldValueInfosResult; @@ -235,7 +237,7 @@ protected Object executeObject(ExecutionContext executionContext, ExecutionStrat Async.CombinedBuilder resultFutures = fieldValuesCombinedBuilder(completeValueInfos); dataLoaderDispatcherStrategy.executeObjectOnFieldValuesInfo(completeValueInfos, parameters); resolveObjectCtx.onFieldValuesInfo(completeValueInfos); - resultFutures.await(graphQLContext).whenComplete(handleResultsConsumer); + resultFutures.await(cancelCF).whenComplete(handleResultsConsumer); }).exceptionally((ex) -> { // if there are any issues with combining/handling the field results, // complete the future at all costs and bubble up any thrown exception so @@ -280,6 +282,23 @@ protected boolean capturePartialResults(ExecutionContext executionContext) { return executionContext.getGraphQLContext().getBoolean(CAPTURE_PARTIAL_RESULTS_ON_CANCEL); } + /** + * Returns the cancellation future from the execution context if partial result capture is enabled, + * or {@code null} otherwise. This is used to pass into {@link Async.CombinedBuilder#await(CompletableFuture)} + * so that the generic utility can race against cancellation without needing to know about GraphQL context. + * + * @param executionContext the execution context + * + * @return the cancellation future, or {@code null} if partial results on cancel is not enabled + */ + protected @Nullable CompletableFuture getCancellationFuture(ExecutionContext executionContext) { + GraphQLContext graphQLContext = executionContext.getGraphQLContext(); + if (!graphQLContext.getBoolean(CAPTURE_PARTIAL_RESULTS_ON_CANCEL)) { + return null; + } + return graphQLContext.get(CANCELLATION_FUTURE_KEY); + } + private BiConsumer, Throwable> buildFieldValueMap(List fieldNames, CompletableFuture> overallResult, ExecutionContext executionContext) { return (List results, Throwable exception) -> { exception = executionContext.possibleCancellation(exception); diff --git a/src/test/groovy/graphql/execution/AsyncTest.groovy b/src/test/groovy/graphql/execution/AsyncTest.groovy index 2661c4f5fd..b8ad6324b3 100644 --- a/src/test/groovy/graphql/execution/AsyncTest.groovy +++ b/src/test/groovy/graphql/execution/AsyncTest.groovy @@ -421,4 +421,145 @@ class AsyncTest extends Specification { return awaited } } + + def "await with null cancelCF behaves like plain await"() { + when: + def asyncBuilder = Async.ofExpectedSize(3) + asyncBuilder.add(completedFuture("A")) + asyncBuilder.add(completedFuture("B")) + asyncBuilder.add(completedFuture("C")) + def list = asyncBuilder.await((CompletableFuture) null).join() + + then: + list == ["A", "B", "C"] + } + + def "await with cancelCF returns all results when all CFs complete before cancellation"() { + when: + def cancelCF = new CompletableFuture() + def asyncBuilder = Async.ofExpectedSize(3) + asyncBuilder.add(completedFuture("A")) + asyncBuilder.add(completedFuture("B")) + asyncBuilder.add(completedFuture("C")) + def list = asyncBuilder.await(cancelCF).join() + + then: + list == ["A", "B", "C"] + } + + def "await with cancelCF returns partial results when cancellation fires before all CFs complete"() { + when: + def cancelCF = new CompletableFuture() + def pending1 = new CompletableFuture() + def pending2 = new CompletableFuture() + + def asyncBuilder = Async.ofExpectedSize(4) + asyncBuilder.add(completedFuture("A")) + asyncBuilder.add(pending1) + asyncBuilder.add(completedFuture("C")) + asyncBuilder.add(pending2) + + def resultCF = asyncBuilder.await(cancelCF) + + // cancel before pending CFs complete + cancelCF.complete(null) + + def list = resultCF.join() + + then: + list == ["A", null, "C", null] + } + + def "await with cancelCF returns partial results with mixed objects and CFs"() { + when: + def cancelCF = new CompletableFuture() + def pending = new CompletableFuture() + + def asyncBuilder = Async.ofExpectedSize(4) + asyncBuilder.addObject("A") + asyncBuilder.add(completedFuture("B")) + asyncBuilder.add(pending) + asyncBuilder.addObject("D") + + def resultCF = asyncBuilder.await(cancelCF) + + cancelCF.complete(null) + def list = resultCF.join() + + then: + list == ["A", "B", null, "D"] + } + + def "await with cancelCF returns full results when all CFs complete even if cancelCF completes later"() { + when: + def cancelCF = new CompletableFuture() + def cf1 = new CompletableFuture() + def cf2 = new CompletableFuture() + + def asyncBuilder = Async.ofExpectedSize(2) + asyncBuilder.add(cf1) + asyncBuilder.add(cf2) + + def resultCF = asyncBuilder.await(cancelCF) + + // complete all CFs before cancellation + cf1.complete("X") + cf2.complete("Y") + + def list = resultCF.join() + + then: + list == ["X", "Y"] + } + + def "await with cancelCF propagates exception if a CF fails before cancellation"() { + when: + def cancelCF = new CompletableFuture() + def failing = new CompletableFuture() + failing.completeExceptionally(new RuntimeException("boom")) + + def asyncBuilder = Async.ofExpectedSize(2) + asyncBuilder.add(completedFuture("A")) + asyncBuilder.add(failing) + + def resultCF = asyncBuilder.await(cancelCF) + resultCF.join() + + then: + thrown(CompletionException) + } + + def "await with cancelCF works with all materialised values"() { + when: + def cancelCF = new CompletableFuture() + def asyncBuilder = Async.ofExpectedSize(3) + asyncBuilder.addObject("A") + asyncBuilder.addObject("B") + asyncBuilder.addObject("C") + def list = asyncBuilder.await(cancelCF).join() + + then: + list == ["A", "B", "C"] + } + + def "await with cancelCF on empty builder returns empty list"() { + when: + def cancelCF = new CompletableFuture() + def asyncBuilder = Async.ofExpectedSize(0) + def list = asyncBuilder.await(cancelCF).join() + + then: + list == [] + } + + def "await with cancelCF on single builder delegates to plain await"() { + when: + def cancelCF = new CompletableFuture() + def asyncBuilder = Async.ofExpectedSize(1) + asyncBuilder.add(completedFuture("A")) + def list = asyncBuilder.await(cancelCF).join() + + then: + list == ["A"] + } } From 4dd9643d6e1b20ed76eff1644840df270b1a2db8 Mon Sep 17 00:00:00 2001 From: Alexandre Carlton Date: Wed, 27 May 2026 21:32:45 +1000 Subject: [PATCH 3/8] Flesh out test cases fro AsyncTest --- .../execution/AsyncExecutionStrategy.java | 2 +- .../groovy/graphql/execution/AsyncTest.groovy | 21 +++++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/main/java/graphql/execution/AsyncExecutionStrategy.java b/src/main/java/graphql/execution/AsyncExecutionStrategy.java index cd06d31c2f..5bb1c098cf 100644 --- a/src/main/java/graphql/execution/AsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncExecutionStrategy.java @@ -74,7 +74,7 @@ public CompletableFuture execute(ExecutionContext executionCont throwable = executionContext.possibleCancellation(throwable); if (throwable != null) { - if (capturePartialResults(executionContext) && completeValueInfos != null) { + if (completeValueInfos != null && capturePartialResults(executionContext)) { // partial results: some FieldValueInfos completed before cancel - build with those // null entries mean that FieldValueInfo CF wasn't done yet (field was cancelled) Async.CombinedBuilder fieldValuesFutures = Async.ofExpectedSize(completeValueInfos.size()); diff --git a/src/test/groovy/graphql/execution/AsyncTest.groovy b/src/test/groovy/graphql/execution/AsyncTest.groovy index b8ad6324b3..7a86065455 100644 --- a/src/test/groovy/graphql/execution/AsyncTest.groovy +++ b/src/test/groovy/graphql/execution/AsyncTest.groovy @@ -508,8 +508,14 @@ class AsyncTest extends Specification { def list = resultCF.join() - then: + then: "full results returned despite cancel firing after" list == ["X", "Y"] + + when: "cancelCF completes after all CFs - should not affect result" + cancelCF.complete(null) + + then: "result is unchanged" + resultCF.join() == ["X", "Y"] } def "await with cancelCF propagates exception if a CF fails before cancellation"() { @@ -553,12 +559,23 @@ class AsyncTest extends Specification { } def "await with cancelCF on single builder delegates to plain await"() { - when: + when: "single builder with a completed CF" def cancelCF = new CompletableFuture() def asyncBuilder = Async.ofExpectedSize(1) asyncBuilder.add(completedFuture("A")) def list = asyncBuilder.await(cancelCF).join() + then: "result is returned normally - cancellation is not raced for single elements" + list == ["A"] + } + + def "await with cancelCF on single builder with materialised value delegates to plain await"() { + when: + def cancelCF = new CompletableFuture() + def asyncBuilder = Async.ofExpectedSize(1) + asyncBuilder.addObject("A") + def list = asyncBuilder.await(cancelCF).join() + then: list == ["A"] } From 2ae7c7c443600c8ab70a29581ba3593edfc59273 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Fri, 29 May 2026 09:03:12 +1000 Subject: [PATCH 4/8] Simplify Async.Many.await(cancellationFuture) Collapse the two whenComplete callbacks into a single one on anyOf(allOf, cancellationFuture). The previous second callback on allOf was unreachable dead code - anyOf already fires when either future completes - and the overallResult.isDone() guards could never be true, hurting branch coverage. Merge harvestPartialResults and collectAllResults into a single harvestResults helper: when allOf has won the race every field future is done so it yields the full result, and when cancellation wins it yields the partial result with null for not-yet-done futures. join() is safe because allOf being incomplete implies no field future has failed. All new code is now 100% line and branch covered. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/main/java/graphql/execution/Async.java | 74 ++++--------------- .../groovy/graphql/execution/AsyncTest.groovy | 11 +++ 2 files changed, 26 insertions(+), 59 deletions(-) diff --git a/src/main/java/graphql/execution/Async.java b/src/main/java/graphql/execution/Async.java index 2439ca03ed..be8d49fb30 100644 --- a/src/main/java/graphql/execution/Async.java +++ b/src/main/java/graphql/execution/Async.java @@ -268,79 +268,35 @@ public CompletableFuture> await(@Nullable CompletableFuture cancel } CompletableFuture> overallResult = new CompletableFuture<>(); - CompletableFuture[] cfsArr = copyOnlyCFsToArray(); - CompletableFuture allOf = CompletableFuture.allOf(cfsArr); - - // race: either all CFs complete normally, or cancellation fires first - CompletableFuture.anyOf(allOf, cancellationFuture) - .whenComplete((ignored, exception) -> { - if (overallResult.isDone()) { - return; - } - if (exception != null) { - // a CF failed — not our abort path, propagate - overallResult.completeExceptionally(exception); - return; - } - if (!allOf.isDone()) { - // cancellation fired before allOf: harvest already-completed CFs - List partialResults = harvestPartialResults(array); - overallResult.complete(partialResults); - return; - } - // allOf finished normally: collect all results - overallResult.complete(collectAllResults(array, cfsArr)); - }); - - // also handle when allOf completes (either normally or exceptionally) after the race - allOf.whenComplete((ignored, exception) -> { - if (overallResult.isDone()) { - return; - } + CompletableFuture allOf = CompletableFuture.allOf(copyOnlyCFsToArray()); + + // Race "all field futures complete" against cancellation. The cancellation future always + // completes normally (see ExecutionInput#cancel), so anyOf can only complete exceptionally + // when a field future fails - in which case we propagate that failure. + CompletableFuture.anyOf(allOf, cancellationFuture).whenComplete((ignored, exception) -> { if (exception != null) { - // a CF in allOf failed — propagate overallResult.completeExceptionally(exception); return; } - overallResult.complete(collectAllResults(array, cfsArr)); + // Either every field future is done (allOf won) or cancellation won the race. In both + // cases we harvest whatever has completed; field futures that are not yet done become + // null. join() is safe here: if allOf is not done then no field future has failed (a + // failure would have completed allOf exceptionally and taken the branch above). + overallResult.complete(harvestResults(array)); }); return overallResult; } @SuppressWarnings("unchecked") - private List harvestPartialResults(Object[] array) { - List partialResults = new ArrayList<>(array.length); + private List harvestResults(Object[] array) { + List results = new ArrayList<>(array.length); for (Object object : array) { if (object instanceof CompletableFuture) { CompletableFuture cf = (CompletableFuture) object; - if (cf.isDone() && !cf.isCompletedExceptionally()) { - partialResults.add(cf.join()); - } else { - partialResults.add(null); - } + results.add(cf.isDone() ? cf.join() : null); } else { - partialResults.add((T) object); - } - } - return partialResults; - } - - @SuppressWarnings("unchecked") - private List collectAllResults(Object[] array, CompletableFuture[] cfsArr) { - List results = new ArrayList<>(array.length); - if (cfsArr.length == array.length) { - for (CompletableFuture cf : cfsArr) { - results.add(cf.join()); - } - } else { - for (Object object : array) { - if (object instanceof CompletableFuture) { - CompletableFuture cf = (CompletableFuture) object; - results.add(cf.join()); - } else { - results.add((T) object); - } + results.add((T) object); } } return results; diff --git a/src/test/groovy/graphql/execution/AsyncTest.groovy b/src/test/groovy/graphql/execution/AsyncTest.groovy index 7a86065455..cef87ceec5 100644 --- a/src/test/groovy/graphql/execution/AsyncTest.groovy +++ b/src/test/groovy/graphql/execution/AsyncTest.groovy @@ -548,6 +548,17 @@ class AsyncTest extends Specification { list == ["A", "B", "C"] } + def "await with null cancelCF delegates to plain await"() { + when: "a many builder is awaited with a null cancellation future" + def asyncBuilder = Async.ofExpectedSize(2) + asyncBuilder.add(completedFuture("A")) + asyncBuilder.add(completedFuture("B")) + def list = asyncBuilder.await((CompletableFuture) null).join() + + then: "it behaves identically to await() and returns all results" + list == ["A", "B"] + } + def "await with cancelCF on empty builder returns empty list"() { when: def cancelCF = new CompletableFuture() From 05149ce4b9cdf5e5eb4cd926c74a0fd27dc9002f Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Fri, 29 May 2026 10:43:13 +1000 Subject: [PATCH 5/8] Remove dead partial-results code in handleResultsWithPartialData The exception != null branch tried to capture partial results, but a whenComplete((value, throwable)) callback always delivers a null value when a throwable is present, so the results != null guard was never true and that capture branch was unreachable (JaCoCo confirmed zero coverage). The exception path now simply delegates to handleNonNullException. Also drop two dead CompletionException unwraps - possibleCancellation returns a raw AbortExecutionException, never a wrapped one - and the always-true instanceof/results != null checks on the cancellation path. No behavioural change (full suite passes); the method is now 100% line and branch covered. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../AbstractAsyncExecutionStrategy.java | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java index fc97f6071b..cce54de90b 100644 --- a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java @@ -4,8 +4,6 @@ import graphql.ExecutionResultImpl; import graphql.PublicSpi; -import java.util.concurrent.CompletionException; - import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; @@ -37,28 +35,18 @@ protected BiConsumer, Throwable> handleResults(ExecutionContext exe protected BiConsumer, Throwable> handleResultsWithPartialData(ExecutionContext executionContext, List fieldNames, CompletableFuture overallResult) { return (List results, Throwable exception) -> { - // when partial results on cancel is enabled the results list will already have partial data - // (already-completed fields) so we can build a partial response even if exception is set if (exception != null) { - Throwable cause = exception instanceof CompletionException ? exception.getCause() : exception; - if (cause instanceof AbortExecutionException && results != null - && capturePartialResults(executionContext)) { - executionContext.addError((AbortExecutionException) cause); - completeResultFuture(overallResult, executionContext, fieldNames, results); - return; - } handleNonNullException(executionContext, overallResult, exception); return; } - // check if cancel fired while results were being gathered (no exception, but cancelled) + // No exception, but cancellation may have fired while the already-completed field values + // were being gathered. When it has, the results list already holds the partial data from + // the fields that completed before cancellation, so we can return it alongside the error. Throwable cancelException = executionContext.possibleCancellation(null); if (cancelException != null) { - Throwable cancelCause = cancelException instanceof CompletionException ? cancelException.getCause() : cancelException; - if (cancelCause instanceof AbortExecutionException && results != null - && capturePartialResults(executionContext)) { - // we have partial data from already-completed CFs — use it - executionContext.addError((AbortExecutionException) cancelCause); + if (capturePartialResults(executionContext)) { + executionContext.addError((AbortExecutionException) cancelException); completeResultFuture(overallResult, executionContext, fieldNames, results); } else { handleNonNullException(executionContext, overallResult, cancelException); From 935bd483cf03741b774ae416ab80ec522115ff96 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Fri, 29 May 2026 11:36:16 +1000 Subject: [PATCH 6/8] Unify handleResults and extract completeFieldValues handleResultsWithPartialData differed from handleResults in exactly one case: a cancellation that fired after some fields completed shows up as a synthesised AbortExecutionException with a non-null results list (a real field failure always carries null results). Fold that single case into handleResults via a `results != null && capturePartialResults` check and delete handleResultsWithPartialData. In AsyncExecutionStrategy.execute the partial-results-on-cancel branch and the normal branch both built field values, fired the instrumentation callbacks and awaited the same way - only differing in tolerating null FieldValueInfo entries. Extract that into completeFieldValues, used by both paths, shrinking the execute() lambda back to roughly its pre-feature shape. The instrumentation list is only copied/filtered when null entries are actually present, so the common path stays allocation-free. Also drop a redundant CompletionException unwrap (handleNonNullException already unwraps) and the now-unused imports. Add a test for the previously-untested behaviour where, with capture disabled, a late cancel discards even fully gathered nested data. All changed code is 100% line and branch covered; full suite passes. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../AbstractAsyncExecutionStrategy.java | 32 ++------ .../execution/AsyncExecutionStrategy.java | 80 +++++++++++-------- .../groovy/graphql/ExecutionInputTest.groovy | 54 +++++++++++++ 3 files changed, 107 insertions(+), 59 deletions(-) diff --git a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java index cce54de90b..6dc606426c 100644 --- a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java @@ -25,32 +25,16 @@ protected BiConsumer, Throwable> handleResults(ExecutionContext exe exception = executionContext.possibleCancellation(exception); if (exception != null) { - handleNonNullException(executionContext, overallResult, exception); - return; - } - - completeResultFuture(overallResult, executionContext, fieldNames, results); - }; - } - - protected BiConsumer, Throwable> handleResultsWithPartialData(ExecutionContext executionContext, List fieldNames, CompletableFuture overallResult) { - return (List results, Throwable exception) -> { - if (exception != null) { - handleNonNullException(executionContext, overallResult, exception); - return; - } - - // No exception, but cancellation may have fired while the already-completed field values - // were being gathered. When it has, the results list already holds the partial data from - // the fields that completed before cancellation, so we can return it alongside the error. - Throwable cancelException = executionContext.possibleCancellation(null); - if (cancelException != null) { - if (capturePartialResults(executionContext)) { - executionContext.addError((AbortExecutionException) cancelException); + // A cancellation that fired after some fields already completed arrives here as a + // synthesised AbortExecutionException with a non-null results list (a real field + // failure always has null results). When partial capture is enabled we keep those + // results and attach the cancellation error; otherwise we report the error as usual. + if (results != null && capturePartialResults(executionContext)) { + executionContext.addError((AbortExecutionException) exception); completeResultFuture(overallResult, executionContext, fieldNames, results); - } else { - handleNonNullException(executionContext, overallResult, cancelException); + return; } + handleNonNullException(executionContext, overallResult, exception); return; } diff --git a/src/main/java/graphql/execution/AsyncExecutionStrategy.java b/src/main/java/graphql/execution/AsyncExecutionStrategy.java index 5bb1c098cf..580176d484 100644 --- a/src/main/java/graphql/execution/AsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncExecutionStrategy.java @@ -4,6 +4,7 @@ import graphql.PublicApi; import graphql.execution.incremental.DeferredExecutionSupport; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext; import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters; @@ -13,8 +14,6 @@ import java.util.Objects; import java.util.Optional; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; -import java.util.function.BiConsumer; import java.util.stream.Collectors; /** @@ -73,42 +72,17 @@ public CompletableFuture execute(ExecutionContext executionCont throwable = executionContext.possibleCancellation(throwable); - if (throwable != null) { - if (completeValueInfos != null && capturePartialResults(executionContext)) { - // partial results: some FieldValueInfos completed before cancel - build with those - // null entries mean that FieldValueInfo CF wasn't done yet (field was cancelled) - Async.CombinedBuilder fieldValuesFutures = Async.ofExpectedSize(completeValueInfos.size()); - for (FieldValueInfo completeValueInfo : completeValueInfos) { - if (completeValueInfo != null) { - fieldValuesFutures.addObject(completeValueInfo.getFieldValueObject()); - } else { - fieldValuesFutures.addObject((Object) null); - } - } - List nonNullValueInfos = completeValueInfos.stream() - .filter(Objects::nonNull) - .collect(Collectors.toList()); - dataLoaderDispatcherStrategy.executionStrategyOnFieldValuesInfo(nonNullValueInfos, parameters); - executionStrategyCtx.onFieldValuesInfo(nonNullValueInfos); - // Let handleResultsWithPartialData add the error to avoid duplication - BiConsumer, Throwable> fieldValuesConsumer = handleResultsWithPartialData(executionContext, fieldsExecutedOnInitialResult, overallResult); - fieldValuesFutures.await(cancelCF).whenComplete(fieldValuesConsumer); - return; - } - Throwable cause = throwable instanceof CompletionException ? throwable.getCause() : throwable; - handleResults(executionContext, fieldsExecutedOnInitialResult, overallResult).accept(null, cause); + if (throwable != null && !(completeValueInfos != null && capturePartialResults(executionContext))) { + // a genuine field failure, or a cancellation we cannot surface partial results for: + // there is nothing usable to return, so just report the error + handleResults(executionContext, fieldsExecutedOnInitialResult, overallResult).accept(null, throwable); return; } - Async.CombinedBuilder fieldValuesFutures = Async.ofExpectedSize(completeValueInfos.size()); - for (FieldValueInfo completeValueInfo : completeValueInfos) { - fieldValuesFutures.addObject(completeValueInfo.getFieldValueObject()); - } - dataLoaderDispatcherStrategy.executionStrategyOnFieldValuesInfo(completeValueInfos, parameters); - executionStrategyCtx.onFieldValuesInfo(completeValueInfos); - - BiConsumer, Throwable> fieldValuesConsumer = handleResultsWithPartialData(executionContext, fieldsExecutedOnInitialResult, overallResult); - fieldValuesFutures.await(cancelCF).whenComplete(fieldValuesConsumer); + // normal completion, or partial-results-on-cancel: completeValueInfos holds the + // FieldValueInfos that completed (with null entries for any cancelled before completing) + completeFieldValues(executionContext, parameters, executionStrategyCtx, dataLoaderDispatcherStrategy, + completeValueInfos, fieldsExecutedOnInitialResult, cancelCF, overallResult); }).exceptionally((ex) -> { // if there are any issues with combining/handling the field results, // complete the future at all costs and bubble up any thrown exception so @@ -123,4 +97,40 @@ public CompletableFuture execute(ExecutionContext executionCont return overallResult; } + /** + * Turns the completed {@link FieldValueInfo}s into field values and completes the {@code overallResult}. + *

+ * When partial-results-on-cancel is in play {@code completeValueInfos} may contain {@code null} + * entries for fields that were cancelled before they completed; those become {@code null} field + * values and are excluded from the instrumentation callbacks. + */ + @SuppressWarnings("FutureReturnValueIgnored") + private void completeFieldValues(ExecutionContext executionContext, + ExecutionStrategyParameters parameters, + ExecutionStrategyInstrumentationContext executionStrategyCtx, + DataLoaderDispatchStrategy dataLoaderDispatcherStrategy, + List completeValueInfos, + List fieldNames, + @Nullable CompletableFuture cancelCF, + CompletableFuture overallResult) { + Async.CombinedBuilder fieldValuesFutures = Async.ofExpectedSize(completeValueInfos.size()); + boolean hasNulls = false; + for (FieldValueInfo completeValueInfo : completeValueInfos) { + if (completeValueInfo != null) { + fieldValuesFutures.addObject(completeValueInfo.getFieldValueObject()); + } else { + hasNulls = true; + fieldValuesFutures.addObject((Object) null); + } + } + // null entries only occur for partial-results-on-cancel; the instrumentation callbacks should + // not see them, so filter only when needed and otherwise pass the list straight through + List valueInfosForInstrumentation = hasNulls + ? completeValueInfos.stream().filter(Objects::nonNull).collect(Collectors.toList()) + : completeValueInfos; + dataLoaderDispatcherStrategy.executionStrategyOnFieldValuesInfo(valueInfosForInstrumentation, parameters); + executionStrategyCtx.onFieldValuesInfo(valueInfosForInstrumentation); + fieldValuesFutures.await(cancelCF).whenComplete(handleResults(executionContext, fieldNames, overallResult)); + } + } diff --git a/src/test/groovy/graphql/ExecutionInputTest.groovy b/src/test/groovy/graphql/ExecutionInputTest.groovy index 84819950b4..d12ce4db73 100644 --- a/src/test/groovy/graphql/ExecutionInputTest.groovy +++ b/src/test/groovy/graphql/ExecutionInputTest.groovy @@ -623,6 +623,60 @@ class ExecutionInputTest extends Specification { er.data == null } + def "without capturePartialResultsOnCancel a late cancel discards even fully gathered nested data"() { + // capture is OFF, so there is no cancellation race; the top-level 'outer' field value info + // completes quickly while its nested 'value' is still resolving. Cancelling at that point and + // then letting 'value' finish means the field values are fully gathered, but because capture + // is disabled the gathered data must be discarded and only the cancel error returned. + def sdl = ''' + type Query { + outer : Inner + } + type Inner { + value : String + } + ''' + + CountDownLatch valueStarted = new CountDownLatch(1) + CountDownLatch valueRelease = new CountDownLatch(1) + + DataFetcher outerDf = { DataFetchingEnvironment env -> [:] } + DataFetcher valueDf = { DataFetchingEnvironment env -> + return CompletableFuture.supplyAsync { + valueStarted.countDown() + valueRelease.await() + return "value" + } + } + + def fetcherMap = ["Query": ["outer": outerDf], "Inner": ["value": valueDf]] + def schema = TestUtil.schema(sdl, fetcherMap) + def graphQL = GraphQL.newGraphQL(schema).build() + + when: + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query("{ outer { value } }") + .build() // capturePartialResultsOnCancel NOT enabled + + def cf = graphQL.executeAsync(executionInput) + + // the nested value DF has started, so 'outer' field value info is already done + valueStarted.await() + // cancel while the nested value is still resolving, then let it finish + executionInput.cancel() + valueRelease.countDown() + + await().atMost(Duration.ofSeconds(10)).until({ -> cf.isDone() }) + def er = cf.join() + + then: + !cf.isCompletedExceptionally() + !er.errors.isEmpty() + er.errors[0]["message"] == "Execution has been asked to be cancelled" + // capture disabled - even though the data was fully gathered, it is discarded + er.data == null + } + private static ExecutionResult awaitAsync(GraphQL graphQL, ExecutionInput executionInput) { def cf = graphQL.executeAsync(executionInput) await().atMost(Duration.ofSeconds(10)).until({ -> cf.isDone() }) From fecf007f750efffc0cebeaef8fcfa5ce50336191 Mon Sep 17 00:00:00 2001 From: bbaker Date: Fri, 29 May 2026 17:47:42 +1000 Subject: [PATCH 7/8] Oops bad merge via web --- src/main/java/graphql/execution/Async.java | 51 ---------------------- 1 file changed, 51 deletions(-) diff --git a/src/main/java/graphql/execution/Async.java b/src/main/java/graphql/execution/Async.java index 83123f585a..4b62c2fa8e 100644 --- a/src/main/java/graphql/execution/Async.java +++ b/src/main/java/graphql/execution/Async.java @@ -195,11 +195,6 @@ public CompletableFuture> await(@Nullable CompletableFuture cancel return CompletableFuture.completedFuture(Collections.singletonList((T) value)); } - @Override - public CompletableFuture> await(@Nullable CompletableFuture cancellationFuture) { - return await(); - } - @Override public Object awaitPolymorphic() { commonSizeAssert(); @@ -328,52 +323,6 @@ private List harvestResults(Object[] array) { return results; } - @SuppressWarnings("unchecked") - @Override - public CompletableFuture> await(@Nullable CompletableFuture cancellationFuture) { - commonSizeAssert(); - if (cfCount == 0) { - return CompletableFuture.completedFuture(materialisedList(array)); - } - if (cancellationFuture == null) { - return await(); - } - - CompletableFuture> overallResult = new CompletableFuture<>(); - CompletableFuture allOf = CompletableFuture.allOf(copyOnlyCFsToArray()); - - // Race "all field futures complete" against cancellation. The cancellation future always - // completes normally (see ExecutionInput#cancel), so anyOf can only complete exceptionally - // when a field future fails - in which case we propagate that failure. - CompletableFuture.anyOf(allOf, cancellationFuture).whenComplete((ignored, exception) -> { - if (exception != null) { - overallResult.completeExceptionally(exception); - return; - } - // Either every field future is done (allOf won) or cancellation won the race. In both - // cases we harvest whatever has completed; field futures that are not yet done become - // null. join() is safe here: if allOf is not done then no field future has failed (a - // failure would have completed allOf exceptionally and taken the branch above). - overallResult.complete(harvestResults(array)); - }); - - return overallResult; - } - - @SuppressWarnings("unchecked") - private List harvestResults(Object[] array) { - List results = new ArrayList<>(array.length); - for (Object object : array) { - if (object instanceof CompletableFuture) { - CompletableFuture cf = (CompletableFuture) object; - results.add(cf.isDone() ? cf.join() : null); - } else { - results.add((T) object); - } - } - return results; - } - @SuppressWarnings("unchecked") @NonNull private CompletableFuture[] copyOnlyCFsToArray() { From 8382a92d6b5cfae3879d41e8b33071507fcfcc47 Mon Sep 17 00:00:00 2001 From: Alexandre Carlton Date: Sun, 14 Jun 2026 21:42:16 +1000 Subject: [PATCH 8/8] Add test for nested object field --- .../groovy/graphql/ExecutionInputTest.groovy | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/test/groovy/graphql/ExecutionInputTest.groovy b/src/test/groovy/graphql/ExecutionInputTest.groovy index d12ce4db73..206d3089f9 100644 --- a/src/test/groovy/graphql/ExecutionInputTest.groovy +++ b/src/test/groovy/graphql/ExecutionInputTest.groovy @@ -623,6 +623,74 @@ class ExecutionInputTest extends Specification { er.data == null } + def "capturePartialResultsOnCancel returns partial data when nested object sub-field is cancelled"() { + // This exercises the buildFieldValueMap lambda inside ExecutionStrategy.executeObject() + // for a nested object type. The parent DF returns a map synchronously so the nested + // object's executeObject starts immediately. Inside the nested executeObject, 'fast' + // completes while 'slow' blocks. Cancellation fires at the await(cancelCF) inside + // executeObject for the nested Parent type, triggering the partial-results branch in + // buildFieldValueMap. + def sdl = ''' + type Query { + parent : Parent + } + type Parent { + fast : String + slow : String + } + ''' + + CountDownLatch slowStarted = new CountDownLatch(1) + CompletableFuture slowResult = new CompletableFuture<>() + + DataFetcher parentDf = { DataFetchingEnvironment env -> [:] } + DataFetcher fastDf = { DataFetchingEnvironment env -> "fast-value" } + DataFetcher slowDf = { DataFetchingEnvironment env -> + slowStarted.countDown() + return slowResult + } + + def fetcherMap = [ + "Query" : ["parent": parentDf], + "Parent": ["fast": fastDf, "slow": slowDf] + ] + def schema = TestUtil.schema(sdl, fetcherMap) + def graphQL = GraphQL.newGraphQL(schema).build() + + when: + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query("{ parent { fast slow } }") + .graphQLContext({ c -> + GraphQL.unusualConfiguration(c).cancellation().capturePartialResultsOnCancel(true) + }) + .build() + + def cf = graphQL.executeAsync(executionInput) + + // wait for the slow DF to have been invoked — fast is already done synchronously + slowStarted.await() + // cancel while slow's CF is still pending; this completes the cancellation future + // which races against the pending slowResult inside await(cancelCF) + executionInput.cancel() + + await().atMost(Duration.ofSeconds(10)).until({ -> cf.isDone() }) + def er = cf.join() + + then: + !cf.isCompletedExceptionally() + !er.errors.isEmpty() + er.errors.any { it["message"] == "Execution has been asked to be cancelled" } + // The top-level partial results capture sees parent's CF as still pending + // (because its nested sub-fields are being resolved), so parent is null. + // The nested buildFieldValueMap lambda also fires on the same cancellation. + er.data != null + er.data.containsKey("parent") + + cleanup: + // release slow so any background threads can finish + slowResult.complete("slow-value") + } + def "without capturePartialResultsOnCancel a late cancel discards even fully gathered nested data"() { // capture is OFF, so there is no cancellation race; the top-level 'outer' field value info // completes quickly while its nested 'value' is still resolving. Cancelling at that point and