-
Notifications
You must be signed in to change notification settings - Fork 27
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
Improve handling of API response with error message #89
Labels
enhancement
New feature or request
Comments
My workaround was to catch the exception earlier in the request/response lifecycle and add additional information: class TimeoutSession(requests.Session):
# Connect timeout and read timeout. Request docs say to make it slightly
# more than multiple of 3. Use '_' in hope that if upstream adds a timeout to
# the session in the future, it won't conflict.
_timeout = (6.25, 30.25)
def __init__(self, timeout=None):
self._timeout = timeout or self._timeout
super().__init__()
def request(self, method, url, **kwargs):
log.debug(kwargs)
kwargs.setdefault('timeout', self._timeout)
try:
response = super().request(method, url, **kwargs)
log.debug(response.content)
response.raise_for_status()
return response
except requests.HTTPError as e:
if text := e.response.text:
raise requests.HTTPError(text, e.response) from e
raise The timeout stuff (#88) isn't related to this issue it just ended up in the same custom |
goncalossilva
added a commit
that referenced
this issue
Mar 31, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Enhancement description
The functions in
http_requests
should be improved so that error details returned in the API response are preserved.The problem it solves
Currently, the functions in
http_requests
useresponse.raise_for_status()
which throws a generic exception based on the HTTP response code. An error response from the API server can include additional details about why the request failed. For example, "At least one of supported fields should be set and non-empty" was stored inresponse.content
for a recent API call I made that failed. But the exception was a generic 400 response exception and I had to make changes to thehttp_requests
function to figure out what the real error was.Alternatives
Catching the generic exceptions and logging the content. But since there is no one single entry point or method that can be overridden, it has to be done in multiple places, which is verbose. The best place to do it would be in
http_requests
.The text was updated successfully, but these errors were encountered: