Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(incremental): failed fragments should never be re-added to the graph #4357

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yaacovCR
Copy link
Contributor

After an execution group "fails," i.e. a null has bubbled up too far, it fails the entire associated deferred fragment. But if the query includes a shared execution group between the failing deferred fragment and a non-failing deferred fragment with some unique subfields, those subfields will still be added to the graph for the failed deferred fragment, which is not present.

This throws an error, because the code assumes that in the situation above, i.e. a shared execution group between two fragments and some unique subfields, both of the deferred fragments must already be in the graph. So a runtime-error is triggered because some of the invariants we rely on when adding deferred fragments are violated.

But even if we didn't have an error, this would be an erroneous situation, as we should never add back a node to the graph that has been failed! The fix should be to retain the failure state on the deferred fragment so as not to attempt to readd the node to the graph.

@yaacovCR yaacovCR requested a review from a team as a code owner March 20, 2025 12:22
Copy link

Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

  • @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
  • @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

@yaacovCR yaacovCR changed the title add failing test for a failed fragment with slower shared execution group fix(incremental): failed fragments should never be re-added to the graph Mar 20, 2025
@yaacovCR yaacovCR requested a review from robrichard March 20, 2025 12:35
@yaacovCR yaacovCR added the PR: bug fix 🐞 requires increase of "patch" version number label Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: bug fix 🐞 requires increase of "patch" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant