fix: Stop polling an action attempt once the timeout passes

seam/modules/action_attempts.py

@@ -4,12 +4,26 @@
 
 from ..client import AsyncSeamHttpClient, SeamHttpClient
 from ..exceptions import SeamActionAttemptFailedError, SeamActionAttemptTimeoutError
+from ..options import SeamInvalidOptionsError
 from ..resources import ActionAttempt, SuccessActionAttempt, action_attempt_from_dict
 
 TIMEOUT = 5.0
 POLLING_INTERVAL = 0.5
 
 
+def validate_poll_options(timeout: float, polling_interval: float) -> None:
+    # Written as negated comparisons so NaN fails both checks.
+    if not timeout >= 0:
+        raise SeamInvalidOptionsError(
+            f"The timeout option must not be negative, got {timeout}"
+        )
+
+    if not polling_interval > 0:
+        raise SeamInvalidOptionsError(
+            f"The polling_interval option must be greater than zero, got {polling_interval}"
+        )
+
+
 def get_action_attempt(client: SeamHttpClient, action_attempt_id: str) -> ActionAttempt:
     res = client.post(
         "/action_attempts/get", json={"action_attempt_id": action_attempt_id}
@@ -24,18 +38,23 @@ def poll_until_ready(
     action_attempt_id: str,
     timeout: float = TIMEOUT,
     polling_interval: float = POLLING_INTERVAL,
+    action_attempt: Optional[ActionAttempt] = None,
 ) -> SuccessActionAttempt:
-    time_waiting = 0.0
+    validate_poll_options(timeout, polling_interval)
+
+    deadline = time.monotonic() + timeout
 
-    action_attempt = get_action_attempt(client, action_attempt_id)
+    if action_attempt is None:
+        action_attempt = get_action_attempt(client, action_attempt_id)
 
     while action_attempt.status == "pending":
-        time.sleep(polling_interval)
-        time_waiting += polling_interval
+        remaining = deadline - time.monotonic()
 
-        if time_waiting > timeout:
+        if remaining <= 0:
             raise SeamActionAttemptTimeoutError(action_attempt, timeout)
 
+        time.sleep(min(polling_interval, remaining))
+
         action_attempt = get_action_attempt(client, action_attempt_id)
 
     if action_attempt.status == "error":
@@ -52,7 +71,9 @@ def resolve_action_attempt(
 ) -> ActionAttempt:
     if wait_for_action_attempt is True:
         return poll_until_ready(
-            client=client, action_attempt_id=action_attempt.action_attempt_id
+            client=client,
+            action_attempt_id=action_attempt.action_attempt_id,
+            action_attempt=action_attempt,
         )
 
     if isinstance(wait_for_action_attempt, dict):
@@ -63,6 +84,7 @@ def resolve_action_attempt(
             polling_interval=wait_for_action_attempt.get(
                 "polling_interval", POLLING_INTERVAL
             ),
+            action_attempt=action_attempt,
         )
 
     return action_attempt
@@ -84,18 +106,23 @@ async def poll_until_ready_async(
     action_attempt_id: str,
     timeout: float = TIMEOUT,
     polling_interval: float = POLLING_INTERVAL,
+    action_attempt: Optional[ActionAttempt] = None,
 ) -> SuccessActionAttempt:
-    time_waiting = 0.0
+    validate_poll_options(timeout, polling_interval)
 
-    action_attempt = await get_action_attempt_async(client, action_attempt_id)
+    deadline = time.monotonic() + timeout
+
+    if action_attempt is None:
+        action_attempt = await get_action_attempt_async(client, action_attempt_id)
 
     while action_attempt.status == "pending":
-        await asyncio.sleep(polling_interval)
-        time_waiting += polling_interval
+        remaining = deadline - time.monotonic()
 
-        if time_waiting > timeout:
+        if remaining <= 0:
             raise SeamActionAttemptTimeoutError(action_attempt, timeout)
 
+        await asyncio.sleep(min(polling_interval, remaining))
+
         action_attempt = await get_action_attempt_async(client, action_attempt_id)
 
     if action_attempt.status == "error":
@@ -112,7 +139,9 @@ async def resolve_action_attempt_async(
 ) -> ActionAttempt:
     if wait_for_action_attempt is True:
         return await poll_until_ready_async(
-            client=client, action_attempt_id=action_attempt.action_attempt_id
+            client=client,
+            action_attempt_id=action_attempt.action_attempt_id,
+            action_attempt=action_attempt,
         )
 
     if isinstance(wait_for_action_attempt, dict):
@@ -123,6 +152,7 @@ async def resolve_action_attempt_async(
             polling_interval=wait_for_action_attempt.get(
                 "polling_interval", POLLING_INTERVAL
             ),
+            action_attempt=action_attempt,
         )
 
     return action_attempt

test/wait_for_action_attempt_test.py

@@ -1,7 +1,22 @@
-import pytest
+import time
 from threading import Timer
+
+import pytest
+
 from seam.exceptions import SeamActionAttemptTimeoutError, SeamActionAttemptFailedError
-from seam import Seam
+from seam import AsyncSeam, Seam, SeamInvalidOptionsError
+
+PENDING_ACTION_ATTEMPT_ID = "11111111-1111-1111-1111-111111111111"
+
+PENDING_ACTION_ATTEMPT_RESPONSE = {
+    "action_attempt": {
+        "action_attempt_id": PENDING_ACTION_ATTEMPT_ID,
+        "action_type": "UNLOCK_DOOR",
+        "status": "pending",
+        "result": None,
+        "error": None,
+    }
+}
 
 
 def test_wait_for_action_attempt_directly_on_returned_action_attempt(server):
@@ -196,10 +211,126 @@ def test_wait_for_action_attempt_times_out_if_waiting_for_polling_interval(serve
         },
     )
 
