GeezerVison

Test Report

Test Results

Code Coverage (Java 25)

Changed Class Coverage (4 classes)

Full HTML report: build artifact jacoco-html-report

Updated: 2026-11-06 14:18:20 UTC

Reviewed 06a04107a6015dc96c4cf0c282031faed9631129, including its interaction with the already-merged #4522. The null check is needed, but I would request changes to the error handling.

1. [P1] Nested directive failures abort the whole operation.

The new AbortExecutionException discards successful sibling results. With a nullable parent field and variables {"flag": null}:

query($flag: Boolean = true) {
  ok
  parent { child @skip(if: $flag) }
}

I verified that both ok and parent execute, but the response contains only errors, classified as ExecutionAborted; data is absent. The nested failure should instead null the enclosing nullable parent, preserve the successful ok result, and report the error path. The same issue occurs with @include, asynchronous parent fetchers, mutations, and list selections. See the spec's execution-error handling.

Subscription handling also remains incomplete. For:

subscription($flag: Boolean = true) {
  event { child @skip(if: $flag) }
}

with the same variables, a source publisher containing two events emits no GraphQL results: the subscriber receives onError(AbortExecutionException) and the stream terminates. I reproduced this for both directives.

2. [P2] Preserve the source location and execution path.

The message-only exception has neither locations nor path, although the offending variable-reference AST node is available. Preserve its source location and attach the execution path when handling a nested failure. The spec recommends locations for identifiable document positions and requires paths for errors associated with a result field.

Relationship to #4522: that PR forces field argument coercion before invoking a data fetcher, within the existing field-error handling. It preserves sibling data and null propagation. This PR addresses directive arguments during field collection, so the changes complement each other; #4522 does not cover this path. The remaining work is to establish consistent coercion-error handling at the field-collection boundaries, rather than simply substituting another exception class.

Coverage: the two added tests cover only root fields. Their result.data == null assertion also accepts both absent data and an explicit "data": null. Please add nested nullable/non-null parents, partial results, paths/locations, and subscription-event cases. I ran 62 existing targeted cases successfully and 49 additional probes covering synchronous/asynchronous execution, mutations, lists, defaults, normalization, and subscriptions. The probes confirm the behavior described above; no production changes were made during the review.

agreed on both, reworked in bbeed5b.

getIfValue now throws NonNullableValueCoercedAsNullException carrying the location of the offending variable reference instead of AbortExecutionException. the two field-collection boundaries handle it:

• nested (ExecutionStrategy.completeValue, around completeValueForObject): it's treated as a field error on the field whose sub-selection failed. the error gets that field's path plus the variable's location, and the field completes as null through the normal non-null validator, so a nullable parent is nulled, a non-null one propagates, and siblings are kept. that covers async parents, mutations and list items since they all go through completeValue, and subscription events too, so each event now emits {event: null} with the error and the stream keeps going.
• root (Execution.executeOperation, around the root collectFields): there's no enclosing field to null, so it returns the error with no data entry, same as the $flag: Boolean! variable coercion failure.

ExecutableNormalizedOperationFactory and the QueryTraverser path now throw the same typed exception instead of the NPE.

tests in SkipAndInclude now cover, for both directives: root (asserts data absent from the spec map, location, no path), nested nullable parent with ok preserved for query and mutation, async parent, non-null parent propagating to the nullable grandparent with path [wrapper, inner], list items with paths [items, 0] / [items, 1], a two-event subscription with no onError, and the normalized operation. the 16 new cases fail on the previous production code; execution/normalized/analysis suites pass.