Skip to content

Commit ed8db21

Browse files
CIclaude
andcommitted
fix(kraken): keep the rates fallback on retry errors and count failures once
Review follow-ups on the HTTP 400 rates fallback: - A network error during the authenticated retry returned (None, None), erasing the 404/410 the public attempt had just proved and dropping the cycle into the generic failure path. Keep the original status so the GraphQL fallback still runs for a tariff the products API has already refused. - async_graphql_query counts its own failures, so a failed GraphQL fallback was counted twice per cycle. Only add a count when the fallback returned empty without counting one itself. - 410 (product retired) was logged as 'private product'; use a label that fits both statuses. - async_fetch_standing_charges now shares KRAKEN_REST_PRODUCT_NOT_FOUND_STATUSES instead of a third inline (404, 410) literal, and its docstring no longer claims it falls back on any non-200. - Restore the E-TOU-* example to the 404 explanation it was documented under; only the day/night EDF 400 is evidenced by GH#5166. Tests: export-tariff 400 fallback routing, network error on the authenticated retry, single failure count for a failed GraphQL fallback; the two existing 400 tests now reuse the file's _mk_get_ctx helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 53e0eae commit ed8db21

2 files changed

Lines changed: 140 additions & 29 deletions

File tree

‎apps/predbat/kraken.py‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,10 @@ async def handle_oauth_401(self):
9595
KRAKEN_VIEWER_QUERY = """{ viewer { accounts { number } } }"""
9696

