Skip to content

fix(incremental): preserve collected errors on incremental failure - #4861

Open
duckki wants to merge 2 commits into
graphql:17.x.xfrom
duckki:codex/incremental-error-reporting
Open

duckki wants to merge 2 commits into
graphql:17.x.xfrom
duckki:codex/incremental-error-reporting

Conversation

@duckki

@duckki duckki commented Sep 28, 2026

Copy link
Copy Markdown

Issue

{
  ... @defer {
    nullable
    required
  }
}

With nullable: String and required: String!, let nullable throw Error("nullable failed"), then let required return null. Both fields execute, and both raise an execution error. The deferred completion currently contains only the error at ["required"]; the error already collected at ["nullable"] disappears.

The same loss occurs for a streamed non-null object item:

{
  users @stream(initialCount: 0) {
    nullable
    required
  }
}

With users: [User!] and the same field behavior on User, the stream's failed completion reports only ["users", 0, "required"] and drops ["users", 0, "nullable"].

executeExecutionGroup and completeStreamItem abort and rethrow the escaping non-null error. The publisher then constructs an error list from that single thrown value, losing the executor's collectedErrors. This also affects the legacy response format.

Relevant sections of the incremental-delivery draft, pinned to 045e193:

These tests concern an error that has already been raised and collected. They do not require execution of siblings that may be cancelled after a non-null failure. The draft's failure-mapping algorithm uses a singular error; preserving the collected list here is the proposed treatment of that transport boundary.

Proposed solution

  • Add the escaping error to the executor's collected errors before aborting, using the existing root-null suppression behavior.
  • Carry that list in an internal IncrementalExecutionError through the existing task and stream rejection paths.
  • Unwrap the internal carrier in both publishers. Ordinary thrown errors still become a singleton list; arbitrary resolver AggregateError objects are not flattened.
  • Cover synchronous and asynchronous defer failures, synchronous and asynchronous stream-item failures, and promised stream items in both response formats.

The first commit observes the existing loss with passing tests. The second commit implements the fix and corrects those expectations to retain both error paths. Each public query is validated before execution.

Verification

  • All 3,042 tests pass with 100% line, branch, and function coverage.
  • TypeScript, ESLint, Prettier (including examples), spelling, and the Deno package build pass.
  • Full npm test stops at the Deno runtime check because Deno is not installed locally. The separately invoked integration suite is blocked by missing Docker.

This PR is scoped separately from the deferred-group lifecycle changes in #4859. Both target 17.x.x. Their publisher edits overlap: when combined, retained group failures should use errors.flatMap(getIncrementalErrors). The combined implementation is also checked locally.

@vercel

vercel Bot commented Sep 28, 2026

Copy link
Copy Markdown

@duckki is attempting to deploy a commit to the The GraphQL Foundation Team on Vercel.

A member of the Team first needs to authorize it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant