Refresh token before execute - #448
Conversation
When a refresh_access_token callback is configured, call it before each HTTP request in execute() so that an expiring token is renewed proactively rather than waiting for the watchdog reconnect cycle. Rename _refresh_access_token_for_reconnect to _ensure_fresh_access_token to reflect its broader role. Behaviour is unchanged for callers that do not supply the callback. Add tests for the refresh, no-callback, and None-return cases.
🤖 Augment PR SummarySummary: This PR proactively refreshes a configured access token before GraphQL execution.
🤖 Was this summary useful? React with 👍 or 👎 |
There was a problem hiding this comment.
Review completed. 2 suggestions posted.
Items Reviewed
- ✅ Review PR #448
Comment augment review to trigger a new review at any time.
| timeout = timeout or self.timeout | ||
|
|
||
| if self._refresh_access_token is not None: | ||
| await self._ensure_fresh_access_token() |
There was a problem hiding this comment.
tibber/__init__.py:136: When this callback returns a new token, _ensure_fresh_access_token() updates only Tibber._access_token and data_api; realtime continues holding the old transport token. An active subscription can then expire and gql retries will keep sending the stale token, causing a realtime outage until the watchdog runs.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
This is out of scope for this PR, since by design we have a separate flow for the realtime refresh access token during reconnect.
It's a good point though, and I think we can look at consolidating the token refresh and token state but that needs to be done separately, and carefully.
I'll mark this as draft for now, and see if we can do something better.
|
|
||
| async def _refresh_access_token_for_reconnect(self) -> str | None: | ||
| """Refresh access token before reconnecting realtime subscriptions.""" | ||
| async def _ensure_fresh_access_token(self) -> str | None: |
There was a problem hiding this comment.
tibber/__init__.py:94: This helper is now used by normal GraphQL execution, but the public Tibber.__init__ documentation still says refresh_access_token runs only before reconnecting. Callers can therefore miss that it is invoked before each request and that callback failures now abort ordinary API operations.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Proactively refresh the access token before every reqest, if possible and needed.