fix(odds): stop a stalled ESPN taking the whole plugin update with it

src/base_odds_manager.py

@@ -12,6 +12,8 @@
 """
 
 import logging
+import time
+
 import requests
 import json
 from typing import Dict, Any, Optional, List
@@ -45,7 +47,14 @@ def __init__(self, cache_manager, config_manager=None):
 
         # Configuration with defaults
         self.update_interval = 3600  # 1 hour default
-        self.request_timeout = 30    # 30 seconds default
+        # Well under the plugin executor's 30s operation budget. At 30s a
+        # single stalled ESPN request consumed the entire budget and the whole
+        # update() was killed -- and odds are fetched per live game, inside the
+        # live update loop, with show_odds defaulting on. Losing one game's
+        # odds beats losing the update that carries every game's score.
+        self.request_timeout = 5
+        # Set when a request fails; until then, skip the network entirely.
+        self._skip_network_until = 0.0
         self.cache_ttl = 1800       # 30 minutes default
 
         # Load configuration if available
@@ -73,6 +82,14 @@ def _load_configuration(self):
         except Exception as e:
             self.logger.warning(f"Failed to load BaseOddsManager configuration: {e}")
 
+    # After a network failure, stop trying for this long and serve cache only.
+    # A short per-request timeout bounds one stall, but a full Sunday slate is
+    # ~16 games fetched in a loop, so 16 consecutive timeouts still blow the
+    # budget. When ESPN is unreachable it is unreachable for all of them, so
+    # the first failure is enough to know: skip the rest of this pass and try
+    # again shortly.
+    _FAILURE_COOLDOWN = 60.0
+
     def get_odds(self, sport: str | None, league: str | None, event_id: str,
                  update_interval_seconds: int = None) -> Optional[Dict[str, Any]]:
         """
