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

RequestsHTTPTransport keeps retrying the query even though it is invalid #432

Closed
Jonas1312 opened this issue Sep 5, 2023 · 4 comments
Closed
Labels
type: question or discussion Issue discussing or asking a question about gql

Comments

@Jonas1312
Copy link

Jonas1312 commented Sep 5, 2023

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!

@leszekhanusz
Copy link
Collaborator

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.

@leszekhanusz leszekhanusz added the type: question or discussion Issue discussing or asking a question about gql label Sep 5, 2023
@Jonas1312
Copy link
Author

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?

@leszekhanusz
Copy link
Collaborator

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.

@leszekhanusz
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question or discussion Issue discussing or asking a question about gql
Projects
None yet
Development

No branches or pull requests

2 participants