graphql-python/gql

RequestsHTTPTransport keeps retrying the query even though it is invalid

Jonas1312 opened this issue · 4 comments

Describe the bug

Hi,

The program (given below) will freeze at the last line, where I forget to pass the skip parameter in the variables.

It appears that the server answers a message with a 500 error:

Variable "$skip" of required type "Int!" was not provided.'

And since I specified retries=5 and the default retry code _default_retry_codes = (429, 500, 502, 503, 504), requests keeps resending the same query again and again, even though the server gave an answer.

When I disable the retries or exclude 500 from the retry_status_forcelist, gql raises the TransportQueryError immediately.

To Reproduce

Code below:

Details
from typing import Any, Dict, Optional, Union

from gql import Client, gql
from gql.transport.requests import RequestsHTTPTransport
from graphql import DocumentNode

api_key = ""
api_endpoint = "https://staging.cloud.kili-technology.com/api/label/v2/graphql"

gql_transport = RequestsHTTPTransport(
    url=api_endpoint,
    headers={
        "Authorization": f"X-API-Key: {api_key}",
        "Accept": "application/json",
        "Content-Type": "application/json",
    },
    use_json=True,
    timeout=30,
    verify=True,
    retries=5,
    method="POST",
    retry_backoff_factor=0.5,
)


client = Client(
    transport=gql_transport,
    fetch_schema_from_transport=True,
    introspection_args={
        "descriptions": True,  # descriptions for the schema, types, fields, and arguments
        "specified_by_url": False,  # https://spec.graphql.org/draft/#sec--specifiedBy
        "directive_is_repeatable": True,  # include repeatability of directives
        "schema_description": True,  # include schema description
        "input_value_deprecation": True,  # request deprecated input fields
    },
)


def execute(
    query: Union[str, DocumentNode], variables: Optional[Dict] = None, **kwargs
) -> Dict[str, Any]:
    document = query if isinstance(query, DocumentNode) else gql(query)
    result = client.execute(
        document=document,
        variable_values=variables,
        **kwargs,
    )
    return result


query = """
query projects($where: ProjectWhere!, $first: PageSize!, $skip: Int!) {
    data: projects(where: $where, first: $first, skip: $skip) {
        id title
    }
}
"""
project_id = "clm6gd8b701yu01324m3cesih"

result = execute(query=query, variables={"where": {"id": project_id}, "first": 1, "skip": 0})  # WORKS
print(result)

result = execute(query=query, variables={"where": {"id": project_id}, "first": 1})  # STUCK HERE

Expected behavior

I'm actually not sure if it's really a bug...

I expected gql to check both the query along with the variables, but it looks like gql only checks the query with respect to the schema, but doesn't check the variables with respect to the query?

I'm not a graphql expert, so I wonder if a server is supposed to answer a 500 error for such a query? If not, the bug is server-side, but if yes, maybe the _default_retry_codes should not include the 500 code?

System info (please complete the following information):

  • OS: macos
  • Python version: 3.8
  • gql version: v3.5.0b5
  • graphql-core version: 3.3.0a3

Thanks!

The 500 http status code means Internal Server Error and usually means that there is a bug or a default in the server itself. It should not be used to report errors in the provided query.

Normally, GraphQL servers are supposed to return errors with a 200 OK status code, and indicate the error in the returned json following the GraphQL spec.

It is not really clear which status codes are supposed to allow a retry, which is why retry_status_forcelist is a parameter of the transport to allow the user to choose.

Hi,

ok I see, so it might be a misconfiguration of the server.

But can't this type of error (forget to provide a mandatory parameter) get caught by gql, before the request is sent to the server?

But can't this type of error (forget to provide a mandatory parameter) get caught by gql, before the request is sent to the server?

Yes, we could.

I made the issue #434 to mark this as a requested feature.