Skip to content

Commit 83ffacd

Browse files
Merge pull request #5249 from springfall2008/fix/octopus-rate-limit-no-retry
fix(octopus): don't re-read a rate-limited GraphQL response
2 parents 8309ea7 + c0d0aed commit 83ffacd

2 files changed

Lines changed: 35 additions & 22 deletions

File tree

‎apps/predbat/octopus.py‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,8 @@ def initialize(self, key, account_id, automatic):
553553
self.token_mint_blocked_until = None
554554
self.token_mint_block_count = 0
555555
self.token_mint_backoff_logged_at = None
556+
# Set by async_read_response() when a GraphQL response is a rate limit, so the retry loop does not re-read it
557+
self.last_read_rate_limited = False
556558
self.account_data = {}
557559
self.tariffs = {}
558560
self.saving_sessions = {}
@@ -2044,14 +2046,16 @@ async def async_read_response_retry(self, response, url, ignore_errors=False):
20442046
self.log("OctopusAPI: Aborting retry loop due to shutdown")
20452047
return None
20462048

2049+
self.last_read_rate_limited = False
20472050
data_as_json = await self.async_read_response(response, url, ignore_errors=ignore_errors)
20482051
if data_as_json is not None:
20492052
return data_as_json
20502053
else:
2051-
# 401/403 are definitive for this response. aiohttp caches the body, so
2052-
# re-reading the same response cannot change the outcome - it would only
2053-
# duplicate log lines and sleep through the backoff for nothing.
2054-
if response.status in [401, 403]:
2054+
# 401/403 and a rate limit (KT-CT-1199) are definitive for this response. aiohttp
2055+
# caches the body, so re-reading the same response cannot change the outcome - it
2056+
# would only duplicate log lines and sleep through the backoff for nothing (a
2057+
# rate-limited request logged five warnings and stalled ~35s before giving up).
2058+
if response.status in [401, 403] or self.last_read_rate_limited:
20552059
self.failures_total += 1
20562060
return None
20572061
if attempt < max_retries - 1:
@@ -2091,17 +2095,16 @@ async def async_read_response(self, response, url, ignore_errors=False):
20912095
self.log(f"Warn: OctopusAPI: Failed to extract response json: {e} - {url} - {text}")
20922096
return None
20932097

2094-
# Check for rate limit errors - these should return None immediately (no retry)
2098+
# Check for rate limit errors - these return None immediately, and async_read_response_retry()
2099+
# does not re-read them (last_read_rate_limited): re-reading the same response cannot help
20952100
if ("graphql" in url) and data_as_json and ("errors" in data_as_json):
20962101
for error in data_as_json.get("errors", []):
20972102
error_code = error.get("extensions", {}).get("errorCode")
20982103
if error_code == "KT-CT-1199":
20992104
msg = f'Warn: OctopusAPI: Rate limit error in request ({url}): {data_as_json["errors"]}'
21002105
self.log(msg)
21012106
record_api_call("octopus", False, "rate_limit")
2102-
# Don't sleep if shutting down
2103-
if not self.api_stop:
2104-
await asyncio.sleep(5) # Sleep briefly to avoid hammering
2107+
self.last_read_rate_limited = True
21052108
return None
21062109

21072110
# Return the response as-is - let caller handle other errors (including auth errors that need retry)

‎apps/predbat/tests/test_octopus_read_response_retry.py‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ async def test_octopus_read_response_retry(my_predbat):
2323
- Test 4: All retries fail, returns None and increments failures_total
2424
- Test 5: Exponential backoff timing is correct
2525
- Test 6: ignore_errors parameter is passed through correctly
26-
- Test 7: Rate limit errors trigger retry with backoff
26+
- Test 7: A rate limit is not re-read or slept on
2727
- Test 8: Auth errors trigger retry with backoff
2828
"""
2929
print("**** Running Octopus async_read_response_retry tests ****")
@@ -207,38 +207,48 @@ def create_mock_response(status, text_content, headers=None):
207207
print("PASS: ignore_errors parameter passed through correctly")
208208

209209
# Test 7: Rate limit errors trigger retry with backoff
210-
print("\n*** Test 7: Rate limit errors trigger retry with backoff ***")
210+
# Test 7: A rate limit (KT-CT-1199) is definitive for the response and is not re-read. aiohttp
211+
# caches the body, so every re-read returned the same rate limit: a single rate-limited request
212+
# logged the warning five times and stalled for ~35s (5s + 1/2/4/8s) before giving up anyway.
213+
print("\n*** Test 7: Rate limit is not re-read or slept on ***")
211214
api = OctopusAPI(my_predbat, key="test-key", account_id="test-account", automatic=False)
212215
api.failures_total = 0
216+
logged = []
217+
api.log = lambda message, *args, **kwargs: logged.append(message)
213218

214219
rate_limit_error = {
215220
"errors": [
216221
{
217222
"message": "Too many requests.",
223+
"path": ["flexPlannedDispatches"],
218224
"extensions": {"errorCode": "KT-CT-1199"},
219225
}
220226
]
221227
}
222-
successful_data = {"data": {"result": "success"}}
223228
response = create_mock_response(200, json.dumps(rate_limit_error))
224-
225-
# Mock async_read_response to return None (rate limit), then succeed
226-
api.async_read_response = AsyncMock(side_effect=[None, successful_data])
229+
graphql_url = "https://api.octopus.energy/v1/graphql/"
227230

228231
with patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep:
229-
result = await api.async_read_response_retry(response, url, ignore_errors=False)
232+
result = await api.async_read_response_retry(response, graphql_url, ignore_errors=True)
230233

231-
if result != successful_data:
232-
print(f"ERROR: Expected successful data after rate limit retry, got {result}")
234+
rate_limit_logs = [message for message in logged if "Rate limit error" in message]
235+
if result is not None:
236+
print(f"ERROR: Expected None for a rate-limited response, got {result}")
233237
failed = True
234-
elif api.async_read_response.call_count != 2:
235-
print(f"ERROR: Expected 2 calls (rate limit + retry), got {api.async_read_response.call_count}")
238+
elif response.text.await_count != 1:
239+
print(f"ERROR: Expected the rate-limited response to be read once, got {response.text.await_count}")
236240
failed = True
237-
elif mock_sleep.call_count != 1:
238-
print(f"ERROR: Expected 1 sleep call after rate limit, got {mock_sleep.call_count}")
241+
elif mock_sleep.call_count != 0:
242+
print(f"ERROR: Expected no sleep on a rate limit, got {mock_sleep.call_count} sleep calls")
243+
failed = True
244+
elif len(rate_limit_logs) != 1:
245+
print(f"ERROR: Expected one rate limit warning, got {len(rate_limit_logs)}: {rate_limit_logs}")
246+
failed = True
247+
elif api.failures_total != 1:
248+
print(f"ERROR: failures_total should be 1, got {api.failures_total}")
239249
failed = True
240250
else:
241-
print("PASS: Rate limit error triggers retry with exponential backoff")
251+
print("PASS: Rate limit returns at once, read once, no sleep, one warning")
242252

243253
# Test 8: Auth/blocked responses are definitive and are not re-read.
244254
# aiohttp caches the response body, so re-reading the same 401/403 response can only

0 commit comments

Comments
 (0)