gql-dart / gql

Libraries supporting GraphQL in Dart
MIT License
268 stars 124 forks source link

bug [codegen]: Nesting InlineFragmentNode on an extended-Type in a FragmentDefinition based on an Interface Type definition causes unbound recursion/Stack_Overflow #372

Closed JELaVallee closed 1 year ago

JELaVallee commented 1 year ago

We ran into this issue in our Flutter project where we've been using Fragments to very effectively share GraphQL sub-graph models between Widgets.

We've recently implemented a feature with a Query Field that returns an Interface instead of the concrete-Type of the parent Node. Suddenly, we were getting Stack Overflow errors thrown when running codegen...

Our use case is similar to this scenario:

query HeroWithFragments($episode: Episode) {
  hero(episode: $episode) {
    ...characterFieldsFragment
  }
}

fragment characterFieldsFragment on Character {
  id
  name
  ... on Human {
    ...humanFieldsFragment
  }
}

fragment humanFieldsFragment on Human {
  homePlanet
}

When I run codegen on this (in the codegen/end_to_end_test sub-proj w/ pub overrides enabled or with just our normal build_runner setup), the build run fails with:

ERR_OUT: `dart pub run build_runner build --delete-conflicting-outputs -v` ``` [FINE] built_value_generator:built_value on lib/fragments/__generated__/hero_with_fragments.var.gql.dart:Running BuiltValueGenerator lib/…/__generated__/hero_with_fragments.var.gql.dart:1 [FINE] built_value_generator:built_value on lib/variables/__generated__/human_with_args.var.gql.dart:Running BuiltValueGenerator lib/…/__generated__/human_with_args.var.gql.dart:1 [FINE] built_value_generator:built_value on lib/variables/__generated__/create_review.var.gql.dart:Running BuiltValueGenerator lib/…/__generated__/create_review.var.gql.dart:1 [FINE] built_value_generator:built_value on lib/aliases/__generated__/aliased_hero.var.gql.dart:Running BuiltValueGenerator lib/…/__generated__/aliased_hero.var.gql.dart:1 [FINE] built_value_generator:built_value on lib/interfaces/__generated__/hero_for_episode.var.gql.dart:Running BuiltValueGenerator lib/…/__generated__/hero_for_episode.var.gql.dart:1 [FINE] built_value_generator:built_value on lib/graphql/__generated__/serializers.gql.dart:Running BuiltValueGenerator lib/…/__generated__/serializers.gql.dart:1 [INFO] Build:Running build completed, took 3.5s [INFO] Build:Caching finalized dependency graph... [INFO] Build:Caching finalized dependency graph completed, took 27ms [SEVERE] Build: Failed after 3.5s Exited (1) ```

We've isolated an unbound-recursion in codegen/gql_code_builder/lib/src/operation/data.dart#buildbuildFragmentDataClasses that appears to attempt to continue parsing the InlineFragmentNode, indefinitely until a Stack_Overflow occurs.

Neat capture of the stack just before that overflow happens:

image

Working on a fix for this... It's quite WIP-ish at this time. Should have a PR with the fix and test scenario up in the next day or so!

Note: There appears to be another issue related to Interfaces/Unions in FragmentSpreads causing failed TypedNode expansion (still working through that one). Wanted to get this recursion-termination issue out of the way before diving into that one. 😉

knaeckeKami commented 1 year ago

Thanks! Looking forward to your PR :D

JELaVallee commented 1 year ago

This was fixed with #373

Yay for Interfaces!