Skip to content

fix: force re-login on every update cycle instead of reusing expired token#34

Merged
smarthomeblack merged 2 commits into
mainfrom
fix/auto-update-relogin
May 12, 2026
Merged

fix: force re-login on every update cycle instead of reusing expired token#34
smarthomeblack merged 2 commits into
mainfrom
fix/auto-update-relogin

Conversation

@smarthomeblack

@smarthomeblack smarthomeblack commented May 12, 2026

Copy link
Copy Markdown
Owner

Summary

Fix auto-update not working - sensors showing stale data despite coordinator running every 10 minutes.

Root Cause

The coordinator only called login() when self.api.access_token was None:

if not self.api.access_token:    # ← Token cũ còn, skip login
    if not await self.api.login():
        raise UpdateFailed("Failed to login")

On auto-refresh, the existing API client retains the old token/session, which may have expired. Expired tokens cause API calls to silently fail (returning empty data or error state with HTTP 200 instead of 401), so no data gets saved to SQLite and sensors show stale values.

On manual reload, a fresh API client is created (access_token = None), so login() is called and gets a fresh token - which is why reload works.

Fix

Always call login() before each update cycle to ensure fresh token/session:

if not await self.api.login():
    raise UpdateFailed("Failed to login")

This matches the behavior of a manual reload.

Test Plan

  • Verify lan_cap_nhat_cuoi updates every 10 minutes as before
  • Verify sensor data (chi_so, tieu_thu, tien_dien) updates without manual reload
  • Verify works across all regions (SPC, HCMC, HN, NPC, CPC)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Always renew the API session and validate authentication during each update cycle to avoid using stale or expired sessions.
    • Fail clearly when authentication cannot be established, improving error visibility and preventing silent data inconsistencies.
    • Result: more reliable, secure data synchronization and clearer failure behavior for update operations.

Review Change Stack

…token

Previously the coordinator only called login() when access_token was None.
On auto-refresh, the existing API client retains the old token/session, which
may have expired. Expired tokens cause API calls to silently fail (returning
empty data or error state with HTTP 200 instead of 401), so no data gets
saved to SQLite and sensors show stale values.

Now login() is called on every update cycle to ensure a fresh token/session
before fetching data, matching the behavior of a manual reload.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 12, 2026 13:06
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 78ee525e-dc7b-485c-94c6-3a6545d070ef

📥 Commits

Reviewing files that changed from the base of the PR and between 364454b and 8793519.

📒 Files selected for processing (1)
  • custom_components/npc/coordinator.py

📝 Walkthrough

Walkthrough

The coordinator's data update now always closes the API session, calls api.login(), and raises UpdateFailed on authentication failure before fetching any data.

Changes

Authentication enforcement in coordinator

Layer / File(s) Summary
Unconditional login in data coordinator
custom_components/npc/coordinator.py
EVNDataUpdateCoordinator._async_update_data now closes the API session, always calls api.login() before fetching data, and raises UpdateFailed if login fails; previous conditional login based on api.access_token was removed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I close the session, tail a-flutter,
I knock and ask to meet the gate,
If login stumbles, I do stutter,
No data fetched — I mark its state.
Hop, retry, secure the crate.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately summarizes the main change: forcing re-login on every update cycle instead of reusing expired tokens, which directly addresses the auto-update coordinator stale data issue described in the objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/auto-update-relogin

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Home Assistant EVN VN coordinator to re-authenticate on every scheduled refresh to avoid stale sensor data caused by expired tokens/sessions being reused across update cycles.

Changes:

  • Always call EVNAPI.login() at the start of each _async_update_data() execution.
  • Fail the update cycle immediately if login fails (via UpdateFailed("Failed to login")).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +37 to 40
# Always re-login to ensure fresh token/session before fetching data
if not await self.api.login():
raise UpdateFailed("Failed to login")

…nnections

The root cause of auto-update not working despite 401 retry correctly
re-logging in: the cached aiohttp ClientSession retains stale TCP
connections that the server may have closed during idle time between
update cycles (10 min). When requests fail due to dead connections,
data is not saved to SQLite and sensors show old values.

On reload, a new EVNAPI creates a fresh ClientSession, which is why
reload works but auto-refresh doesn't.

Also always call login() to ensure fresh token, matching reload behavior.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@smarthomeblack smarthomeblack merged commit bf3760d into main May 12, 2026
4 checks passed
@smarthomeblack smarthomeblack deleted the fix/auto-update-relogin branch May 12, 2026 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants