Fix: Trakt refresh token after expiry#88
Conversation
WalkthroughAdds an internal concurrency guard (TOKEN_UNDER_REFRESH) in TokenAuth within Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client A
actor Client B
participant TokenAuth
participant Config
participant TraktOAuth as Trakt OAuth
Client A->>TokenAuth: get_token()
TokenAuth->>Config: have_refresh_token()?
alt refresh token present
TokenAuth->>TokenAuth: TOKEN_UNDER_REFRESH == False?
alt not under refresh
TokenAuth->>TokenAuth: set TOKEN_UNDER_REFRESH = True
TokenAuth->>TokenAuth: validate_token()
alt token expired/near-expiry
TokenAuth->>TraktOAuth: refresh_token()
TraktOAuth-->>TokenAuth: new tokens
end
TokenAuth->>TokenAuth: set TOKEN_UNDER_REFRESH = False
TokenAuth->>Client A: return token
else already under refresh
Note right of TokenAuth: Concurrency guard prevents another refresh
TokenAuth->>Client A: return existing token / wait outcome
end
else no refresh token
TokenAuth->>Client A: return existing token
end
Client B->>TokenAuth: get_token() (concurrent)
TokenAuth->>TokenAuth: sees TOKEN_UNDER_REFRESH True -> avoids duplicate refresh
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
trakt/api.py (4)
215-231: Avoid concurrent double-refresh with a lock + double-check.Multiple parallel requests can now hit validate_token(), race into refresh_token(), and cause duplicate refreshes or invalid_grant if the refresh token rotates. Guard the refresh path with a lock and re-check expiry inside the lock.
+import threading @@ class TokenAuth(AuthBase): @@ def __init__(self, client: HttpClient, config: AuthConfig): super().__init__() self.config = config self.client = client # OAuth token validity checked self.OAUTH_TOKEN_VALID = None self.refresh_attempts = 0 + self._refresh_lock = threading.Lock() @@ def validate_token(self): @@ - if margin > timedelta(**self.TOKEN_REFRESH_MARGIN): + if margin > timedelta(**self.TOKEN_REFRESH_MARGIN): self.OAUTH_TOKEN_VALID = True else: - self.logger.debug("Token expires in %s, refreshing (margin: %s)", margin, self.TOKEN_REFRESH_MARGIN) - self.refresh_token() + self.logger.debug("Token expires in %s, refreshing (margin: %s)", margin, self.TOKEN_REFRESH_MARGIN) + # Serialize refresh to avoid duplicate refreshes under load + with self._refresh_lock: + # Double-check after acquiring the lock in case another thread already refreshed + current = datetime.now(tz=timezone.utc) + expires_at = datetime.fromtimestamp(self.config.OAUTH_EXPIRES_AT, tz=timezone.utc) + if (expires_at - current) <= timedelta(**self.TOKEN_REFRESH_MARGIN): + self.refresh_token()
240-247: Tighten the log message (not always “expired”).We refresh when within the margin too. Adjust wording to reduce confusion during debugging.
- self.logger.info("OAuth token has expired, refreshing now...") + self.logger.info("OAuth token expired or expiring soon; refreshing now...")
169-170: Consider modest retries with backoff for transient failures.MAX_RETRIES = 1 makes any blip a hard stop until restart. Suggest 2–3 attempts with small backoff/jitter. Happy to draft a looped implementation if desired.
180-183: Remove vestigialOAUTH_TOKEN_VALIDattribute
Theself.OAUTH_TOKEN_VALIDflag is only assigned intrakt/api.py(lines 181, 227, 274) and never read. Remove its declaration and related assignments to eliminate dead code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
trakt/api.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
trakt/api.py (2)
trakt/core.py (1)
config(73-83)trakt/config.py (1)
have_refresh_token(23-24)
🔇 Additional comments (1)
trakt/api.py (1)
205-209: Good fix: always validate when a refresh token exists.This removes the stale OAUTH_TOKEN_VALID gate and ensures tokens are refreshed during runtime expiry windows. Change aligns with the linked issue and is backward-safe given have_refresh_token() also asserts OAUTH_EXPIRES_AT presence.
In case there's 2 simultaneous token refresh attempts
2 participants
If token expires while the app is running, the refresh is currently not occurring. This is due to
OAUTH_TOKEN_VALID, that takes the following values:Because of this, it fails to validate the stored token as it always relies on a static value, unless the app restarts.
This addresses plextracksync issue Taxel/PlexTraktSync#1546