diff --git a/src/roe/__init__.py b/src/roe/__init__.py index ece5b2c..4bd9244 100644 --- a/src/roe/__init__.py +++ b/src/roe/__init__.py @@ -30,6 +30,7 @@ ForbiddenError, InsufficientCreditsError, NotFoundError, + RateLimitError, RoeAPIException, ServerError, ) @@ -58,5 +59,6 @@ "ForbiddenError", "InsufficientCreditsError", "NotFoundError", + "RateLimitError", "ServerError", ] diff --git a/src/roe/api/policies.py b/src/roe/api/policies.py index f02c177..2d18a7d 100644 --- a/src/roe/api/policies.py +++ b/src/roe/api/policies.py @@ -43,13 +43,9 @@ from roe.utils.generated_request import request_json, request_raw -_ZERO_UUID = "00000000-0000-0000-0000-000000000000" - - def _normalize_policy_version_wire(data: dict[str, Any]) -> dict[str, Any]: normalized = dict(data) - if normalized.get("base_version_id") is None: - normalized["base_version_id"] = _ZERO_UUID + normalized.setdefault("base_version_id", None) return normalized diff --git a/src/roe/exceptions.py b/src/roe/exceptions.py index ef91446..07b6356 100644 --- a/src/roe/exceptions.py +++ b/src/roe/exceptions.py @@ -62,6 +62,12 @@ class NotFoundError(RoeAPIException): pass +class RateLimitError(RoeAPIException): + """429 Too Many Requests - Rate limit exceeded.""" + + pass + + class ServerError(RoeAPIException): """500+ Server Error - Internal server errors.""" @@ -76,6 +82,7 @@ def get_exception_for_status_code(status_code: int) -> type[RoeAPIException]: 402: InsufficientCreditsError, 403: ForbiddenError, 404: NotFoundError, + 429: RateLimitError, } if status_code in exception_map: @@ -115,6 +122,10 @@ def translate_response(response: Any) -> None: if isinstance(body, dict): message = body.get("detail") or body.get("error") or body.get("message") + if isinstance(message, list): + message = "; ".join(str(e) for e in message) + elif message and not isinstance(message, str): + message = str(message) if not message: for value in body.values(): if isinstance(value, list) and value: diff --git a/src/roe/models/file.py b/src/roe/models/file.py index ea1bcd6..b365cb0 100644 --- a/src/roe/models/file.py +++ b/src/roe/models/file.py @@ -4,21 +4,23 @@ import os from typing import BinaryIO -from pydantic import BaseModel, Field, model_validator +from pydantic import BaseModel, ConfigDict, Field, SkipValidation, model_validator class FileUpload(BaseModel): """Helper class for explicit file uploads with metadata.""" path: str | None = Field(default=None, description="File path to upload") - file_obj: BinaryIO | None = Field(default=None, description="File object to upload") + # SkipValidation: real file objects aren't isinstance(BinaryIO), so validation rejects them. + file_obj: SkipValidation[BinaryIO] | None = Field( + default=None, description="File object to upload" + ) filename: str | None = Field(default=None, description="Override filename") mime_type: str | None = Field( default=None, description="MIME type (auto-detected if not provided)" ) - class Config: - arbitrary_types_allowed = True + model_config = ConfigDict(arbitrary_types_allowed=True) @model_validator(mode="after") def validate_file_source(self): diff --git a/src/roe/utils/transport.py b/src/roe/utils/transport.py index d18fce6..1c429e6 100644 --- a/src/roe/utils/transport.py +++ b/src/roe/utils/transport.py @@ -1,6 +1,7 @@ """Custom httpx transport with retry policy for the Roe SDK. -Retries failed requests with exponential backoff (capped at ~10 seconds): +Retries failed requests with exponential backoff (capped at ~10 seconds, or +longer when a 429/503 sends an integer ``Retry-After``, up to 60 seconds): - Transport errors: ``httpx.TransportError`` (disconnects, timeouts, etc.). - HTTP statuses: ``5xx``, ``408``, ``429``. @@ -32,6 +33,13 @@ def _should_retry_status(status_code: int) -> bool: return status_code >= 500 or status_code in (408, 429) +def _retry_after_seconds(response: httpx.Response) -> int: + value = response.headers.get("retry-after", "") + if response.status_code in (429, 503) and value.isdecimal(): + return min(int(value), 60) + return 0 + + class RoeRetryTransport(httpx.HTTPTransport): """httpx transport with configurable retries aligned with TS and Go SDKs.""" @@ -68,7 +76,7 @@ def handle_request(self, request: httpx.Request) -> httpx.Response: ): return response - wait_time = min(2**attempt, 10) + wait_time = max(min(2**attempt, 10), _retry_after_seconds(response)) logger.warning( "Roe API returned %d for %s %s, retrying in %ds (attempt %d/%d)", response.status_code, diff --git a/tests/unit/test_file_upload.py b/tests/unit/test_file_upload.py new file mode 100644 index 0000000..6994464 --- /dev/null +++ b/tests/unit/test_file_upload.py @@ -0,0 +1,27 @@ +import io +import subprocess +import sys + +from roe.models.file import FileUpload + + +def test_file_upload_import_emits_no_deprecation_warning(): + result = subprocess.run( + [ + sys.executable, + "-W", + "error::DeprecationWarning", + "-c", + "import roe.models.file", + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr + + +def test_file_upload_accepts_file_object(): + buf = io.BytesIO(b"data") + upload = FileUpload(file_obj=buf, filename="a.txt") + + assert upload.to_multipart_tuple() == ("a.txt", buf, "text/plain") diff --git a/tests/unit/test_policies_wrapper_transport.py b/tests/unit/test_policies_wrapper_transport.py index 4aa46f2..a1fed37 100644 --- a/tests/unit/test_policies_wrapper_transport.py +++ b/tests/unit/test_policies_wrapper_transport.py @@ -44,3 +44,21 @@ def test_policy_replace_uses_put_with_org_query_and_model_body(): assert kwargs["params"] == {"organization_id": ORG_ID} assert kwargs["json"] == {"name": "Policy", "description": "desc"} assert result.id == UUID(POLICY_ID) + + +def test_policy_version_without_base_version_keeps_none(): + version = { + "id": "00000000-0000-0000-0000-000000000555", + "version_name": "v1", + "content": {}, + "created_at": "2025-01-01T00:00:00Z", + "updated_at": "2025-01-01T00:00:00Z", + "policy": _update_policy_json(), + "created_by": {"id": 1, "email": "a@b.c", "display_name": "A"}, + "base_version_id": None, + } + api, _ = _api(httpx.Response(200, json=version)) + + result = api.versions.retrieve(POLICY_ID, version["id"]) + + assert result.base_version_id is None diff --git a/tests/unit/test_translate_response.py b/tests/unit/test_translate_response.py index 9f99e31..f7719e2 100644 --- a/tests/unit/test_translate_response.py +++ b/tests/unit/test_translate_response.py @@ -17,6 +17,7 @@ ForbiddenError, InsufficientCreditsError, NotFoundError, + RateLimitError, RoeAPIException, ServerError, translate_response, @@ -40,7 +41,7 @@ def test_2xx_is_noop(): (402, InsufficientCreditsError), (403, ForbiddenError), (404, NotFoundError), - (429, RoeAPIException), + (429, RateLimitError), (500, ServerError), (502, ServerError), ], @@ -66,6 +67,19 @@ def test_list_body_joined_as_message(): assert exc_info.value.response is None +@pytest.mark.parametrize( + "content,expected", + [ + (b'{"detail": ["a", "b"]}', "a; b"), + (b'{"error": {"code": "bad"}}', "{'code': 'bad'}"), + ], +) +def test_non_string_detail_becomes_string_message(content, expected): + with pytest.raises(BadRequestError) as exc_info: + translate_response(_resp(400, content)) + assert exc_info.value.message == expected + + def test_non_json_body_falls_back_to_status_snippet(): with pytest.raises(NotFoundError) as exc_info: translate_response(_resp(404, b"not found")) diff --git a/tests/unit/test_transport.py b/tests/unit/test_transport.py new file mode 100644 index 0000000..f74b6ff --- /dev/null +++ b/tests/unit/test_transport.py @@ -0,0 +1,30 @@ +import httpx +import pytest + +from roe.utils import transport +from roe.utils.transport import RoeRetryTransport + + +@pytest.mark.parametrize( + ("status", "retry_after", "expected_sleep"), + [(429, "5", 5), (503, "120", 60), (429, "Wed, 21 Oct 2025 07:28:00 GMT", 1)], +) +def test_retry_waits_for_retry_after_seconds( + monkeypatch, status, retry_after, expected_sleep +): + responses = [ + httpx.Response(status, headers={"Retry-After": retry_after}), + httpx.Response(200), + ] + sleeps = [] + monkeypatch.setattr( + httpx.HTTPTransport, "handle_request", lambda self, req: responses.pop(0) + ) + monkeypatch.setattr(transport.time, "sleep", sleeps.append) + + response = RoeRetryTransport().handle_request( + httpx.Request("GET", "https://example.com") + ) + + assert response.status_code == 200 + assert sleeps == [expected_sleep]