Skip to content

Check for errors during client.fetch_schema() #328

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

Merged

Conversation

luketaverne
Copy link
Contributor

@luketaverne luketaverne commented May 16, 2022

Hi everyone!

We (@wang2bo2 and I) came across a bug when our JWT tokens expired.

Current behavior / steps to reproduce

  • Use an expired JWT token
  • The server returns an error for the expired token, but the error is not checked client side (see PR changes).
  • The result is the error being added added to the client as the data/schema.
  • This causes another error to be raised, which isn't immediately obvious that it's due to an expired JWT. See stacktrace below.
... [project trace omitted] ...
File "/root/.cache/pypoetry/virtualenvs/project-xS3fZVNL-py3.8/lib/python3.8/site-packages/gql/client.py", line 617, in __enter__
         self.session.fetch_schema()
File "/root/.cache/pypoetry/virtualenvs/project-xS3fZVNL-py3.8/lib/python3.8/site-packages/gql/client.py", line 806, in fetch_schema
         self.client.schema = build_client_schema(self.client.introspection)
File "/root/.cache/pypoetry/virtualenvs/project-xS3fZVNL-py3.8/lib/python3.8/site-packages/gql/utilities/build_client_schema.py", line 79, in build_client_schema
         raise TypeError(
TypeError: Invalid or incomplete introspection result. Ensure that you are passing the 'data' attribute of an introspection response and no 'errors' were returned alongside: None.

Expected behavior

  • The error from the server should be raised inside the client sdk.

Fix

The error handling used elsewhere in the library was copied over to the sync and async fetch_schema functions.

Thank you all for maintaining this useful SDK 🙂

@codecov-commenter
Copy link

codecov-commenter commented May 16, 2022

Codecov Report

Merging #328 (64ac830) into master (321c606) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #328   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        26           
  Lines         2156      2159    +3     
=========================================
+ Hits          2156      2159    +3     
Impacted Files Coverage Δ
gql/client.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 321c606...64ac830. Read the comment docs.

@leszekhanusz
Copy link
Collaborator

Thanks for your PR!
I modified the message so that we could see directly that the error comes from fetching the schema and added some tests.

@leszekhanusz leszekhanusz merged commit 9f2139b into graphql-python:master May 19, 2022
@leszekhanusz
Copy link
Collaborator

Fixed in v3.3.0

@luketaverne luketaverne deleted the fetch-schema-error-handling branch May 21, 2022 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants