rick/LEDMatrix
1
0
Fork 0
mirror of https://github.com/ChuckBuilds/LEDMatrix.git synced 2026-10-04 14:25:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(sports): ask the endpoint the league actually publishes for standings (#600)

Browse Source 
* fix(sports): ask the endpoint the league actually publishes for standings

ESPNDataSource.fetch_standings tried /standings first regardless of league
and fell back to /rankings only on a 404. College leagues answer /standings
with a 200 that carries no poll, so the fallback never fired and the poll
came back empty every time. Nothing failed; the rank badge simply never
appeared, and anything keyed off rankings quietly did nothing.

Endpoints are now ordered by whether the league publishes a poll, a 200
that lacks the key counts as a miss so a league answering both still ends
up with whichever one carries the poll, and only a 404 is treated as
routine -- it is how a league says it has none. A connection error, a
timeout or an unparseable body is logged as an error again.

This is the implementation the football, baseball and hockey boards already
ship; core was the last copy still on the old one. Verified against live
ESPN: mens-college-basketball returns a populated rankings key where it
previously returned nothing, and nba still resolves from /standings alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

* fix(standings): stop the endpoint handler from swallowing its own bugs

Addresses both CodeRabbit findings on #600.

The handler caught `Exception`, so an AttributeError or TypeError raised
while *inspecting* the payload was indistinguishable from an endpoint that
failed. The loop would move on and, if the other endpoint had nothing
either, return {} -- silently dropping rankings for a league that has them.
That is the precise failure this function was written to fix, so the
handler was able to reintroduce it.

Only the request is guarded now. `requests.RequestException` covers the
transport failures and `ValueError` covers a body that will not parse;
payload inspection happens after the handler, where a bug surfaces instead
of being logged as a missing poll. A non-dict payload is treated as a miss
explicitly rather than by tripping over `.get`.

Tests: the fallback paths had no coverage -- the old single-endpoint code
would have passed the suite unchanged. Added order assertions for both
league kinds, a 200-without-a-poll fall-through, 404 and non-404 recovery,
a non-object payload, and a guard proving a bug is no longer swallowed.

`test_fetch_standings_returns_empty_on_error` faked a transport failure
with a bare `Exception`, which only passed because the handler caught
everything. It now raises ConnectionError, which is what actually happens.

