Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions pyicloud/services/invites/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,29 @@
_RATE_LIMITED_STATUS = 429


def _retry_after_of(exc: PyiCloudAPIResponseException) -> float | None:
"""Return the ``Retry-After`` Apple sent with a rate limit, if any.

The session attaches the response to the exception, so the header survives
the trip. Dropping it would leave callers backing off on a guess when Apple
has told them exactly how long to wait.
"""

response = getattr(exc, "response", None)
if response is None:
return None
try:
header = response.headers.get("Retry-After")
except AttributeError:
return None
if not header:
return None
try:
return float(header)
except (TypeError, ValueError):
return None


def _status_of(exc: PyiCloudAPIResponseException) -> int | None:
"""Return the exception's status as an int, whichever form it arrived in.

Expand Down Expand Up @@ -168,7 +191,9 @@ def _raise_invites_error(exc: Exception) -> NoReturn:
if status in _UNAUTHORIZED_STATUSES:
raise InvitesAuthError(str(exc)) from cause
if status == _RATE_LIMITED_STATUS:
raise InvitesRateLimited(str(exc)) from cause
raise InvitesRateLimited(
str(exc), retry_after=_retry_after_of(exc)
) from cause
raise InvitesApiError(str(exc)) from cause
raise exc

Expand Down Expand Up @@ -340,7 +365,13 @@ def _post_public(self, path: str, payload: dict[str, Any]) -> dict[str, Any]:
url = f"{url}?{urlencode(params)}"
LOGGER.debug("CloudKit Invites POST %s", path)

resp = self._session.post(url, json=payload, timeout=self._timeout)
try:
resp = self._session.post(url, json=payload, timeout=self._timeout)
except PyiCloudAPIResponseException as exc:
# Same trap as the scoped wrappers above: PyiCloudSession raises on
# a non-ok JSON response, so every status check below is dead code
# for a 4xx. Map it the way those checks would have.
self._raise_invites_error(exc)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
code = getattr(resp, "status_code", 0)
if not isinstance(code, int):
code = 200
Expand Down
64 changes: 64 additions & 0 deletions tests/test_invites.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,70 @@ def test_an_unknown_shared_zone_falls_back_to_no_owner(self) -> None:
self.assertEqual(zone_id.zoneName, "MISSING-ZONE")
self.assertIsNone(zone_id.ownerRecordName)

def test_the_public_endpoint_translates_a_transport_error(self) -> None:
"""resolve() and accept() go through a different path to the rest.

`_post_public` inspects `resp.status_code` itself, but PyiCloudSession
raises on a non-ok JSON response before it ever returns, so those
checks are dead for a 4xx and the raw exception reached callers. The
five scoped wrappers were fixed for this; this sixth site uses a
different shape and was missed.
"""
session = MagicMock()
session.post.side_effect = PyiCloudAPIResponseException("Bad Request", 400)
self._monkeypatch.setattr(self.service.raw, "_session", session)

with self.assertRaises(InvitesApiError):
self.service.raw.resolve(["008FIXTURE"])
with self.assertRaises(InvitesApiError):
self.service.raw.accept(["008FIXTURE"])

def test_the_public_endpoint_maps_auth_and_rate_limits(self) -> None:
"""The same mapping as everywhere else, not a generic error."""
for code, expected in (
(401, InvitesAuthError),
(429, InvitesRateLimited),
):
session = MagicMock()
session.post.side_effect = PyiCloudAPIResponseException("nope", code)
self._monkeypatch.setattr(self.service.raw, "_session", session)
with self.subTest(code=code), self.assertRaises(expected):
self.service.raw.resolve(["008FIXTURE"])

def test_a_rate_limit_keeps_the_retry_after_apple_sent(self) -> None:
"""Apple says how long to wait; dropping it makes callers guess.

The header survives on the exception's response, so the mapping has to
read it rather than raise a bare InvitesRateLimited.
"""
response = MagicMock()
response.headers = {"Retry-After": "42"}
session = MagicMock()
session.post.side_effect = PyiCloudAPIResponseException(
"slow down", 429, response
)
self._monkeypatch.setattr(self.service.raw, "_session", session)

with self.assertRaises(InvitesRateLimited) as caught:
self.service.raw.resolve(["008FIXTURE"])

self.assertEqual(caught.exception.retry_after, 42.0)

def test_a_rate_limit_without_a_retry_after_is_still_mapped(self) -> None:
"""A missing or unreadable header must not break the mapping."""
for headers in ({}, {"Retry-After": "soon"}):
session = MagicMock()
response = MagicMock()
response.headers = headers
session.post.side_effect = PyiCloudAPIResponseException(
"slow down", 429, response
)
self._monkeypatch.setattr(self.service.raw, "_session", session)
with self.subTest(headers=headers):
with self.assertRaises(InvitesRateLimited) as caught:
self.service.raw.resolve(["008FIXTURE"])
self.assertIsNone(caught.exception.retry_after)

def test_a_status_code_maps_the_same_as_a_string_or_an_int(self) -> None:
"""`PyiCloudAPIResponseException.code` is typed `int | str | None`.

Expand Down