fix: respect challenge rate limits (#1211)

Treat generic 429 responses from the challenge endpoint as bot rate limits, honoring Retry-After when present and otherwise using bounded exponential backoff. Also prevent expired outgoing challenges from bypassing the local challenge rate-limit timer.

Validation:
- python3 -m py_compile lib/lichess.py lib/matchmaking.py test_bot/test_lichess.py test_bot/test_matchmaking.py
- python -m ruff check lib/lichess.py lib/matchmaking.py test_bot/test_lichess.py test_bot/test_matchmaking.py
- python -m pytest test_bot/test_lichess.py::test_challenge_429_without_ratelimit_body_sets_bot_rate_limit test_bot/test_lichess.py::test_challenge_429_without_retry_after_uses_exponential_backoff test_bot/test_matchmaking.py::test_should_create_challenge_respects_rate_limit_when_previous_challenge_expired

Author: ilAYAli

lib/lichess.py

@@ -145,6 +145,7 @@ def __init__(self, token: str, url: str, version: str, logging_level: int, max_r
         self.logging_level = logging_level
         self.max_retries = max_retries
         self.rate_limit_timers: defaultdict[str, Timer] = defaultdict(Timer)
+        self.challenge_rate_limit_backoff = seconds(60)
 
         # Confirm that the OAuth token has the proper permission to play on lichess
         token_response = cast(TOKEN_TESTS_TYPE, self.api_post("token_test", data=token))
@@ -308,6 +309,28 @@ def handle_challenge(self, response: requests.models.Response) -> ChallengeType:
             challenge_response["bot_is_rate_limited"] = bot_is_rate_limited
             challenge_response["opponent_is_rate_limited"] = opponent_is_rate_limited
             challenge_response["rate_limit_timeout"] = delay
+        elif is_new_rate_limit(response):
+            # Generic 429 without a ratelimit body (no bot.vsBot.day key). Honor
+            # Retry-After if lichess sent it, otherwise back off exponentially
+            # (60 → 120 → 240 → 480, capped at 600s) so repeated 429s escalate
+            # the cooldown instead of retrying at the same short interval.
+            delay = None
+            retry_after = response.headers.get("Retry-After")
+            if retry_after:
+                try:
+                    delay = seconds(float(retry_after))
+                except ValueError:
+                    delay = None
+            if delay is None:
+                delay = self.challenge_rate_limit_backoff
+                self.challenge_rate_limit_backoff = min(seconds(600), self.challenge_rate_limit_backoff * 2)
+            self.set_rate_limit_delay(ENDPOINTS["challenge"], delay)
+            challenge_response["bot_is_rate_limited"] = True
+            challenge_response["opponent_is_rate_limited"] = False
+            challenge_response["rate_limit_timeout"] = delay
+        else:
+            # Any non-429 response resets the backoff to its floor.
+            self.challenge_rate_limit_backoff = seconds(60)
 
         return challenge_response
 

lib/matchmaking.py

@@ -53,15 +53,16 @@ def __init__(self, li: Lichess, config: Configuration, user_profile: UserProfile
     def should_create_challenge(self) -> bool:
         """Whether we should create a challenge."""
         matchmaking_enabled = self.matchmaking_cfg.allow_matchmaking
-        time_has_passed = self.last_game_ended_delay.is_expired() and self.rate_limit_timer.is_expired()
+        rate_limit_ok = self.rate_limit_timer.is_expired()
+        time_has_passed = self.last_game_ended_delay.is_expired()
         challenge_expired = self.last_challenge_created_delay.is_expired() and self.challenge_id
         min_wait_time_passed = self.last_challenge_created_delay.time_since_reset() > self.min_wait_time
         if challenge_expired:
             self.li.cancel(self.challenge_id)
             logger.info(f"Challenge id {self.challenge_id} cancelled.")
             self.discard_challenge(self.challenge_id)
             self.show_earliest_challenge_time()
-        return bool(matchmaking_enabled and (time_has_passed or challenge_expired) and min_wait_time_passed)
+        return bool(matchmaking_enabled and rate_limit_ok and (time_has_passed or challenge_expired) and min_wait_time_passed)
 
     def create_challenge(self, username: str, base_time: int, increment: int, days: int, variant: str,
                          mode: str) -> str:

test_bot/test_lichess.py

@@ -1,9 +1,60 @@
 """Tests for the lichess communication."""
 
 from lib import lichess
+from lib.timer import Timer, seconds
+from collections import defaultdict
+from requests.models import Response
 import logging
 import os
 import pytest
+from typing import cast
+
+
+def mock_response(status_code: int, body: dict[str, object], headers: dict[str, str] | None = None) -> Response:
+    """Create a mock HTTP response."""
+    class MockResponse:
+        def __init__(self) -> None:
+            self.status_code = status_code
+            self.headers = headers or {}
+
+        def json(self) -> dict[str, object]:
+            return dict(body)
+
+    return cast(Response, MockResponse())
+
+
+def lichess_without_init() -> lichess.Lichess:
+    """Create a minimal Lichess instance without checking a real token."""
+    li = object.__new__(lichess.Lichess)
+    li.rate_limit_timers = defaultdict(Timer)
+    li.challenge_rate_limit_backoff = seconds(60)
+    return li
+
+
+def test_challenge_429_without_ratelimit_body_sets_bot_rate_limit() -> None:
+    """Generic challenge 429s should still block new challenge attempts."""
+    li = lichess_without_init()
+    response = mock_response(429, {"error": "Too many requests. Try again later."}, {"Retry-After": "120"})
+
+    challenge_response = li.handle_challenge(response)
+
+    assert challenge_response["bot_is_rate_limited"] is True
+    assert challenge_response["opponent_is_rate_limited"] is False
+    assert challenge_response["rate_limit_timeout"] == seconds(120)
+    assert li.is_rate_limited(lichess.ENDPOINTS["challenge"])
+
+
+def test_challenge_429_without_retry_after_uses_exponential_backoff() -> None:
+    """Repeated generic challenge 429s should increase the local cooldown."""
+    li = lichess_without_init()
+    response = mock_response(429, {"error": "Too many requests. Try again later."})
+
+    first_response = li.handle_challenge(response)
+    second_response = li.handle_challenge(response)
+
+    assert first_response["rate_limit_timeout"] == seconds(60)
+    assert second_response["rate_limit_timeout"] == seconds(120)
+    assert li.challenge_rate_limit_backoff == seconds(240)
 
 
 def test_lichess() -> None:

test_bot/test_matchmaking.py

@@ -2,6 +2,7 @@
 from unittest.mock import Mock
 from lib.matchmaking import game_category, Matchmaking
 from lib.config import Configuration
+from lib.timer import Timer, seconds
 from lib.lichess_types import UserProfileType
 
 
@@ -198,6 +199,27 @@ def test_get_random_config_value__returns_specific_value() -> None:
     assert result == "atomic", f"Expected 'atomic' but got '{result}'"
 
 
+def test_should_create_challenge_respects_rate_limit_when_previous_challenge_expired() -> None:
+    """An expired previous challenge should not bypass the challenge rate-limit timer."""
+    mock_li = Mock()
+    mock_config = Configuration({
+        "challenge": {"variants": ["standard"]},
+        "matchmaking": {
+            "allow_matchmaking": True,
+            "block_list": [],
+            "online_block_list": [],
+            "challenge_timeout": 0
+        }
+    })
+    mock_user_profile: UserProfileType = {"username": "testbot", "perfs": {}}
+    matchmaking = Matchmaking(mock_li, mock_config, mock_user_profile)
+    matchmaking.challenge_id = "challenge-id"
+    matchmaking.last_challenge_created_delay.starting_time -= 100
+    matchmaking.rate_limit_timer = Timer(seconds(60))
+
+    assert matchmaking.should_create_challenge() is False
+
+
 def test_get_random_config_value__returns_from_choices_when_random() -> None:
     """Test that get_random_config_value returns a value from choices when config value is 'random'."""
     # Create mock objects