-
-
Notifications
You must be signed in to change notification settings - Fork 94
Fix Octopus API rate limiting and auth error retry handling #3105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add rate limit error detection (KT-CT-1199) with immediate return - Implement automatic token refresh and retry for auth errors (KT-CT-1139/1111/1143) - Preserve existing data when API calls fail due to rate limiting - Return full response from async_read_response except for rate limits - Handle auth errors in async_graphql_query with single retry after token refresh - Add comprehensive test suite (test_octopus_rate_limit.py) with 11 test scenarios - Update test_octopus_read_response.py to match new error handling behavior - Fix test cleanup: restore mocked functions to prevent test interference - Clear dummy_items in fetch_octopus_rates test for test isolation All octopus tests now pass (13/13).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements robust error handling for Octopus Energy API rate limiting and authentication errors. It addresses rate limit errors (KT-CT-1199) by immediately returning None without retry, while implementing automatic token refresh with one retry for authentication errors (KT-CT-1139/1111/1143). The implementation ensures existing cached data is preserved when API calls fail due to rate limiting.
Key changes:
- Rate limit errors are detected early in
async_read_responseand return None immediately to prevent data corruption - Auth errors trigger automatic token refresh and single retry in
async_graphql_querywith infinite loop prevention - Comprehensive test suite validates all error scenarios including data preservation, token refresh failures, and multi-error handling
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/predbat/octopus.py | Core implementation: rate limit detection in async_read_response, auth error retry logic in async_graphql_query, and improved error handling with data preservation |
| apps/predbat/tests/test_octopus_rate_limit.py | New comprehensive test suite covering rate limiting, token refresh, auth errors, and data preservation scenarios across 11 test cases |
| apps/predbat/tests/test_octopus_read_response.py | Updated test expectations to reflect new behavior where full response data is returned for non-rate-limit errors |
| apps/predbat/tests/test_fetch_octopus_rates.py | Test isolation fix: clears dummy_items to prevent interference between tests |
| apps/predbat/unit_test.py | Integration of new test suite into test runner |
| apps/predbat/predbat.py | Version bump to v8.30.4 |
| - Test 5: Rate limit followed by successful request | ||
| - Test 6: Rate limit error on async_get_account - account_data preserved | ||
| - Test 7: async_graphql_query fails immediately when token refresh fails | ||
| - Test 8: Token refresh fails during retry after auth error |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test 8 is documented in the docstring but is not implemented in the test function. Either implement the test case "Token refresh fails during retry after auth error" or remove it from the documentation to ensure the test suite accurately reflects what is being tested.
| print("PASS: ignore_errors=True suppresses failure counting when token refresh fails") | ||
|
|
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "failures_total should increment even with ignore errors (due to API key)" but this is misleading. The failures_total increments on token refresh failure regardless of the ignore_errors parameter. The increment occurs at line 1234 of octopus.py before any ignore_errors check. Consider clarifying the comment to accurately reflect that token refresh failures always increment failures_total, independent of ignore_errors.
|
|
||
| # Test 9: Expired token is automatically refreshed and query retried successfully | ||
| print("\n*** Test 9: Expired token is automatically refreshed and query retried successfully ***") | ||
| api = OctopusAPI(my_predbat, key="test-key", account_id="test-account", automatic=False) |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test 7c's print statement says "ignore_errors=True suppresses failure counting when token refresh fails" but this is inaccurate - the test actually verifies that failures_total DOES increment (line 430 expects initial_failures + 1). The print message contradicts the test expectation. Consider changing the message to something like "failures_total increments on token refresh failure regardless of ignore_errors".
| } | ||
|
|
||
| # Call update - should not override existing data when API fails | ||
| result = await api.async_update_intelligent_device("test-account") |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to 'result' is unnecessary as it is redefined before this value is used.
| result = await api.async_update_intelligent_device("test-account") | |
| await api.async_update_intelligent_device("test-account") |
|
|
||
| if api.failures_total != initial_failures + 1: | ||
| print(f"ERROR: failures_total not incremented. Expected {initial_failures + 1}, got {api.failures_total}") | ||
| failed = True |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to 'result' is unnecessary as it is redefined before this value is used.
|
|
||
| if api.failures_total != initial_failures + 1: | ||
| print(f"ERROR: failures_total should increment even with ignore errors (due to API key) Expected {initial_failures + 1}, got {api.failures_total}") | ||
| failed = True |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to 'result' is unnecessary as it is redefined before this value is used.
| import asyncio | ||
| import json | ||
| from unittest.mock import AsyncMock, MagicMock | ||
| from octopus import OctopusAPI |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'integration_context_header' is not used.
…ll2008#3105) * Fix Octopus API rate limiting and auth error retry handling - Add rate limit error detection (KT-CT-1199) with immediate return - Implement automatic token refresh and retry for auth errors (KT-CT-1139/1111/1143) - Preserve existing data when API calls fail due to rate limiting - Return full response from async_read_response except for rate limits - Handle auth errors in async_graphql_query with single retry after token refresh - Add comprehensive test suite (test_octopus_rate_limit.py) with 11 test scenarios - Update test_octopus_read_response.py to match new error handling behavior - Fix test cleanup: restore mocked functions to prevent test interference - Clear dummy_items in fetch_octopus_rates test for test isolation All octopus tests now pass (13/13). * [pre-commit.ci lite] apply automatic fixes --------- Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Summary
Fixes Octopus API rate limiting error handling (KT-CT-1199) and implements automatic token refresh with retry for authentication errors (KT-CT-1139/1111/1143).
Changes
Core Functionality
async_read_responsenow detects rate limit errors (KT-CT-1199) early and returnsNoneimmediately without retryasync_graphql_querydetects auth errors (KT-CT-1139/1111/1143), forces token refresh, and retries onceasync_read_responsereturns full response data (except rate limits) to allow caller to handle errors appropriatelyImplementation Details
async_read_response()in octopus.py:Noneimmediatelyasync_graphql_query()in octopus.py:async_refresh_token()on auth error_retry_count=1to prevent infinite loopsin_refreshparameter fromasync_refresh_token()Testing
dummy_itemsfor test isolationget_state_wrapperto prevent interference between testsTest Results
✅ All 13 octopus tests pass:
Benefits