Repository navigation
fix(credentials): recheck expiry after advisory refresh failure - #2058
Open
Linxiushen wants to merge 2 commits into
Open
Linxiushen wants to merge 2 commits into
Linxiushen wants to merge 2 commits into
Conversation
This branch has not been deployed
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.
An advisory credential refresh can fail after the previously cached bearer has expired.
TokenCachecurrently decides whether fallback is allowed before making the refresh attempt, then returns that bearer without rechecking its expiry after the failure. A Messages request can therefore carry an expired bearer while the original refresh error is swallowed.Recheck the fallback token's expiry when the failed refresh finishes. If it has expired, preserve the provider's original exception. A still-valid token keeps the existing advisory fallback behavior, and a later successful refresh recovers normally. This does not change in-flight invalidation semantics.
The public-client regression covers the built-in
WorkloadIdentityCredentialspath: a cached token has 31 seconds remaining, the SDK retries a token-endpoint 401 once, and the retry returns 503 after expiry.respxadvances the credential clock by 16 seconds for each exchange. Each modeled exchange is below the default 30-second token timeout; their combined 32 seconds exceed the remaining 31-second validity. This is deterministic offline coverage, not a measured network latency or timeout test. Sync and async Messages clients must propagate the original 503, send no second Messages request, and recover with a fresh bearer on the following call. Additional public-client controls cover a failure just before, exactly at, and just after expiry.Validation on Windows / Python 3.10.20:
4c7b598466b88559583f98216139be8e9b34c1fcproduced 6 failed / 2 passed cases. Both built-in WIF cases swallowed the original failure and sent the expired bearer.scripts/test tests/lib/test_credentials.py -k 'TestTokenCache or advisory_failure_checks_expiry_before_sending or failed_workload_retry_does_not_send_expired_bearer or advisory_failure_serves_cached_and_mandatory_failure_raises or cancelled_leader_releases_waiters or concurrent_cold_calls_share_one_provider_call' -n0: 43 passed / 2 skipped in each of the Pydantic 2 and Pydantic 1 matrices. The two existingos.forktests do not apply on Windows.scripts/formatpassed.scripts/lintpassed Ruff and dependency-cap checks; full lint remains red because Pyright reports 229 errors / 19 warnings on both the unchanged base and this patch. All 248 diagnostic blocks, including continuations, match after normalizing checkout paths and line/column offsets; none concern the two changed files. The subsequent import-lint stage was not reached.Release note: avoid sending an expired cached bearer after an advisory credential refresh fails.
Cross-SDK review: the current TypeScript, Go, Java, C#, and PHP native cache implementations do not share this waited-failure fallback path. Their advisory paths return immediately or their blocking refresh paths propagate the provider error. Ruby has the analogous blocking fallback; the sibling port is anthropics/anthropic-sdk-ruby#276.
Prepared with AI assistance. No live Anthropic API, model, GPU, or production performance validation was used.