diff --git a/src/base_odds_manager.py b/src/base_odds_manager.py index f35c4d54..50b6fe63 100644 --- a/src/base_odds_manager.py +++ b/src/base_odds_manager.py @@ -12,6 +12,8 @@ Follows LEDMatrix configuration management patterns: """ import logging +import time + import requests import json from typing import Dict, Any, Optional, List @@ -45,7 +47,14 @@ class BaseOddsManager: # 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 @@ class BaseOddsManager: 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 @@ class BaseOddsManager: 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 @@ class BaseOddsManager: 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 @@ class BaseOddsManager: 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}.") diff --git a/test/test_base_odds_manager.py b/test/test_base_odds_manager.py index b08183ad..3eb591e3 100644 --- a/test/test_base_odds_manager.py +++ b/test/test_base_odds_manager.py @@ -87,7 +87,11 @@ class TestGetOdds: 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 @@ class TestLoadConfiguration: 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 diff --git a/test/test_odds_request_budget.py b/test/test_odds_request_budget.py new file mode 100644 index 00000000..233780fb --- /dev/null +++ b/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