From c647cf14deabe997ceeafc89fd015aae60defd37 Mon Sep 17 00:00:00 2001 From: mrjarnould Date: Fri, 4 Sep 2026 15:40:52 +0200 Subject: [PATCH 1/2] fix(invites): translate transport errors from the public-scope endpoint `resolve()` and `accept()` reach Apple through `_post_public`, which inspects `resp.status_code` and raises InvitesAuthError, InvitesRateLimited or InvitesApiError. None of that runs: PyiCloudSession raises PyiCloudAPIResponseException on a non-ok JSON response before the method returns, so every status check below it is dead code for a 4xx and the raw exception reaches callers. This is the same defect #339 fixed for the five scoped wrappers. It was missed there because `_post_public` does not use the same `except` shape -- it inspects the status itself, which reads as though it already handles the case. The session call is now wrapped and mapped through the existing `_raise_invites_error`, so a bad invite id raises InvitesApiError and an expired session raises InvitesAuthError, as callers catching InvitesError already expect. Found by an `icloud invites resolve` command producing a bare traceback for a malformed invite id. Both new tests fail against the current code. Co-Authored-By: Claude Opus 5 --- pyicloud/services/invites/client.py | 8 +++++++- tests/test_invites.py | 30 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/pyicloud/services/invites/client.py b/pyicloud/services/invites/client.py index 50133de0..6227cc60 100644 --- a/pyicloud/services/invites/client.py +++ b/pyicloud/services/invites/client.py @@ -340,7 +340,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) code = getattr(resp, "status_code", 0) if not isinstance(code, int): code = 200 diff --git a/tests/test_invites.py b/tests/test_invites.py index 1da9aea0..d796951c 100644 --- a/tests/test_invites.py +++ b/tests/test_invites.py @@ -453,6 +453,36 @@ 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_status_code_maps_the_same_as_a_string_or_an_int(self) -> None: """`PyiCloudAPIResponseException.code` is typed `int | str | None`. From babdafeba67fa6bb46bc6ac00096a2d608999d08 Mon Sep 17 00:00:00 2001 From: mrjarnould Date: Fri, 4 Sep 2026 16:18:54 +0200 Subject: [PATCH 2/2] fix(invites): keep the Retry-After Apple sends with a rate limit The 429 branch of the new mapping raised a bare InvitesRateLimited, losing the delay Apple supplied. `_post_public`'s own status handling read the header before this change routed around it, so this was a regression the fix introduced rather than a pre-existing gap. The session attaches the response to PyiCloudAPIResponseException, so the header survives the trip and the mapping now reads it. A missing or unreadable value still maps to InvitesRateLimited, just without a delay. Callers backing off on a guess when Apple has told them exactly how long to wait is the kind of thing that only shows up under load. Co-Authored-By: Claude Opus 5 --- pyicloud/services/invites/client.py | 27 ++++++++++++++++++++++- tests/test_invites.py | 34 +++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/pyicloud/services/invites/client.py b/pyicloud/services/invites/client.py index 6227cc60..fa272797 100644 --- a/pyicloud/services/invites/client.py +++ b/pyicloud/services/invites/client.py @@ -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. @@ -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 diff --git a/tests/test_invites.py b/tests/test_invites.py index d796951c..20ffa364 100644 --- a/tests/test_invites.py +++ b/tests/test_invites.py @@ -483,6 +483,40 @@ def test_the_public_endpoint_maps_auth_and_rate_limits(self) -> None: 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`.