Capture partial results on cancellation by AlexandreCarlton · Pull Request #4398 · graphql-java/graphql-java

bbakerman

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.

`Async` is intended to be a simple utility class - we thus just pass in
the `CompletableFuture`s that are necessary to have this function.

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) <noreply@anthropic.com>

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) <noreply@anthropic.com>

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) <noreply@anthropic.com>