9797
# GraphQL applicableRates query — fallback when the REST product endpoint cannot serve the
98-
# tariff: 404 (product code removed/replaced while customer is still on the tariff, or a private
99-
# product such as an EDF SEG export tariff) or 400 (tariff has no /standard-unit-rates/ REST
100-
# endpoint, e.g. E-TOU-* on E.ON Next or day/night-structured EDF tariffs).
98+
# tariff: 404/410 (product code removed/replaced while customer is still on the tariff, a private
99+
# product such as an EDF SEG export tariff, or a TOU tariff absent from the REST API such as
100+
# E-TOU-* on E.ON Next) or 400 (the product is in the REST API but has no /standard-unit-rates/
101+
# resource, as day/night-structured EDF tariffs answer — GH#5166).
101102
# Returns value (pence/kWh inc VAT), validFrom, validTo for the requested window.
102103
# applicableRates is a Relay-style connection (ApplicableRateConnectionTypeConnection),
103104
# so the rate fields live under edges { node { ... } }, not directly on the field. The
@@ -849,7 +850,8 @@ async def async_fetch_rates(self, tariff=None):
849850
query (which is not available on every provider), so export rates are recovered wherever
850851
possible. A 400 goes straight to that fallback without the authenticated retry: it means
851852
the tariff has no /standard-unit-rates/ resource (day/night-structured tariffs), which
852-
auth cannot change.
853+
auth cannot change. A network error during the authenticated retry keeps the status the
854+
public attempt returned, so the fallback still runs for a tariff already proven 404/410.
853855
"""
854856
tariff = tariff or self.current_tariff
855857
if not tariff:
@@ -871,7 +873,15 @@ async def async_fetch_rates(self, tariff=None):
871873
# 2) On a permanent "not found", the product is likely private — retry authenticated.
872874
if err in KRAKEN_REST_PRODUCT_NOT_FOUND_STATUSES:
873875
self.log(f"Kraken: REST rates HTTP {err} for {tariff['tariff_code']}, retrying authenticated")
876+
rest_unavailable_status = err
874877
results, err = await self._fetch_rates_rest(url, authenticate=True)
878+
if results is None and err is None:
879+
# (None, None) is a network error on the retry, which would otherwise erase the
880+
# status the public attempt just proved and drop this cycle into the generic
881+
# failure path. The products API still cannot serve this tariff, so keep that
882+
# status and take the GraphQL fallback rather than losing the rates to a blip.
883+
self.log(f"Warn: Kraken: Authenticated retry for {tariff['tariff_code']} hit a network error, treating as HTTP {rest_unavailable_status}")
884+
err = rest_unavailable_status
875885

876886
if err is not None:
877887
# A "REST cannot serve this tariff" status with a known MPAN is the EXPECTED path for
@@ -883,10 +893,16 @@ async def async_fetch_rates(self, tariff=None):
883893
# fall through to the genuine-failure path below.
884894
if err in KRAKEN_REST_RATES_UNAVAILABLE_STATUSES and fallback_mpan:
885895
kind = "export" if is_export else "import"
886-
reason = "private product" if err in KRAKEN_REST_PRODUCT_NOT_FOUND_STATUSES else "tariff has no standard-unit-rates endpoint"
896+
# 404/410 covers both a private product and one retired from the REST API, so the
897+
# label has to fit both — a retired product is not a private one.
898+
reason = "product not in the REST API" if err in KRAKEN_REST_PRODUCT_NOT_FOUND_STATUSES else "tariff has no standard-unit-rates endpoint"
887899
self.log(f"Kraken: REST rates HTTP {err} for {tariff['tariff_code']} ({reason}), using GraphQL applicableRates for {kind} MPAN {fallback_mpan}")
900+
failures_before = self.failures_total
888901
rates = await self.async_fetch_rates_graphql(fallback_mpan, account_id=fallback_account)
889-
if rates is None:
902+
if rates is None and self.failures_total == failures_before:
903+
# async_graphql_query counts its own failures (auth, non-200, GraphQL errors,
904+
# network), so only add a count when the fallback came back empty without
905+
# counting one — otherwise a down GraphQL would score 2 per cycle.
890906
self.failures_total += 1
891907
return rates
892908
self.log(f"Warn: Kraken: Rates HTTP {err} for {url}")
@@ -951,9 +967,11 @@ async def async_fetch_standing_charges_graphql(self, mpan):
951967
async def async_fetch_standing_charges(self, tariff=None):
952968
"""Fetch standing charges from public REST endpoint. No auth needed.
953969
954-
Falls back to GraphQL applicableStandingCharges if the REST endpoint returns a non-200
955-
status (e.g. 404 for TOU tariffs on E.ON Next whose product is not in the REST API)
956-
and self.import_mpan is known.
970+
Falls back to GraphQL applicableStandingCharges only on a permanent "product not found"
971+
status (404/410 — e.g. TOU tariffs on E.ON Next whose product is not in the REST API)
972+
when self.import_mpan is known. Transient statuses (429/500/503) are counted as failures
973+
instead, and unlike the rates endpoint a 400 does not arise here: /standing-charges/ is
974+
register-independent, so day/night-structured tariffs still serve it (GH#5166).
957975
"""
958976
tariff = tariff or self.current_tariff
959977
if not tariff:
@@ -979,10 +997,12 @@ async def async_fetch_standing_charges(self, tariff=None):
979997
return None
980998

981999
if http_error_status is not None:
982-
# Only fall back to GraphQL for permanent "product not found" responses (404/410).
983-
# Transient errors (429, 500, 503, …) should surface as failures, not trigger
984-
# an extra GraphQL request that would mask the outage.
985-
if http_error_status in (404, 410) and self.import_mpan:
1000+
# Only fall back to GraphQL for permanent "product not found" responses (404/410) —
1001+
# the same statuses the rates fallback retries authenticated on, shared via the
1002+
# constant so the gates cannot drift. Transient errors (429, 500, 503, …) should
1003+
# surface as failures, not trigger an extra GraphQL request that would mask the
1004+
# outage. 400 is deliberately absent: it is a rates-endpoint-only answer.
1005+
if http_error_status in KRAKEN_REST_PRODUCT_NOT_FOUND_STATUSES and self.import_mpan:
9861006
self.log(f"Kraken: REST standing charges returned HTTP {http_error_status}, falling back to GraphQL applicableStandingCharges for MPAN {self.import_mpan}")
9871007
return await self.async_fetch_standing_charges_graphql(self.import_mpan)
9881008
return None

‎apps/predbat/tests/test_kraken.py‎

Lines changed: 107 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,15 +1210,8 @@ def test_fetch_rates_rest_400_day_night_tariff_falls_back_to_graphql():
12101210
graphql_rates = [{"value_inc_vat": 27.5, "value_exc_vat": round(27.5 / 1.05, 4), "valid_from": "2026-09-19T00:00:00Z", "valid_to": "2026-09-21T00:00:00Z"}]
12111211
api.async_fetch_rates_graphql = AsyncMock(return_value=graphql_rates)
12121212

1213-
mock_response = AsyncMock()
1214-
mock_response.status = 400
12151213
mock_session = AsyncMock()
1216-
mock_session.get = MagicMock(
1217-
return_value=AsyncMock(
1218-
__aenter__=AsyncMock(return_value=mock_response),
1219-
__aexit__=AsyncMock(return_value=None),
1220-
)
1221-
)
1214+
mock_session.get = MagicMock(side_effect=lambda url, **kwargs: _mk_get_ctx(400))
12221215
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
12231216
mock_session.__aexit__ = AsyncMock(return_value=None)
12241217

@@ -1231,6 +1224,38 @@ def test_fetch_rates_rest_400_day_night_tariff_falls_back_to_graphql():
12311224
assert api.failures_total == 0, "A recovered 400 must not count as a failure"
12321225

12331226

1227+
def test_fetch_rates_rest_400_falls_back_to_graphql_for_export():
1228+
"""A day/night-structured EXPORT tariff's 400 falls back on the export MPAN and account.
1229+
1230+
The 400 gate shares the fallback wiring with the 404 path, which selects export_mpan and
1231+
export_account_id for an export tariff — but only the 404 side of that wiring was covered,
1232+
so a 400 mis-routed to the import MPAN/account would have passed the suite.
1233+
"""
1234+
api = make_kraken_api(provider="edf", account_id="A-9C006563", export_account_id="A-EXPORT456")
1235+
api.current_tariff = {"tariff_code": "E-1R-IMP-01-J", "product_code": "IMP-01"}
1236+
api.import_mpan = "1900000000456"
1237+
export_tariff = {"tariff_code": "E-FLAT2R-EDF_EXPORT_VARIABLE-D", "product_code": "EDF_EXPORT_VARIABLE"}
1238+
api.export_tariff = export_tariff
1239+
api.export_mpan = "2000000000789"
1240+
api.failures_total = 0
1241+
1242+
graphql_rates = [{"value_inc_vat": 15.0, "value_exc_vat": round(15.0 / 1.05, 4), "valid_from": "2026-09-19T00:00:00Z", "valid_to": "2026-09-21T00:00:00Z"}]
1243+
api.async_fetch_rates_graphql = AsyncMock(return_value=graphql_rates)
1244+
1245+
mock_session = AsyncMock()
1246+
mock_session.get = MagicMock(side_effect=lambda url, **kwargs: _mk_get_ctx(400))
1247+
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
1248+
mock_session.__aexit__ = AsyncMock(return_value=None)
1249+
1250+
with patch("aiohttp.ClientSession", return_value=mock_session):
1251+
result = asyncio.run(api.async_fetch_rates(tariff=export_tariff))
1252+
1253+
assert result is graphql_rates
1254+
api.async_fetch_rates_graphql.assert_called_once_with("2000000000789", account_id="A-EXPORT456")
1255+
assert mock_session.get.call_count == 1, f"400 must not trigger an authenticated retry, got {mock_session.get.call_count} REST requests"
1256+
assert api.failures_total == 0, "A recovered 400 must not count as a failure"
1257+
1258+
12341259
def test_fetch_rates_rest_400_no_fallback_without_import_mpan():
12351260
"""async_fetch_rates() returns None and counts a failure when REST 400s with no MPAN to fall back on."""
12361261
api = make_kraken_api(provider="edf")
@@ -1239,15 +1264,8 @@ def test_fetch_rates_rest_400_no_fallback_without_import_mpan():
12391264
api.failures_total = 0
12401265
api.async_fetch_rates_graphql = AsyncMock()
12411266

1242-
mock_response = AsyncMock()
1243-
mock_response.status = 400
12441267
mock_session = AsyncMock()
1245-
mock_session.get = MagicMock(
1246-
return_value=AsyncMock(
1247-
__aenter__=AsyncMock(return_value=mock_response),
1248-
__aexit__=AsyncMock(return_value=None),
1249-
)
1250-
)
1268+
mock_session.get = MagicMock(side_effect=lambda url, **kwargs: _mk_get_ctx(400))
12511269
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
12521270
mock_session.__aexit__ = AsyncMock(return_value=None)
12531271

@@ -1463,6 +1481,79 @@ def test_fetch_rates_404_graphql_fallback_empty_counts_one_failure():
14631481
assert api.failures_total == 1
14641482

14651483

1484+
def test_fetch_rates_404_then_network_error_on_retry_still_falls_back_to_graphql():
1485+
"""A network error during the authenticated retry must not lose the GraphQL fallback.
1486+
1487+
_fetch_rates_rest returns (None, None) on a network error, which would otherwise erase the
1488+
404 the public attempt just proved and drop the cycle into the generic-failure path — losing
1489+
a private product's rates to a transient blip even though GraphQL would have served them.
1490+
"""
1491+
import aiohttp
1492+
1493+
api = make_kraken_api()
1494+
export_tariff = {"tariff_code": "E-1R-EDF_EXPORT_SEG_12M_HH-B", "product_code": "EDF_EXPORT_SEG_12M"}
1495+
api.export_tariff = export_tariff
1496+
api.export_mpan = "2000000000789"
1497+
api.failures_total = 0
1498+
1499+
graphql_rates = [{"value_inc_vat": 15.0, "value_exc_vat": round(15.0 / 1.05, 4), "valid_from": "2026-09-19T00:00:00Z", "valid_to": "2026-09-21T00:00:00Z"}]
1500+
api.async_fetch_rates_graphql = AsyncMock(return_value=graphql_rates)
1501+
1502+
call_count = [0]
1503+
1504+
def get_side_effect(url, **kwargs):
1505+
"""Public attempt 404s; the authenticated retry fails with a connection error."""
1506+
call_count[0] += 1
1507+
if call_count[0] == 1:
1508+
return _mk_get_ctx(404)
1509+
raise aiohttp.ClientConnectionError("connection reset")
1510+
1511+
mock_session = AsyncMock()
1512+
mock_session.get = MagicMock(side_effect=get_side_effect)
1513+
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
1514+
mock_session.__aexit__ = AsyncMock(return_value=None)
1515+
1516+
with patch("aiohttp.ClientSession", return_value=mock_session):
1517+
result = asyncio.run(api.async_fetch_rates(tariff=export_tariff))
1518+
1519+
assert result is graphql_rates
1520+
assert call_count[0] == 2, "expected an unauthenticated attempt then an authenticated retry"
1521+
api.async_fetch_rates_graphql.assert_called_once_with("2000000000789", account_id="A-TEST123")
1522+
assert api.failures_total == 0, "rates recovered via GraphQL must not count as a failure"
1523+
1524+
1525+
def test_fetch_rates_graphql_fallback_failure_counts_one_failure_not_two():
1526+
"""A GraphQL fallback that fails internally is counted once, not once per counting layer.
1527+
1528+
async_graphql_query increments failures_total on every one of its own failure paths (auth
1529+
unavailable, no token, non-200, GraphQL errors, network), so async_fetch_rates must not add
1530+
a second count for the same cycle: for a day/night tariff whose GraphQL is down that would
1531+
score 2 every 5 minutes and mislead anything watching the metric for component health.
1532+
"""
1533+
api = make_kraken_api(provider="edf", account_id="A-9C006563")
1534+
api.current_tariff = {"tariff_code": "E-FLAT2R-EDF_STANDARD_VARIABLE-D", "product_code": "EDF_STANDARD_VARIABLE"}
1535+
api.import_mpan = "1900000000456"
1536+
api.failures_total = 0
1537+
1538+
def graphql_failure(mpan, account_id=None):
1539+
"""Stand in for a fallback that failed inside async_graphql_query, which counts its own."""
1540+
api.failures_total += 1
1541+
return None
1542+
1543+
api.async_fetch_rates_graphql = AsyncMock(side_effect=graphql_failure)
1544+
1545+
mock_session = AsyncMock()
1546+
mock_session.get = MagicMock(side_effect=lambda url, **kwargs: _mk_get_ctx(400))
1547+
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
1548+
mock_session.__aexit__ = AsyncMock(return_value=None)
1549+
1550+
with patch("aiohttp.ClientSession", return_value=mock_session):
1551+
result = asyncio.run(api.async_fetch_rates())
1552+
1553+
assert result is None
1554+
assert api.failures_total == 1, f"one failed cycle must count once, got {api.failures_total}"
1555+
1556+
14661557
def test_connection_nodes_extracts_edges():
14671558
"""_connection_nodes returns node dicts from a Relay connection and tolerates other shapes."""
14681559
from kraken import KrakenAPI

0 commit comments

Comments
 (0)