Skip to content

Commit

Permalink
feat: add trace_id to API exceptions (#404)
Browse files Browse the repository at this point in the history
- Do not use helper functions to raise exceptions,
- Add the request trace ID to the API exceptions,
- Refactor the code.
  • Loading branch information
jooola authored Jul 2, 2024
1 parent 93eb56b commit 8375261
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 34 deletions.
65 changes: 34 additions & 31 deletions hcloud/_client.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import annotations

import time
from typing import NoReturn, Protocol
from typing import Protocol

import requests

Expand Down Expand Up @@ -190,20 +190,6 @@ def _get_headers(self) -> dict:
}
return headers

def _raise_exception_from_response(self, response: requests.Response) -> NoReturn:
raise APIException(
code=response.status_code,
message=response.reason,
details={"content": response.content},
)

def _raise_exception_from_content(self, content: dict) -> NoReturn:
raise APIException(
code=content["error"]["code"],
message=content["error"]["message"],
details=content["error"]["details"],
)

def request( # type: ignore[no-untyped-def]
self,
method: str,
Expand All @@ -229,23 +215,40 @@ def request( # type: ignore[no-untyped-def]
**kwargs,
)

content = {}
trace_id = response.headers.get("X-Correlation-Id")
payload = {}
try:
if len(response.content) > 0:
content = response.json()
except (TypeError, ValueError):
self._raise_exception_from_response(response)
payload = response.json()
except (TypeError, ValueError) as exc:
raise APIException(
code=response.status_code,
message=response.reason,
details={"content": response.content},
trace_id=trace_id,
) from exc

if not response.ok:
if content:
assert isinstance(content, dict)
if content["error"]["code"] == "rate_limit_exceeded" and _tries < 5:
time.sleep(_tries * self._retry_wait_time)
_tries = _tries + 1
return self.request(method, url, _tries=_tries, **kwargs)

self._raise_exception_from_content(content)
else:
self._raise_exception_from_response(response)

return content
if not payload or "error" not in payload:
raise APIException(
code=response.status_code,
message=response.reason,
details={"content": response.content},
trace_id=trace_id,
)

error: dict = payload["error"]

if error["code"] == "rate_limit_exceeded" and _tries < 5:
time.sleep(_tries * self._retry_wait_time)
_tries = _tries + 1
return self.request(method, url, _tries=_tries, **kwargs)

raise APIException(
code=error["code"],
message=error["message"],
details=error["details"],
trace_id=trace_id,
)

return payload
18 changes: 16 additions & 2 deletions hcloud/_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,22 @@ class HCloudException(Exception):
class APIException(HCloudException):
"""There was an error while performing an API Request"""

def __init__(self, code: int | str, message: str | None, details: Any):
super().__init__(code if message is None and isinstance(code, str) else message)
def __init__(
self,
code: int | str,
message: str,
details: Any,
*,
trace_id: str | None = None,
):
extras = [str(code)]
if trace_id is not None:
extras.append(trace_id)

error = f"{message} ({', '.join(extras)})"

super().__init__(error)
self.code = code
self.message = message
self.details = details
self.trace_id = trace_id
27 changes: 26 additions & 1 deletion tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,31 @@ def test_request_fails(self, client, fail_response):
assert error.message == "invalid input in field 'broken_field': is too long"
assert error.details["fields"][0]["name"] == "broken_field"

def test_request_fails_trace_id(self, client, response):
response.headers["X-Correlation-Id"] = "67ed842dc8bc8673"
response.status_code = 409
response._content = json.dumps(
{
"error": {
"code": "conflict",
"message": "some conflict",
"details": None,
}
}
).encode("utf-8")

client._requests_session.request.return_value = response
with pytest.raises(APIException) as exception_info:
client.request(
"POST", "http://url.com", params={"argument": "value"}, timeout=2
)
error = exception_info.value
assert error.code == "conflict"
assert error.message == "some conflict"
assert error.details is None
assert error.trace_id == "67ed842dc8bc8673"
assert str(error) == "some conflict (conflict, 67ed842dc8bc8673)"

def test_request_500(self, client, fail_response):
fail_response.status_code = 500
fail_response.reason = "Internal Server Error"
Expand Down Expand Up @@ -153,7 +178,7 @@ def test_request_500_empty_content(self, client, fail_response):
assert error.code == 500
assert error.message == "Internal Server Error"
assert error.details["content"] == ""
assert str(error) == "Internal Server Error"
assert str(error) == "Internal Server Error (500)"

def test_request_limit(self, client, rate_limit_response):
client._retry_wait_time = 0
Expand Down

0 comments on commit 8375261

Please sign in to comment.