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

Can default values form cycles? #196

Open
mat-sop opened this issue Mar 30, 2023 · 2 comments
Open

Can default values form cycles? #196

mat-sop opened this issue Mar 30, 2023 · 2 comments
Labels
upstream Should be solved or discussed upstream

Comments

@mat-sop
Copy link

mat-sop commented Mar 30, 2023

Hi, I’ve been working with input fields' default values, and I encountered this question - can default values form cycles? I looked into the spec but I haven’t found answer. I tried to create a schema with such values:


from graphql import build_schema

schema_str = """
schema { query: Query }
type Query { _skip: String! }

input InputA {
  id: ID!
  fieldB: InputB = {id: "1"}
}

input InputB {
  id: ID!
  fieldA: InputA = {id: "1"}
}
"""
build_schema(schema_str)

I used graphql-core==3.2.3 and python-3.11.1, as a result, I got this exception:

Traceback (most recent call last):
  File ".../lib/python3.11/site-packages/graphql/type/definition.py", line 1456, in fields
    fields = resolve_thunk(self._fields)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/type/definition.py", line 300, in resolve_thunk
    return thunk() if callable(thunk) else thunk
           ^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/utilities/extend_schema.py", line 605, in <lambda>
    fields=lambda: build_input_field_map(all_nodes),
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RecursionError: maximum recursion depth exceeded

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File ".../lib/python3.11/site-packages/graphql/type/definition.py", line 1456, in fields
    fields = resolve_thunk(self._fields)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/type/definition.py", line 300, in resolve_thunk
    return thunk() if callable(thunk) else thunk
           ^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/utilities/extend_schema.py", line 605, in <lambda>
    fields=lambda: build_input_field_map(all_nodes),
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/utilities/extend_schema.py", line 471, in build_input_field_map
    default_value=value_from_ast(field.default_value, type_),
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/utilities/value_from_ast.py", line 107, in value_from_ast
    fields = type_.fields
             ^^^^^^^^^^^^
  File "".../lib/python3.11/functools.py", line 1001, in __get__
    val = self.func(instance)
          ^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/type/definition.py", line 1459, in fields
    raise cls(f"{self.name} fields cannot be resolved. {error}") from error
TypeError: InputB fields cannot be resolved. maximum recursion depth exceeded

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File ".../lib/python3.11/site-packages/graphql/type/definition.py", line 1456, in fields
    fields = resolve_thunk(self._fields)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/type/definition.py", line 300, in resolve_thunk
    return thunk() if callable(thunk) else thunk
           ^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/utilities/extend_schema.py", line 605, in <lambda>
    fields=lambda: build_input_field_map(all_nodes),
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/utilities/extend_schema.py", line 471, in build_input_field_map
    default_value=value_from_ast(field.default_value, type_),
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/utilities/value_from_ast.py", line 107, in value_from_ast
    fields = type_.fields
             ^^^^^^^^^^^^
  File "".../lib/python3.11/functools.py", line 1001, in __get__
    val = self.func(instance)
          ^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/type/definition.py", line 1459, in fields
    raise cls(f"{self.name} fields cannot be resolved. {error}") from error
TypeError: InputA fields cannot be resolved. InputB fields cannot be resolved. maximum recursion depth exceeded

The above exception was the direct cause of the following exception:

...
# 4k lines of the same error, each with extra `InputB fields cannot be resolved.`
...

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "scheam_test.py", line 17, in <module>
    build_schema(schema_str)
  File ".../lib/python3.11/site-packages/graphql/utilities/build_ast_schema.py", line 95, in build_schema
    return build_ast_schema(
           ^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/utilities/build_ast_schema.py", line 84, in build_ast_schema
    return GraphQLSchema(**schema_kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/type/schema.py", line 218, in __init__
    collect_referenced_types(type_)
  File ".../lib/python3.11/site-packages/graphql/type/schema.py", line 438, in collect_referenced_types
    for field in named_type.fields.values():
                 ^^^^^^^^^^^^^^^^^
  File "".../lib/python3.11/functools.py", line 1001, in __get__
    val = self.func(instance)
          ^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.11/site-packages/graphql/type/definition.py", line 1459, in fields
    raise cls(f"{self.name} fields cannot be resolved. {error}") from error
TypeError: InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. InputA fields cannot be resolved. InputB fields cannot be resolved. maximum recursion depth exceeded

I wonder if this is not allowed but not mentioned in the specification or is it a bug in graphql-core?

@Cito
Copy link
Member

Cito commented Mar 30, 2023

Thanks for the feedback @mat-sop.

Let's first simplify the problematic SDL a bit to get to the core of it:

"""Example 1"""
type Query { field(recursiveInput: RecursiveInput): String }

input RecursiveInput {
  self: RecursiveInput = {value: "foo"}
  value: String 
}

The spec actually says something about such recursions here. Though it does not explicitly mention your example, the spirit of the spec is that recursive definitions are in principle allowed, but only if it is possible to construct valid values from them. This can be possible by breaking the chain with a null value if the definition allows that.

The problem I see in this example is the default value of {value: "foo"}. Since this is lacking the self field, the default value for self is looked up, which leads to a recursion. If you leave out the default value or change it to be = null, everything is fine:

"""Example 2"""
type Query { field(recursiveInput: RecursiveInput): String }

input RecursiveInput {
  self: RecursiveInput
  value: String 
}

However, the following also looks fine to me but still breaks:

"""Example 3"""
type Query { field(recursiveInput: RecursiveInput): String }

input RecursiveInput {
  self: RecursiveInput = {value: "foo", self: null}
  value: String 
}

On the other hand, the following actually invalid example does not give an error:

"""Example 4"""
type Query { field(recursiveInput: RecursiveInput): String }

input RecursiveInput {
  self: RecursiveInput!
  value: String 
}

I found that the same problems exists in graphql-js. which also throws a "Maximum call stack size exceeded" error in the cases where Python raises a RecursionError. Since the goal of graphql-core is to replicate graphql-js, I do not consider it a bug in graphql-core, but something that might need to be fixed in grapqhl-js.

I currently see three things that could be improved here:

  • Example 1 should produce a proper error instead of creating a stack overflow
  • Example 3 should not produce an error
  • Example 4 should produce an error showing that this is an invalid definition

But this should be discussed in the issue tracker of GraphQL.js.

I would be glad if you could create an issue there. Otherwise I will do it when I find some more time.

@mat-sop
Copy link
Author

mat-sop commented Mar 31, 2023

@Cito Thanks for your reply! I created an issue and used your examples, but please add more context if you think that something is missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Should be solved or discussed upstream
Projects
None yet
Development

No branches or pull requests

2 participants