Skip to content

fix(incremental): retain deferred outcomes until group release - #4859

Open
duckki wants to merge 4 commits into
graphql:17.x.xfrom
duckki:unannounced-defer-completion
Open

duckki wants to merge 4 commits into
graphql:17.x.xfrom
duckki:unannounced-defer-completion

Conversation

@duckki

@duckki duckki commented Sep 22, 2026 •

Copy link
Copy Markdown

Issue

{
  ... @defer(label: "R") { bad }
  ... @defer(label: "P") {
    slow
    ... @defer(label: "C") { bad }
  }
}

With bad: String! returning null and slow: String returning "ok", bad is shared by R and C. Initially, only R and P are pending. Upstream completes both R and C when the shared task fails, before P releases C, so the response contains a completed ID for C, which never appeared in pending.

The draft spec's Incremental Completion Notice requires the associated pending notice to appear in an earlier response or the same response. C needs to delay its failure notice until P succeeds and announces it. If P instead fails, C is cancelled with P and doesn't need to be announced.

The earlier guard in this PR suppressed the invalid completion but discarded C's failure. Further tests expose the same retention problem for successful values:

{
  ... @defer(label: "R") { bad x }
  ... @defer(label: "P") {
    slow
    ... @defer(label: "C") { x }
  }
}

All three resolvers start. Settle bad = null, then x = "X", then slow = "ok", allowing each settlement to reach the queue. pruneEmptyGroups sees C's pending count reach zero and deletes it, even though C still owns the unpublished value. The response terminates without delivering x. Only if the last two settlements are reversed, x is delivered successfully. Both schedules are tested with early execution enabled and disabled.

This conflicts with the draft's pending notice semantics: when no notice is returned, the corresponding data should already be available in the initial or prior results. Here the successful value is absent everywhere.

Finally, a shared producer can reveal a nested defer group after its parent has failed and been removed. Without remembering failed parents, the late child can remain an owner of shared work and affect later failure reporting. A regression covers this with and without an intervening empty defer group.

Proposed solution

Treat deferred outcomes as retained state until their groups are released or cancelled:

  • Cache accepted failures for unreleased groups; emit their failure only after an announcing event.
  • Prune a group only when it has neither remaining task memberships nor retained errors. A settled task may still hold unpublished data.
  • Drain settled groups when they are released, publishing buffered values and releasing descendant groups and streams without waiting for another settlement.
  • Remember cancelled group identities so children discovered later inherit cancellation, including through empty groups.
  • Ignore late outcomes with no surviving healthy owner, and prevent a retained failed group from starting further work.
  • Carry multiple retained failures in the internal group-failure event and adapt both response publishers.

These changes belong together: retaining values without draining can stall a group, while suppressing early failure notices without retention loses the group's error.

The follow-up commits first characterize the previous candidate with passing tests, then implement the correction and update the expectations. The tests include validated public operations and direct queue regressions for retained child streams, cancelled late outcomes, shared-value deduplication, and multiple accepted failures.

Verification

  • All 3,047 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.

(Follow-up PR: Error collection before a task's non-null failure is handled separately in #4861.)

@vercel

vercel Bot commented Sep 22, 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.

@duckki
duckki marked this pull request as draft September 24, 2026 22:58
@duckki

duckki commented Sep 24, 2026

Copy link
Copy Markdown
Author

This is a real bug. But, the current proposed fix may not be the right fix. I'm validating a new fix.

  • GraphQL.js emits a failure completion for the unannounced child. This is the bug.
  • But, the spec does not allow dropping the child without justification.
  • So, a better solution might be delaying child's pending and completion (w/ failure) until its parent is released.

@duckki duckki changed the title fix(incremental): avoid completing unreleased defer groups fix(incremental): retain deferred outcomes until group release Sep 28, 2026
@duckki

duckki commented Sep 28, 2026

Copy link
Copy Markdown
Author

Updated the fix to retain failures and buffered values until their defer groups are announced, and to cancel children discovered after their parent fails.
Added passing characterization tests, followed by a fix commit correcting their expectations. All 3,047 tests pass with 100% coverage.
The separate issue of losing already-collected errors is addressed in #4861.

@duckki
duckki marked this pull request as ready for review September 28, 2026 14:04

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