Verified by mutation: restoring standings-first fails 5 tests, restoring
the catch-all fails the bug-not-swallowed guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-09-21 16:07:51 -04:00
committed by GitHub
co-authored by Claude Opus 5
parent 81e1bc596f
commit cf02538d2e
2 changed files with 160 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/base_classes/data_sources.py
+61 -28
View File
@@ -114,35 +114,68 @@ class ESPNDataSource(DataSource):
return []
def fetch_standings(self, sport: str, league: str) -> Dict:
"""Fetch standings from ESPN API."""
# Try standings endpoint first (for professional leagues like NFL, NBA, etc.)
try:
url = f"{self.base_url}/{sport}/{league}/standings"
response = self.session.get(url, headers=self.get_headers(), timeout=15)
response.raise_for_status()
data = response.json()
self.logger.debug(f"Fetched standings for {sport}/{league}")
"""Fetch standings, or the poll for leagues that have one.
Order matters and used to be wrong. College leagues publish a poll at
/rankings and a records table at /standings; professional leagues have
only /standings. The old code tried /standings first and fell back to
/rankings only on a 404 -- but college /standings answers 200, so the
fallback never fired and college rankings came back empty forever.
Nothing failed; the AP rank badge simply never appeared, and anything
else keyed off rankings quietly did nothing.
A 200 that lacks the key is treated as a miss, so a league answering
both endpoints still ends up with whichever one actually carries a poll.
"""
league_name = (league or "").lower()
wants_poll = "college" in league_name or "ncaa" in league_name
endpoints = ["rankings", "standings"] if wants_poll else ["standings", "rankings"]
for endpoint in endpoints:
url = f"{self.base_url}/{sport}/{league}/{endpoint}"
# Only the request is guarded. Inspecting the payload happens
# below, outside the handler, so that a bug in this method cannot
# be mistaken for an endpoint that failed -- that mistake would
# silently drop rankings for a league that has them, which is the
# exact failure this function was written to fix.
try:
response = self.session.get(
url, headers=self.get_headers(), timeout=15
)
response.raise_for_status()
data = response.json()
except (requests.RequestException, ValueError) as e:
status = getattr(getattr(e, "response", None), "status_code", None)
# Only a 404 is routine -- it is how a league says "no poll
# here". Everything else is worth an error, and `status is
# None` covers the ones that matter most: ConnectionError,
# Timeout, a body that would not parse. Silencing those left a
# board that could not reach ESPN with one debug line, and the
# ranked filter running on an empty table.
if status != 404:
self.logger.error(
f"Error fetching {endpoint} from ESPN for "
f"{sport}/{league}: {e}"
)
continue
if not isinstance(data, dict):
# A list or a bare string is not something the callers can
# read. Treat it as a miss so the other endpoint still gets a
# turn, but say so -- this means ESPN changed shape.
self.logger.error(
f"Unexpected {endpoint} payload for {sport}/{league}: "
f"got {type(data).__name__}, expected an object"
)
continue
if endpoint == "rankings" and not data.get("rankings"):
continue
self.logger.debug(f"Fetched {endpoint} for {sport}/{league}")
return data
except Exception as e:
# If standings doesn't exist, try rankings (for college sports)
if hasattr(e, 'response') and hasattr(e.response, 'status_code') and e.response.status_code == 404:
try:
url = f"{self.base_url}/{sport}/{league}/rankings"
response = self.session.get(url, headers=self.get_headers(), timeout=15)
response.raise_for_status()
data = response.json()
self.logger.debug(f"Fetched rankings for {sport}/{league}")
return data
except Exception:
# Both endpoints failed - standings/rankings may not be available for this sport/league
self.logger.debug(f"Standings/rankings not available for {sport}/{league} from ESPN API")
return {}
else:
# Non-404 error - log at debug level since standings are optional
self.logger.debug(f"Error fetching standings from ESPN for {sport}/{league}: {e}")
return {}
self.logger.debug(
f"Standings/rankings not available for {sport}/{league} from ESPN API"
)
return {}
class MLBAPIDataSource(DataSource):
test/test_data_sources.py
+99 -1
View File
@@ -92,10 +92,108 @@ class TestESPNDataSource:
assert result == payload
def test_fetch_standings_returns_empty_on_error(self):
with patch.object(self.source.session, "get", side_effect=Exception("error")):
# A transport failure is a RequestException, not a bare Exception.
# The old stand-in passed only because the handler caught everything,
# including bugs in the method under test.
with patch.object(self.source.session, "get",
side_effect=requests.ConnectionError("error")):
result = self.source.fetch_standings("football", "nfl")
assert result == {}
# ------------------------------------------------------------------
# fetch_standings endpoint selection
#
# College leagues publish a poll at /rankings and a records table at
# /standings; professional leagues have only /standings. Probing them in
# the wrong order still returns 200 -- just without a poll in it -- so
# nothing failed and the rank badge simply never appeared. Order is the
# behaviour here, so these tests assert it directly.
# ------------------------------------------------------------------
@staticmethod
def _requested_endpoints(mock_get):
"""The endpoint names probed, in the order they were requested."""
return [call.args[0].rsplit("/", 1)[-1] for call in mock_get.call_args_list]
def test_professional_league_asks_standings_first(self):
payload = {"standings": []}
with patch.object(self.source.session, "get",
return_value=_mock_response(payload)) as mock_get:
result = self.source.fetch_standings("football", "nfl")
assert result == payload
assert self._requested_endpoints(mock_get) == ["standings"]
def test_college_league_asks_rankings_first(self):
poll = {"rankings": [{"name": "AP Top 25"}]}
with patch.object(self.source.session, "get",
return_value=_mock_response(poll)) as mock_get:
result = self.source.fetch_standings("football", "college-football")
assert result == poll
assert self._requested_endpoints(mock_get) == ["rankings"]
def test_rankings_200_without_a_poll_falls_through_to_standings(self):
"""A 200 is not the same as an answer.
This is the case the old code could not see: the endpoint responded,
so nothing raised, but the body carried no poll.
"""
empty_poll = _mock_response({"rankings": []})
table = _mock_response({"standings": [{"entries": []}]})
with patch.object(self.source.session, "get",
side_effect=[empty_poll, table]) as mock_get:
result = self.source.fetch_standings(
"basketball", "mens-college-basketball")
assert result == {"standings": [{"entries": []}]}
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
def test_404_on_the_first_endpoint_falls_through_quietly(self):
missing = _mock_response({}, status_code=404)
table = _mock_response({"standings": []})
with patch.object(self.source.session, "get",
side_effect=[missing, table]) as mock_get:
result = self.source.fetch_standings("baseball", "college-baseball")
assert result == {"standings": []}
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
def test_recovers_from_a_non_404_failure_on_the_first_endpoint(self):
table = _mock_response({"standings": [{"entries": []}]})
with patch.object(self.source.session, "get",
side_effect=[requests.ConnectionError("reset"), table]) as mock_get:
result = self.source.fetch_standings("football", "college-football")
assert result == {"standings": [{"entries": []}]}
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
def test_both_endpoints_failing_returns_empty(self):
with patch.object(self.source.session, "get",
side_effect=requests.ConnectionError("down")) as mock_get:
result = self.source.fetch_standings("football", "nfl")
assert result == {}
assert self._requested_endpoints(mock_get) == ["standings", "rankings"]
def test_a_non_object_payload_is_treated_as_a_miss(self):
odd = _mock_response(["not", "an", "object"])
table = _mock_response({"standings": []})
with patch.object(self.source.session, "get",
side_effect=[odd, table]) as mock_get:
result = self.source.fetch_standings("football", "college-football")
assert result == {"standings": []}
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
def test_a_bug_in_this_method_is_not_swallowed_as_a_failed_endpoint(self):
"""The guard for the narrowed handler.
An error raised while reading the payload used to be caught by the
endpoint handler and reported as 'no poll here', which would silently
drop rankings for a league that has them. It must surface instead.
"""
boom = Mock(spec=requests.Response)
boom.status_code = 200
boom.raise_for_status = Mock()
boom.json.side_effect = TypeError("a bug, not a network failure")
with patch.object(self.source.session, "get", return_value=boom):
with pytest.raises(TypeError):
self.source.fetch_standings("football", "nfl")
def test_base_url_set_correctly(self):
assert "espn.com" in self.source.base_url