fix(connect): token refresh lifecycle, backoff and error-taxonomy follow-ups - #2140
Open
g-despot wants to merge 3 commits into
Open
fix(connect): token refresh lifecycle, backoff and error-taxonomy follow-ups#2140g-despot wants to merge 3 commits into
g-despot wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Infrastructure as Code | View in Orca | ||
| SAST | View in Orca | ||
| Secrets | View in Orca | ||
| Vulnerabilities | View in Orca |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2140 +/- ##
==========================================
+ Coverage 86.64% 88.67% +2.02%
==========================================
Files 300 303 +3
Lines 23172 23489 +317
==========================================
+ Hits 20077 20828 +751
+ Misses 3095 2661 -434 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…a task in the async client - close() now stops the background token refresher: the shutdown event was created but never set, so the sync daemon thread kept refreshing after close() - the sync thread waits on that event instead of time.sleep, so close() ends it promptly instead of up to expires_in-30 s later - each refresher captures its own shutdown event: after close()+connect() the old thread used to re-read the attribute, pick up the new (unset) event and keep refreshing next to the new thread - the async client refreshes on an asyncio task instead of a daemon thread plus an event-loop sidecar thread (threads cannot start under WASM/Pyodide); close() cancels and awaits it - any refresh failure is caught and retried after 1 s, not only httpx.HTTPError: an authlib OAuthError (e.g. invalid_grant) used to kill the sync thread silently Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GUNU7QgDr9MmFZnjKY9zFN
g-despot
force-pushed
the
fix/token-refresh-lifecycle
branch
from
August 21, 2026 13:26
0b58414 to
21dc4b5
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Improves OIDC token refresher lifecycle management for synchronous and asynchronous clients.
Changes:
- Adds cancellable async token-refresh tasks.
- Signals sync refresh threads during shutdown and retries broader refresh failures.
- Adds lifecycle, retry, and reconnect tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
weaviate/connect/v4.py |
Revises token refresh and shutdown handling. |
mock_tests/test_auth.py |
Adds refresh lifecycle and failure tests. |
💡 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>
…nt per refresh round - the first wait used the full expires_in while every later wait subtracts the 30 s safety window; both colours now start with expires_in - 30 (clamped to 1 s by the loops) - the sync refresh loop binds self._client once per round, like the async task, so a refresh that is still running when close()/connect() replaces the client finishes against the client it started with; the three inner helper functions are folded into the loop - simpler comments Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GUNU7QgDr9MmFZnjKY9zFN
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.
Why
Split out of #2056 (the Pyodide/WASM PR). Fixes for the OIDC token refresher. They affect every platform, so they get their own review. #2142 (the trimmed WASM PR) is based on this branch.
What changes
close()now stops the background token refresher. Before, the stop flag was never set, so the refresh thread kept running afterclose().close()ends it right away instead of up toexpires_in − 30s later.close()+connect()the old thread picked up the new flag and two threads were refreshing at once.close()cancels it and waits for it. (Also needed for Pyodide, where threads can't start.)httpx.HTTPErrorwas caught, so e.g. an authlibinvalid_granterror silently killed the refresh thread.close()orconnect()replaces the client finishes against the client it started with.Warnings and error messages are unchanged.
Tests (
mock_tests/test_auth.py, 5 new)close()close()ends the thread right away, and a reconnect leaves exactly one thread