-
Notifications
You must be signed in to change notification settings - Fork 184
4XX errors with valid json not being properly handled #194
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
Comments
Current Code uses str(e) to bring out the message, however I think it would be better if response code is also send back to the caller. This way it can be handled more gracefully. User can identify the issue by looking at the code instead of parsing the string |
Sorry, I don't understand your problem. @pytest.mark.asyncio
async def test_aiohttp_error_code_401(event_loop, aiohttp_server):
from aiohttp import web
from gql.transport.aiohttp import AIOHTTPTransport
async def handler(request):
# Will generate http error code 401
raise web.HTTPUnauthorized
app = web.Application()
app.router.add_route("POST", "/", handler)
server = await aiohttp_server(app)
url = server.make_url("/")
sample_transport = AIOHTTPTransport(url=url)
async with Client(transport=sample_transport,) as session:
query = gql(query1_str)
with pytest.raises(TransportServerError) as exc_info:
await session.execute(query)
assert "401, message='Unauthorized'" in str(exc_info.value)
@pytest.mark.aiohttp
@pytest.mark.asyncio
async def test_requests_error_code_401(event_loop, aiohttp_server, run_sync_test):
from aiohttp import web
from gql.transport.requests import RequestsHTTPTransport
async def handler(request):
# Will generate http error code 401
raise web.HTTPUnauthorized
app = web.Application()
app.router.add_route("POST", "/", handler)
server = await aiohttp_server(app)
url = server.make_url("/")
def test_code():
sample_transport = RequestsHTTPTransport(url=url)
with Client(transport=sample_transport,) as session:
query = gql(query1_str)
with pytest.raises(TransportServerError) as exc_info:
session.execute(query)
assert "401 Client Error: Unauthorized" in str(exc_info.value)
await run_sync_test(event_loop, server, test_code) |
Oooh I see, your server is not returning a 401 http error code but instead is returning a json with a "status" key with a value of "401". Sorry but this is not valid GraphQL following the spec, so it is completely normal to return a TransportProtocolError in this case. I agree that it is confusing and we could improve the TransportProtocolError as it was done for the aiohttp transport |
The server is sending the correct http error code. It can be seen in below curl request If I tweak the above test code it starts to throw TransportProtocolError @pytest.mark.aiohttp
@pytest.mark.asyncio
async def test_requests_error_code_401(event_loop, aiohttp_server, run_sync_test):
from aiohttp import web
from gql.transport.requests import RequestsHTTPTransport
async def handler(request):
# Will generate http error code 401
return web.Response(
text='{"timestamp":1614062931012,"status":401,"error":"Unauthorized","message":"No message","path":"/"}',
content_type="application/json",
status=401
)
app = web.Application()
app.router.add_route("POST", "/", handler)
server = await aiohttp_server(app)
url = server.make_url("/")
def test_code():
sample_transport = RequestsHTTPTransport(url=url)
with Client(transport=sample_transport,) as session:
query = gql(query1_str)
with pytest.raises(TransportServerError) as exc_info:
session.execute(query)
assert "401 Client Error: Unauthorized" in str(exc_info.value)
await run_sync_test(event_loop, server, test_code) Note that instead of raising an HTTPUnauthorized exception, I returned a proper response with status code 401 and text describing it. Test fails as seen below The issue is at requests.py:154. Here the assumption is that 4XX and 5XX errors won't return a proper json. Same issue is in aiohttp.py:210. I have not gone through the graphql specs but this issue seems to be more related to http and not graphql and thus should be handled. |
Indeed, I understand now |
Although we are getting 401, it is not properly handled. Looking at the code it seems 4XX errors are handled in except block. However except block never gets executed.

In this situation we should get TransportServerError, but instead we get TransportProtocolError
The text was updated successfully, but these errors were encountered: