fix(kraken): do not treat a CDN block as a permanent auth failure - #4850
Conversation
is_edge_block_body, the CDN markers and the token-mint backoff schedule are not Octopus-specific: every Kraken-based provider (Octopus, EDF, E.ON) mints its JWT through the same CDN-fronted endpoint and can be blocked the same way. Move them to utils so the Kraken auth mixin can use them too. No behaviour change. EDGE_BLOCK_REFRESH_AFTER stays in octopus.py - it is policy for that component's query path, not a shared helper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnzhfR8uDgGUtYLNpDA4ka
The Kraken auth mixin fails in the opposite direction to the Octopus component, from the same trigger. _kraken_token_request returned None for any non-200 without reading the body or logging anything, so a CDN 403 was indistinguishable from a 500 or a rejected credential. check_and_refresh_oauth_token then set oauth_failed, which is a latch: its first line is `if self.oauth_failed: return False`, and nothing clears it outside _init_kraken_auth. So a single transient edge block at refresh time disables the provider for the lifetime of the process - silently, and with no retry - until a restart. That is not a credential problem and should not be treated as one. Read the body on a non-200 and use is_edge_block_body to tell an edge block from a real rejection. On a block: back off on the shared schedule, keep any token we still hold that has not actually expired, and leave oauth_failed alone. Do not retry immediately with primary credentials either - that is a second request into the same block. Genuine failures keep the existing behaviour: retry once with primary credentials, then latch. Other non-200s are now logged rather than swallowed; previously every failure path in _kraken_token_request returned None without a word. Also bind the three new backoff helpers onto the instance in KrakenAPI. When OAuthMixin is the class base only the auth entry points are bound, so a helper called by check_and_refresh_oauth_token has to be bound alongside it or it raises AttributeError at runtime - caught here by an existing test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnzhfR8uDgGUtYLNpDA4ka
…ng tests Second review pass. None of the seven tests added by the previous commit actually ran: run_kraken_auth_mixin_tests() carried a hardcoded list and the new tests were defined below it, so unit_test.py - which calls that runner, not pytest discovery - skipped every one. CDN detection, the non-latching behaviour, suppression, the valid-token fallback and the elapsed-deadline path could all have regressed with the suite green. Both Kraken runners now discover module-level test_* functions instead of listing them, so the next test added cannot go unrun either. Also from review: check_and_refresh_oauth_token compared the retained token against a timestamp sampled BEFORE the request. A token with ten seconds left when a fifteen second edge-blocked request started was reported as usable. Re-read the clock before answering. New coverage for the two gaps the review named: - An edge block while a refresh token is held: it must not burn the refresh token, must not recurse into the same block, and must not latch. - A real KrakenAPI driven through block, suppression and recovery. KrakenAPI binds mixin methods onto the instance one at a time, so every helper reached from a bound method has to be bound alongside it; the direct-mixin tests cannot see that. Confirmed to fail when any one binding is removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnzhfR8uDgGUtYLNpDA4ka
6baa0c2 to
0dce653
Compare
There was a problem hiding this comment.
🟡 Changes recommended
New KrakenAuthMixin warning logs use an inconsistent prefix versus existing Kraken logs, reducing operational observability and making log filtering/triage harder.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens Kraken-based provider authentication (EDF/E.ON/Octopus local auth) so a transient CDN/WAF block during token minting is treated as a temporary rate-limit condition rather than a permanent credential failure, aligning behavior with the existing OAuth mixin approach.
Changes:
- Shared edge-block detection and token-mint backoff schedule moved from
octopus.pyintoutils.pyfor reuse across Kraken-based providers. KrakenAuthMixinnow reads non-200 bodies, detects CDN/WAF blocks, backs off without latchingoauth_failed, and avoids immediate credential retries into the same block.- Tests updated/added to cover edge-block detection, backoff suppression/recovery, and to avoid silently skipping newly-added tests in Kraken suites.
File summaries
| File | Description |
|---|---|
| apps/predbat/utils.py | Adds reusable CDN/WAF block detection + token-mint backoff helpers/constants. |
| apps/predbat/octopus.py | Switches Octopus to import shared edge-block/backoff helpers from utils.py. |
| apps/predbat/kraken_auth_mixin.py | Implements edge-block-aware token minting behavior and backoff handling for local Kraken auth. |
| apps/predbat/kraken.py | Ensures newly-added backoff helper methods are bound when binding mixin entry points. |
| apps/predbat/tests/test_octopus_refresh_token.py | Updates imports to use backoff constants from utils.py. |
| apps/predbat/tests/test_kraken_auth_mixin.py | Adds edge-block/backoff behavior tests and switches runner to discovered tests. |
| apps/predbat/tests/test_kraken.py | Switches runner to discovered tests and adds regression test for bound helper methods. |
Review details
Suppressed comments (2)
apps/predbat/kraken_auth_mixin.py:62
- This warning should follow the existing Kraken log prefix convention ("Warn: Kraken: …") used throughout apps/predbat/kraken.py, so log filtering/grepping stays consistent.
self.log(f"Warn: Kraken token mint still edge/WAF blocked (block {self.token_mint_block_count}) - suppressing mints until {self.token_mint_blocked_until}")
apps/predbat/kraken_auth_mixin.py:179
- This warning message should use the established Kraken prefix ("Warn: Kraken: …") for consistency with other Kraken warnings.
self.log(f"Warn: Kraken token request failed with HTTP {response.status}")
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Problem
While fixing the Octopus token-mint lockout in #4849 I checked whether the Kraken auth mixin (EDF, E.ON, and Octopus in local API-key/email mode) had the same hole. It does — but it fails in the opposite direction from the same trigger, and that failure is arguably worse for the user.
_kraken_token_requestreturnsNonefor any non-200 without reading the body and without logging anything:So a CDN 403 is indistinguishable from a 500, a timeout, or a genuinely rejected credential.
check_and_refresh_oauth_tokenthen reaches:and
oauth_failedis a latch — the first line of the function isif self.oauth_failed: return False, and nothing clears it outside_init_kraken_auth.The result: one transient edge block at token-refresh time disables the provider for the lifetime of the process, silently, with no retry, until a restart. Rate limiting is not a credential problem and should not be treated as one.
(For contrast,
oauth_mixin.pyalready gets this right — it only latchesoauth_failedon an explicitneeds_reauth, treats everything else as transient, keeps the token it holds, and says so in the log. This change brings the Kraken mixin in line with that.)Change
refactor(octopus)— moveis_edge_block_body, the CDN markers and the token-mint backoff schedule fromoctopus.pyintoutils.py. They are not Octopus-specific: every Kraken-based provider mints its JWT through the same CDN-fronted endpoint. No behaviour change.EDGE_BLOCK_REFRESH_AFTERstays inoctopus.py— it is policy for that component's query path.fix(kraken)is_edge_block_bodyto tell an edge block from a real rejection.oauth_failedalone._kraken_token_requestreturnedNonewithout a word.KrakenAPI. WhenOAuthMixinis the class base, only the auth entry points are bound, so a helper called bycheck_and_refresh_oauth_tokenmust be bound alongside it or it raisesAttributeErrorat runtime. An existing test caught this.Tests
Seven new cases in
tests/test_kraken_auth_mixin.py:oauth_failed, and makes exactly one request (no immediate credential retry)oauth_failedVerified by mutation: reinstating the
oauth_failedlatch on an edge block fails 3 tests, and a presence-only backoff guard fails 1.unit_test.py -k krakenand-k octopusboth pass in full. The-t inverterfailure in this tree (write_and_poll_switchunpacking) reproduces identically onorigin/mainand is unrelated.