Skip to content

Commit e28c2a2

Browse files
committed
fix: address error contract review
1 parent bca8064 commit e28c2a2

4 files changed

Lines changed: 61 additions & 29 deletions

File tree

‎docs/index.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ client = OilPriceAPI(
182182
### Error Handling
183183

184184
```python
185-
from oilpriceapi.exceptions import (
185+
from oilpriceapi import (
186186
DataNotFoundError,
187187
OilPriceAPIError,
188188
RateLimitError,

‎oilpriceapi/client.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,12 @@ def request_with_headers(
401401
elif response.status_code >= 500:
402402
if self._retry_strategy.should_retry(attempt, response.status_code):
403403
wait_time = self._retry_strategy.calculate_wait_time(attempt)
404+
self._retry_strategy.log_retry(
405+
attempt,
406+
f"Server error {response.status_code}",
407+
wait_time,
408+
is_async=False,
409+
)
404410
time.sleep(wait_time)
405411
continue
406412
raise error_from_response(
@@ -416,6 +422,9 @@ def request_with_headers(
416422
)
417423
if self._retry_strategy.should_retry_on_exception(attempt):
418424
wait_time = self._retry_strategy.calculate_wait_time(attempt)
425+
self._retry_strategy.log_retry(
426+
attempt, "Request timeout", wait_time, is_async=False
427+
)
419428
time.sleep(wait_time)
420429
continue
421430
raise last_exception
@@ -426,6 +435,12 @@ def request_with_headers(
426435
)
427436
if self._retry_strategy.should_retry_on_exception(attempt):
428437
wait_time = self._retry_strategy.calculate_wait_time(attempt)
438+
self._retry_strategy.log_retry(
439+
attempt,
440+
f"Request error: {error.__class__.__name__}",
441+
wait_time,
442+
is_async=False,
443+
)
429444
time.sleep(wait_time)
430445
continue
431446
raise last_exception

‎oilpriceapi/exceptions.py‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
from datetime import datetime
44
from typing import Any, Dict, Iterable, List, Mapping, Optional, Tuple
55

6+
import httpx
7+
68
_SENSITIVE_HEADERS = {
79
"authorization",
810
"proxy-authorization",
@@ -360,9 +362,10 @@ def __init__(
360362
self,
361363
message: str = "Request timed out",
362364
timeout: Optional[float] = None,
365+
cause_type: str = "TimeoutException",
363366
**kwargs: Any,
364367
):
365-
super().__init__(message, cause_type="TimeoutException", **kwargs)
368+
super().__init__(message, cause_type=cause_type, **kwargs)
366369
self.timeout = timeout
367370

368371
def __str__(self) -> str:
@@ -509,9 +512,13 @@ def error_from_exception(
509512
) -> OilPriceAPIError:
510513
"""Normalize transport failures while redacting the configured API key."""
511514
secrets = [api_key] if api_key else []
512-
if "Timeout" in error.__class__.__name__:
513-
return TimeoutError(timeout=timeout)
514515
message = _redact_text(str(error), secrets)
516+
if isinstance(error, httpx.TimeoutException):
517+
return TimeoutError(
518+
message=f"Request timed out: {message}" if message else "Request timed out",
519+
timeout=timeout,
520+
cause_type=error.__class__.__name__,
521+
)
515522
return NetworkError(
516523
message=f"Request failed: {message}",
517524
cause_type=error.__class__.__name__,

‎tests/unit/test_error_contract.py‎

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ def test_canonical_nested_error_preserves_recovery_contract():
8080
assert "authorization" not in error.headers
8181
assert "secret-test-key" not in error.raw_text
8282
assert "secret-test-key" not in repr(error.raw_body)
83+
assert error.retryable is False
84+
assert error.is_client_error is True
85+
assert error.is_server_error is False
8386

8487

8588
def test_legacy_flat_error_and_rate_limit_headers_are_normalized():
@@ -148,35 +151,42 @@ def test_malformed_non_json_body_is_preserved_without_losing_type():
148151
assert error.raw_body == "<html>upstream unavailable</html>"
149152
assert error.raw_text == "<html>upstream unavailable</html>"
150153
assert error.headers["content-type"] == "text/html"
154+
assert error.retryable is True
155+
assert error.is_server_error is True
151156

152157

153158
def test_sync_client_normalizes_timeout_and_network_failures():
154159
timeout_request = httpx.Request("GET", "https://api.oilpriceapi.com/v1/prices/latest")
155-
client = OilPriceAPI(api_key="secret-test-key", max_retries=1)
156-
157-
with patch.object(
158-
client._client,
159-
"request",
160-
side_effect=httpx.ReadTimeout("timed out", request=timeout_request),
161-
):
162-
with pytest.raises(TimeoutError) as timeout:
163-
client.request("GET", "/v1/prices/latest")
164-
165-
assert timeout.value.timeout == client.timeout
166-
167-
with patch.object(
168-
client._client,
169-
"request",
170-
side_effect=httpx.ConnectError(
171-
"connection failed for secret-test-key",
172-
request=timeout_request,
173-
),
174-
):
175-
with pytest.raises(NetworkError) as network:
176-
client.request("GET", "/v1/prices/latest")
177-
178-
assert "secret-test-key" not in str(network.value)
179-
assert network.value.cause_type == "ConnectError"
160+
with OilPriceAPI(api_key="secret-test-key", max_retries=1) as client:
161+
with patch.object(
162+
client._client,
163+
"request",
164+
side_effect=httpx.ReadTimeout(
165+
"timed out for secret-test-key",
166+
request=timeout_request,
167+
),
168+
):
169+
with pytest.raises(TimeoutError) as timeout:
170+
client.request("GET", "/v1/prices/latest")
171+
172+
assert timeout.value.timeout == client.timeout
173+
assert timeout.value.cause_type == "ReadTimeout"
174+
assert "secret-test-key" not in str(timeout.value)
175+
176+
with patch.object(
177+
client._client,
178+
"request",
179+
side_effect=httpx.ConnectError(
180+
"connection failed for secret-test-key",
181+
request=timeout_request,
182+
),
183+
):
184+
with pytest.raises(NetworkError) as network:
185+
client.request("GET", "/v1/prices/latest")
186+
187+
assert "secret-test-key" not in str(network.value)
188+
assert network.value.cause_type == "ConnectError"
189+
assert network.value.retryable is True
180190

181191

182192
@pytest.mark.asyncio

0 commit comments

Comments
 (0)