+    start = time.monotonic()
+
     with pytest.raises(SeamActionAttemptTimeoutError) as exc_info:
         seam.action_attempts.get(
             action_attempt_id=action_attempt.action_attempt_id,
             wait_for_action_attempt={"timeout": 0.5, "polling_interval": 5},
         )
 
+    # The wait sleeps only the time remaining until the deadline, never a
+    # full polling_interval past it.
+    assert time.monotonic() - start < 2.5
+
     assert exc_info.value.action_attempt == action_attempt
+
+
+def test_wait_for_action_attempt_rejects_a_zero_polling_interval(recording_server):
+    with recording_server([(200, PENDING_ACTION_ATTEMPT_RESPONSE)]) as (
+        endpoint,
+        requests,
+    ):
+        seam = Seam.from_api_key("seam_apikey_token", endpoint=endpoint)
+
+        with pytest.raises(
+            SeamInvalidOptionsError,
+            match="The polling_interval option must be greater than zero, got 0",
+        ):
+            seam.action_attempts.get(
+                action_attempt_id=PENDING_ACTION_ATTEMPT_ID,
+                wait_for_action_attempt={"timeout": 1, "polling_interval": 0},
+            )
+
+        assert len(requests) == 1
+
+
+def test_wait_for_action_attempt_rejects_a_negative_polling_interval(recording_server):
+    with recording_server([(200, PENDING_ACTION_ATTEMPT_RESPONSE)]) as (endpoint, _):
+        seam = Seam.from_api_key("seam_apikey_token", endpoint=endpoint)
+
+        with pytest.raises(
+            SeamInvalidOptionsError,
+            match="The polling_interval option must be greater than zero, got -1",
+        ):
+            seam.action_attempts.get(
+                action_attempt_id=PENDING_ACTION_ATTEMPT_ID,
+                wait_for_action_attempt={"timeout": 1, "polling_interval": -1},
+            )
+
+
+def test_wait_for_action_attempt_rejects_a_negative_timeout(recording_server):
+    with recording_server([(200, PENDING_ACTION_ATTEMPT_RESPONSE)]) as (endpoint, _):
+        seam = Seam.from_api_key("seam_apikey_token", endpoint=endpoint)
+
+        with pytest.raises(
+            SeamInvalidOptionsError,
+            match="The timeout option must not be negative, got -1",
+        ):
+            seam.action_attempts.get(
+                action_attempt_id=PENDING_ACTION_ATTEMPT_ID,
+                wait_for_action_attempt={"timeout": -1},
+            )
+
+
+def test_wait_for_action_attempt_polls_at_least_once_before_timing_out(
+    recording_server,
+):
+    with recording_server([(200, PENDING_ACTION_ATTEMPT_RESPONSE)]) as (
+        endpoint,
+        requests,
+    ):
+        seam = Seam.from_api_key("seam_apikey_token", endpoint=endpoint)
+
+        start = time.monotonic()
+
+        with pytest.raises(SeamActionAttemptTimeoutError):
+            seam.action_attempts.get(
+                action_attempt_id=PENDING_ACTION_ATTEMPT_ID,
+                wait_for_action_attempt={"timeout": 0.1, "polling_interval": 60},
+            )
+
+        # One request resolves the route, and the wait still polls once
+        # before the deadline passes instead of sleeping a full interval.
+        assert len(requests) == 2
+        assert time.monotonic() - start < 5
+
+
+async def test_wait_for_action_attempt_rejects_a_zero_polling_interval_async(
+    recording_server,
+):
+    with recording_server([(200, PENDING_ACTION_ATTEMPT_RESPONSE)]) as (
+        endpoint,
+        requests,
+    ):
+        async with AsyncSeam(api_key="seam_apikey_token", endpoint=endpoint) as seam:
+            with pytest.raises(
+                SeamInvalidOptionsError,
+                match="The polling_interval option must be greater than zero, got 0",
+            ):
+                await seam.action_attempts.get(
+                    action_attempt_id=PENDING_ACTION_ATTEMPT_ID,
+                    wait_for_action_attempt={"timeout": 1, "polling_interval": 0},
+                )
+
+            assert len(requests) == 1
+
+
+async def test_wait_for_action_attempt_polls_at_least_once_before_timing_out_async(
+    recording_server,
+):
+    with recording_server([(200, PENDING_ACTION_ATTEMPT_RESPONSE)]) as (
+        endpoint,
+        requests,
+    ):
+        async with AsyncSeam(api_key="seam_apikey_token", endpoint=endpoint) as seam:
+            start = time.monotonic()
+
+            with pytest.raises(SeamActionAttemptTimeoutError):
+                await seam.action_attempts.get(
+                    action_attempt_id=PENDING_ACTION_ATTEMPT_ID,
+                    wait_for_action_attempt={"timeout": 0.1, "polling_interval": 60},
+                )
+
+            assert len(requests) == 2
+            assert time.monotonic() - start < 5