fix(octopus): back off when the CDN blocks the token mint - #4849
Merged
Conversation
A 403 carrying a CDN block page is already handled for GraphQL queries, which keep their cached JWT rather than discarding it. The token mint sits behind the same CDN but had no such handling, and unlike a query it has no cached result to fall back on: once the JWT expires, every authenticated call needs a new one. The result is a lockout that cannot self-heal. One edge block near token expiry is enough - the mint is refused, so is the retry, and the component re-mints on every poll indefinitely (~50 attempts an hour observed over 26 hours, with no successes). Rates keep downloading over REST throughout, so the integration looks healthy while every dispatch, settings and saving-session query silently returns nothing. Detect the block in async_refresh_token with the existing is_edge_block_body check and back off exponentially instead, 5 minutes doubling to a 1 hour cap, cleared on the next successful mint. A block that lifts is still picked up within the hour, but a block that does not is no longer hammered. The block is also now logged as an edge block rather than as the generic "Unauthenticated request: 403", which reads like a revoked API key and sends anyone diagnosing this down the wrong path. A 403 that is not identifiable as a CDN page keeps the existing refresh-and-retry behaviour, so genuinely revoked credentials still recover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnzhfR8uDgGUtYLNpDA4ka
… off Review catch on the previous commit. The backoff guard sat after the five minute proactive-refresh check, so a token with four minutes of genuine life left was treated as unusable the moment a mint was refused: one transient CDN 403 could cost up to five minutes of queries that would otherwise have succeeded. Before the backoff existed the next poll would simply re-mint and recover, so this was a regression the block itself did not cause. Decode the expiry once and, while backing off, hand back a token that has not actually expired. An expired or undecodable token still returns None. Callers that hit a genuine auth failure clear graphql_token before re-minting, so this can only revive a token we have not been able to renew yet, never a revoked one. Also from review: - Repeat the backoff reason at most every 10 minutes while suppressed. The mint makes no request during the window and so logged nothing, leaving anyone reading a short log window unable to tell a deliberate cooldown from a bad API key - every caller only says "token refresh failed". - Clamp the doubling exponent so a long block cannot evaluate 2 ** block_count without bound. The delay was already capped; this bounds the arithmetic. - Extract the schedule into token_mint_backoff_seconds(). Tests: assert the exact backoff schedule rather than mere monotonicity (a 1, 2, 2, 2... implementation passed the old check), and add two cases the previous ones could not catch - an elapsed deadline reopening the mint with the deadline left set, so a guard that tests the field for presence rather than against the clock fails, and a near-expiry token still being served during a backoff. Both were confirmed to fail against the corresponding mutation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnzhfR8uDgGUtYLNpDA4ka
Second review pass. The previous commit only handled the backoff guard, so the call that actually met the CDN 403 still returned None even when the token it held had not expired - the fallback started one call late, for no reason. The guard would have handed that same token to the very next caller anyway. Return it from the edge-block branch too, re-reading the clock rather than reusing the timestamp sampled before the request went out. Tests: Test 14 asserted the old behaviour, so it was pinning the defect - it now requires the token from the triggering call. Adds an undecodable token during an active backoff (no provable life left, so it must not be served) and a throttle test proving the reason is restated once per interval rather than per call. Both new assertions were confirmed to fail against the corresponding mutation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnzhfR8uDgGUtYLNpDA4ka
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A few small but concrete docstring/typo and PR-description mismatches were found that should be corrected before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds CDN/WAF edge-block handling to Octopus token minting so installs don’t get stuck in an endless re-mint loop when the CDN serves a 403 block page near JWT expiry. This extends the existing is_edge_block_body behavior from GraphQL queries to async_refresh_token, introducing exponential backoff and clearer logging.
Changes:
- Add token-mint backoff constants/helpers and per-instance backoff state in
OctopusAPI. - Detect CDN/WAF 403 bodies during token mint, suppress repeated mints during a backoff window, and clear backoff on successful mint.
- Expand
async_refresh_tokentests to cover edge-block backoff behavior, suppression, cap/clear behavior, and log throttling.
File summaries
| File | Description |
|---|---|
| apps/predbat/octopus.py | Implements token-mint CDN/WAF detection plus exponential backoff + log throttling and backoff state handling. |
| apps/predbat/tests/test_octopus_refresh_token.py | Adds/extends unit tests validating backoff start/suppression/growth/cap/clear and logging behavior. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
26
to
+30
| - Test 5: Token refresh handles API failure gracefully | ||
| - Test 6: Token refresh handles timeout gracefully | ||
| - Test 7: Token expiry decoding from JWT | ||
| - Test 9: A CDN/WAF 403 on the mint backs off instead of re-minting every poll | ||
| - Test 10: No HTTP request is made at all while the mint backoff is active |
|
|
||
|
|
||
| def token_mint_backoff_seconds(block_count): | ||
| """Backoff delay in seconds for the block_count'th consecutive CDN block on a token mint. |
Comment on lines
+29
to
+35
| - Test 9: A CDN/WAF 403 on the mint backs off instead of re-minting every poll | ||
| - Test 10: No HTTP request is made at all while the mint backoff is active | ||
| - Test 11: Backoff grows per block, is capped, and a success clears it | ||
| - Test 12: A non-CDN 403 does not start a backoff | ||
| - Test 13: An elapsed backoff deadline reopens the mint without touching the state | ||
| - Test 14: A token inside the proactive-refresh window is still used while backing off | ||
| - Test 15: The backoff reason is repeated but throttled |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
async_graphql_queryalready handles a 403 that carries a CDN block page: it recognises the body viais_edge_block_bodyand keeps the cached JWT rather than discarding it and immediately re-minting.async_refresh_tokenhas no equivalent handling, and unlike a query it has no cached result to fall back on — once the JWT expires, every authenticated call needs a new one.So one edge block landing near token expiry is enough to lock an install out permanently:
It is also well hidden. REST rate downloads keep working throughout, so import/export rates stay correct and the entity keeps serving last-known device settings — while every dispatch, settings and saving-session query silently returns nothing.
And it is easy to misdiagnose: the block is logged as the generic
which reads like a revoked API key. A genuinely bad key looks quite different — HTTP 200 with a null
obtainKrakenToken, and no 403 line at all.Change
async_refresh_tokenusing the existingis_edge_block_bodycheck.A 403 that is not identifiable as a CDN page keeps the existing refresh-and-retry behaviour, so genuinely revoked credentials still recover without a restart.
Tests
Four new cases in
tests/test_octopus_refresh_token.py(Tests 9-12):python3 unit_test.py -k octopuspasses in full, including the existingoctopus_waf_blockandoctopus_refresh_tokensuites.