@@ -101,8 +118,18 @@ def get_odds(self, sport: str | None, league: str | None, event_id: str,
             self.logger.info(f"Using cached odds from ESPN for {cache_key}")
             return cached_data
 
+        if time.monotonic() < self._skip_network_until:
+            # A recent request failed, so ESPN is very likely still unreachable.
+            # Returning now keeps the caller's update inside its time budget
+            # instead of paying the timeout again for every remaining game.
+            self.logger.debug(
+                "Skipping odds fetch for %s: a recent request failed, holding off "
+                "for another %.0fs", cache_key,
+                self._skip_network_until - time.monotonic())
+            return None
+
         self.logger.info(f"Cache miss - fetching fresh odds from ESPN for {cache_key}")
-        
+
         try:
             # Map league names to ESPN API format
             league_mapping = {
@@ -121,6 +148,8 @@ def get_odds(self, sport: str | None, league: str | None, event_id: str,
             response.raise_for_status()
             raw_data = response.json()
 
+            self._skip_network_until = 0.0   # reachable again
+
             self.logger.debug(f"Received raw odds data from ESPN: {json.dumps(raw_data, indent=2)}")
 
             odds_data = self._extract_espn_data(raw_data)
@@ -140,7 +169,11 @@ def get_odds(self, sport: str | None, league: str | None, event_id: str,
             return odds_data
 
         except requests.exceptions.RequestException as e:
-            self.logger.error(f"Error fetching odds from ESPN API for {cache_key}: {e}")
+            self._skip_network_until = time.monotonic() + self._FAILURE_COOLDOWN
+            self.logger.error(
+                "Error fetching odds from ESPN API for %s: %s. Holding off on odds "
+                "for %.0fs so a slate of games does not pay this timeout each.",
+                cache_key, e, self._FAILURE_COOLDOWN)
         except json.JSONDecodeError:
             self.logger.error(f"Error decoding JSON response from ESPN API for {cache_key}.")
 

test/test_base_odds_manager.py

@@ -87,7 +87,11 @@ def test_cache_key_and_url(self, manager, cache_manager, mock_get):
         assert '/events/401/competitions/401/odds' in url
         assert url == ('https://sports.core.api.espn.com/v2/sports/football/'
                        'leagues/nfl/events/401/competitions/401/odds')
-        assert mock_get.call_args.kwargs['timeout'] == 30
+        # The number matters less than the property: a single stalled request
+        # must not be able to consume the plugin executor's 30s operation
+        # budget, since odds are fetched per live game inside update().
+        assert mock_get.call_args.kwargs['timeout'] == 5
+        assert mock_get.call_args.kwargs['timeout'] < 30
 
     def test_ncaa_fb_maps_to_college_football(self, manager, mock_get):
         manager.get_odds('football', 'ncaa_fb', '401')
@@ -355,5 +359,5 @@ def test_get_config_raising_keeps_defaults(self, cache_manager):
         manager = BaseOddsManager(cache_manager, config_manager=config_manager)
 
         assert manager.update_interval == 3600
-        assert manager.request_timeout == 30
+        assert manager.request_timeout == 5
         assert manager.cache_ttl == 1800

test/test_odds_request_budget.py

@@ -0,0 +1,122 @@
+"""Tests that a slow ESPN cannot take a whole plugin update with it.
+
+Odds are fetched per live game from inside SportsLive.update(), with show_odds
+defaulting on, and the plugin executor kills an operation at 30s. The odds
+request timeout was also 30s, so one stalled request consumed the entire budget
+and the update carrying every game's score was killed:
+
+    00:43:43  ERROR  plugin football-scoreboard operation timed out after 30.0s
+    01:43:43  ERROR  plugin football-scoreboard operation timed out after 30.0s
+
+Invisible out of season -- preseason week 1 returns a single game -- and a
+Sunday slate is around sixteen.
+"""
+
+from unittest.mock import Mock
+
+from src.base_odds_manager import BaseOddsManager
+
+PLUGIN_BUDGET = 30.0   # PluginExecutor(default_timeout=30.0)
+
+
+def _manager(cache=None):
+    cache = cache or Mock()
+    cache.get_with_auto_strategy.return_value = None
+    return BaseOddsManager(cache_manager=cache, config_manager=None)
+
+
+class TestRequestTimeout:
+    def test_leaves_room_in_the_operation_budget(self):
+        assert _manager().request_timeout < PLUGIN_BUDGET / 2
+
+    def test_the_timeout_is_the_one_actually_used(self):
+        m = _manager()
+        import src.base_odds_manager as mod
+        real = mod.requests.get
+        try:
+            mod.requests.get = Mock(side_effect=mod.requests.exceptions.Timeout("x"))
+            m.get_odds("football", "nfl", "401")
+            assert mod.requests.get.call_args.kwargs["timeout"] == m.request_timeout
+        finally:
+            mod.requests.get = real
+
+
+class TestSlowEspnCannotKillTheUpdate:
+    def test_one_failure_stops_the_rest_of_the_slate_hitting_the_network(self):
+        m = _manager()
+        import src.base_odds_manager as mod
+        real = mod.requests.get
+        calls = {"n": 0}
+
+        def timeout(*a, **k):
+            calls["n"] += 1
+            raise mod.requests.exceptions.Timeout("timed out")
+
+        try:
+            mod.requests.get = timeout
+            for i in range(16):          # a full slate, one game at a time
+                m.get_odds("football", "nfl", "4018730%02d" % i)
+        finally:
+            mod.requests.get = real
+
+        assert calls["n"] == 1, (
+            "%d games each paid the timeout; the breaker should have stopped "
+            "after the first" % calls["n"])
+
+    def test_worst_case_slate_stays_inside_the_budget(self):
+        m = _manager()
+        assert m.request_timeout * 1 < PLUGIN_BUDGET
+
+    def test_recovery_is_automatic(self):
+        m = _manager()
+        import src.base_odds_manager as mod
+        real_get, real_monotonic = mod.requests.get, mod.time.monotonic
+        clock = {"t": 1000.0}
+        try:
+            mod.time.monotonic = lambda: clock["t"]
+            mod.requests.get = Mock(
+                side_effect=mod.requests.exceptions.Timeout("timed out"))
+            m.get_odds("football", "nfl", "401")
+            assert m._skip_network_until > clock["t"], "breaker did not open"
+
+            clock["t"] += 1
+            before = mod.requests.get.call_count
+            m.get_odds("football", "nfl", "402")
+            assert mod.requests.get.call_count == before, "should not have retried"
+
+            clock["t"] += m._FAILURE_COOLDOWN
+            m.get_odds("football", "nfl", "403")
+            assert mod.requests.get.call_count > before, "never retried"
+        finally:
+            mod.requests.get, mod.time.monotonic = real_get, real_monotonic
+
+    def test_a_healthy_fetch_clears_the_breaker(self):
+        m = _manager()
+        m._skip_network_until = 0.0
+        m._extract_espn_data = Mock(return_value=None)
+        import src.base_odds_manager as mod
+        real = mod.requests.get
+        try:
+            resp = Mock()
+            resp.json.return_value = {}
+            resp.raise_for_status.return_value = None
+            mod.requests.get = Mock(return_value=resp)
+            m.get_odds("football", "nfl", "401")
+        finally:
+            mod.requests.get = real
+        assert m._skip_network_until == 0.0
+
+    def test_the_stale_cache_fallback_still_works(self):
+        # The failing request must still hand back whatever was cached; only
+        # the *subsequent* games skip the network.
+        cache = Mock()
+        cache.get_with_auto_strategy.side_effect = [None, {"details": "stale"}]
+        m = BaseOddsManager(cache_manager=cache, config_manager=None)
+        import src.base_odds_manager as mod
+        real = mod.requests.get
+        try:
+            mod.requests.get = Mock(
+                side_effect=mod.requests.exceptions.Timeout("timed out"))
+            assert m.get_odds("football", "nfl", "401") == {"details": "stale"}
+        finally:
+            mod.requests.get = real