-
Notifications
You must be signed in to change notification settings - Fork 6
Description
Component
Python SDK
Task Description
In a number of places throughout the SDK we're calling .raise_for_status() directly on the httpx response. This is problematic in the sense that it often hides the real errors making it hard to troubleshoot.
As an example, prior to opsmill/infrahub#7119 being merged the below code:
from infrahub_sdk import InfrahubClientSync
client = InfrahubClientSync()
client.query_gql_query(name="made_up", variables={ "invalid_entry": 42})Would raise an error:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/patrick/Code/opsmill/infrahub/python_sdk/infrahub_sdk/client.py", line 2288, in query_gql_query
resp.raise_for_status()
File "/Users/patrick/.virtualenvs/infrahub/lib/python3.12/site-packages/httpx/_models.py", line 763, in raise_for_status
raise HTTPStatusError(message, request=request, response=self)
httpx.HTTPStatusError: Client error '422 Unprocessable Entity' for url 'http://localhost:8000/api/query/made_up?update_group=false&'
For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/422It's practically impossible to figure out what's going on here.
Instead of calling resp.raise_for_status() we could analyze the response and return a proper error that's easier to understand. If we would have caught the error above and looked at it it would have been easier for the user to figure out the problem.
>>> resp.json()
{'detail': [{'type': 'string_type', 'loc': ['body', 'variables', 'invalid_entry'], 'msg': 'Input should be a valid string', 'input': 42}]}It would be good to completely eliminate the call to .raise_for_status() and instead introduce our own version that would better highlight errors like this.
Even though opsmill/infrahub#7119 has been merged that PR doesn't fix the general problem with troubleshooting even though it will be harder to reach the error specifically within the .query_gql_query() method.