From 709a2476ea2c7b2376a4615faec4d2126e65fda5 Mon Sep 17 00:00:00 2001 From: Jaden Fix Date: Thu, 8 Oct 2026 20:03:13 -0700 Subject: [PATCH 1/6] Keep a null policy base_version_id as None PolicyVersion.base_version_id is typed None | UUID, but the policy version wrapper replaced a null value with the all-zero UUID before parsing. Callers could not tell "no base version" apart from a real one. The zero-UUID substitution dates from when the generated field was non-nullable; now a null stays None (a missing key is still tolerated). Tested: new test fails before (got UUID('00000000-...')) and passes after; uv run pytest, ruff check, ruff format --check all pass. --- src/roe/api/policies.py | 6 +----- tests/unit/test_policies_wrapper_transport.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) 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/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 From 1ffd07ad012859c2c207b9c997b6a6e6dd045996 Mon Sep 17 00:00:00 2001 From: Jaden Fix Date: Thu, 8 Oct 2026 20:03:30 -0700 Subject: [PATCH 2/6] Always give API exceptions a string message translate_response took "detail", "error" or "message" from a JSON error body as-is, so a body like {"detail": ["a", "b"]} or {"error": {...}} produced an exception whose .message was a list or dict, despite the str annotation. A list is now joined with "; " (as list bodies already are) and any other non-string value is str()'d. Tested: new parametrized test fails before (message was ['a', 'b'] / {'code': 'bad'}) and passes after; uv run pytest, ruff check, ruff format --check all pass. --- src/roe/exceptions.py | 4 ++++ tests/unit/test_translate_response.py | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/roe/exceptions.py b/src/roe/exceptions.py index ef91446..1602b89 100644 --- a/src/roe/exceptions.py +++ b/src/roe/exceptions.py @@ -115,6 +115,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/tests/unit/test_translate_response.py b/tests/unit/test_translate_response.py index 9f99e31..b1f401b 100644 --- a/tests/unit/test_translate_response.py +++ b/tests/unit/test_translate_response.py @@ -66,6 +66,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")) From 3530575678044b0387e794e88140dd9ea17456ab Mon Sep 17 00:00:00 2001 From: Jaden Fix Date: Thu, 8 Oct 2026 20:04:02 -0700 Subject: [PATCH 3/6] Use ConfigDict for FileUpload model config FileUpload declared its Pydantic settings with the class-based `class Config`, which Pydantic 2 deprecates and which emits a PydanticDeprecatedSince20 warning every time roe.models.file is imported. Switch to model_config = ConfigDict(...) with the same setting (arbitrary_types_allowed=True). Tested: new test importing roe.models.file under -W error::DeprecationWarning fails before and passes after; the pytest warning summary is now empty; uv run pytest, ruff check, ruff format --check all pass. --- src/roe/models/file.py | 5 ++--- tests/unit/test_file_upload.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_file_upload.py diff --git a/src/roe/models/file.py b/src/roe/models/file.py index ea1bcd6..9030a02 100644 --- a/src/roe/models/file.py +++ b/src/roe/models/file.py @@ -4,7 +4,7 @@ import os from typing import BinaryIO -from pydantic import BaseModel, Field, model_validator +from pydantic import BaseModel, ConfigDict, Field, model_validator class FileUpload(BaseModel): @@ -17,8 +17,7 @@ class FileUpload(BaseModel): 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/tests/unit/test_file_upload.py b/tests/unit/test_file_upload.py new file mode 100644 index 0000000..184f811 --- /dev/null +++ b/tests/unit/test_file_upload.py @@ -0,0 +1,17 @@ +import subprocess +import sys + + +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 From f1983b618c5f450d1277e5e94089d76d6084d8a6 Mon Sep 17 00:00:00 2001 From: Jaden Fix Date: Thu, 8 Oct 2026 20:08:09 -0700 Subject: [PATCH 4/6] Accept real file objects in FileUpload(file_obj=...) file_obj is typed typing.BinaryIO, and with arbitrary_types_allowed pydantic validates it with isinstance(). Real file objects (open(..., "rb"), io.BytesIO) are not instances of typing.BinaryIO, so every FileUpload(file_obj=...) raised ValidationError and the file-object path was unusable. Keep the annotation but skip validation for this field. Tested: new test in tests/unit/test_file_upload.py fails before (ValidationError: Input should be an instance of BinaryIO), passes after; full suite, ruff check and ruff format --check pass. --- src/roe/models/file.py | 7 +++++-- tests/unit/test_file_upload.py | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/roe/models/file.py b/src/roe/models/file.py index 9030a02..b365cb0 100644 --- a/src/roe/models/file.py +++ b/src/roe/models/file.py @@ -4,14 +4,17 @@ import os from typing import BinaryIO -from pydantic import BaseModel, ConfigDict, 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)" diff --git a/tests/unit/test_file_upload.py b/tests/unit/test_file_upload.py index 184f811..6994464 100644 --- a/tests/unit/test_file_upload.py +++ b/tests/unit/test_file_upload.py @@ -1,6 +1,9 @@ +import io import subprocess import sys +from roe.models.file import FileUpload + def test_file_upload_import_emits_no_deprecation_warning(): result = subprocess.run( @@ -15,3 +18,10 @@ def test_file_upload_import_emits_no_deprecation_warning(): 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") From c11cd97cb9ef078876adc6230ff08b5bfee659e1 Mon Sep 17 00:00:00 2001 From: Jaden Fix Date: Thu, 8 Oct 2026 22:09:24 -0700 Subject: [PATCH 5/6] Raise RateLimitError for 429 responses 429 fell through to the base RoeAPIException, so callers couldn't catch rate limiting by type. RateLimitError subclasses RoeAPIException, so existing handlers still match. Part of #88. --- src/roe/__init__.py | 2 ++ src/roe/exceptions.py | 7 +++++++ tests/unit/test_translate_response.py | 3 ++- 3 files changed, 11 insertions(+), 1 deletion(-) 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/exceptions.py b/src/roe/exceptions.py index 1602b89..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: diff --git a/tests/unit/test_translate_response.py b/tests/unit/test_translate_response.py index b1f401b..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), ], From 006408fece31a53f52cc6b0c5a6ef7613fd1bae0 Mon Sep 17 00:00:00 2001 From: Jaden Fix Date: Thu, 8 Oct 2026 22:09:54 -0700 Subject: [PATCH 6/6] Honour Retry-After when retrying 429 and 503 responses The transport retried rate-limited requests after 1-4s regardless of the server's Retry-After. Wait for an integer Retry-After (capped at 60s) when it exceeds the backoff; HTTP-date values keep the backoff. Part of #88. --- src/roe/utils/transport.py | 12 ++++++++++-- tests/unit/test_transport.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_transport.py 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_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]