fix: survive a transient HA failure in the charging-power tick (#643)

[johanzander]

Summary

• The 5-minute adjust_charging_power cron tick now survives a transient Home Assistant outage instead of crashing and being silently skipped.

Root cause

grid_charge_enabled() reads through _api_request, which re-raises requests.RequestException once its retries are exhausted (only ValueError is caught inside the method). The caller adjust_charging_power() had an except clause naming only AttributeError/ValueError/KeyError, so a Supervisor 502 on Check grid charge state escaped to APScheduler and killed the tick. A real outage hits a second read too: get_charging_power_rate() already degrades a RequestException to None, and that None reached abs(target_power - current_power) as a TypeError, escaping the same tick.

Fix

Two narrow changes that match the graceful-degradation contract already used by _get_raw_state() and every other sensor-read getter in ha_api_controller.py:

• adjust_charging_power() catches requests.RequestException alongside the existing tuple — a transient HA failure logs and skips the tick, and the inverter keeps its current rate.
• adjust_battery_charging() logs and returns early when the charge-rate read degrades to None, instead of computing on it.

Deliberately not fixed by catching inside grid_charge_enabled() and returning False (as the Stage 2 diagnosis suggested): False selects the "solar-only, no fuse risk" branch in power_monitor.py, which would grant full target charging power precisely when grid charging can't be confirmed off.

Test plan

• ./scripts/quality-check.sh passes locally (0 errors, mypy ratchet clean)
• .venv/bin/pytest -m slow passes (554 passed, 8 skipped)
• Local run & observe — real HTTP, real code. Stood up a real HTTP server returning 502 for the grid-charge entity and drove the real adjust_charging_power() through a real HomeAssistantAPIController:
  • Route 1 (the issue's exact failure): 502 on Check grid charge state → 4 real retries (2s/4s/8s backoff) → failure tracker records [sensor_read]: Check grid charge state → Failed to adjust charging power: 502 Server Error... logged → tick survives.
  • Route 2: charge-rate read returns an empty body → _get_raw_state degrades it to None → Charging power rate unreadable; leaving the inverter rate unchanged logged → tick survives.

Evidence the test discriminates

• Reverted: requests.RequestException added to the except tuple in adjust_charging_power()
• Result: test_transient_ha_failure_does_not_crash_adjust_charging_power FAILED — requests.exceptions.RequestException: 502 Server Error... escaped (1 failed)
• Reverted: the current_power is None early-return in adjust_battery_charging()
• Result: test_unreadable_charge_rate_does_not_crash_adjust_charging_power FAILED — TypeError: unsupported operand type(s) for -: 'int' and 'NoneType' (1 failed)
• Restored: tree clean, both pass

Outcome-level coverage

The two new tests are call-level by design, and each docstring states why: the outcome under test is whether the scheduled tick survives the exception, and there is no execution-model axis for an HA outage mid-tick (the inverter simulator has no HA entity-failure input). No expected_results/golden pin applies — this change is pure resilience; the optimizer's economics and the rate-mapping math are byte-identical.

Documentation check

Neither docs/agents/bess-knowledge.md nor docs/SOFTWARE_DESIGN.md describes the exception-handling path this fix changes. Both mention adjust_charging_power only for its purpose (fuse-limited charging) and a known cosmetic-setting limitation — neither invalidated.

Refs #643

[bess-agent]

@claude-bot review

[bess-manager-claude-bot]

Review: PR #675 (fix/issue-643-grid-charge-read)

Root-cause match: Confirmed against Stage 2's analysis and the source. grid_charge_enabled() (ha_api_controller.py:1630-1652) only catches ValueError, so a RequestException from _api_request() (retries exhausted) propagates through adjust_battery_charging() into adjust_charging_power()'s except (AttributeError, ValueError, KeyError) (battery_system_manager.py:3467, pre-fix) — unguarded, killing the APScheduler tick. Verified get_charging_power_rate() → _get_sensor_value → _get_raw_state (ha_api_controller.py:1413-1417) already degrades RequestException to None, and that None reaches abs(target_power - current_power) (power_monitor.py:264 pre-fix) as an unguarded TypeError — a second, independent crash path from the same outage class. Both match the PR description exactly.

Good deviation from Stage 2, well justified: Stage 2 proposed catching inside grid_charge_enabled() and returning False. The PR correctly rejects this — False there means "grid charging confirmed off," which routes power_monitor.py's if not self.controller.grid_charge_enabled(): into the solar-only, no-fuse-risk branch and grants full target charging power precisely when the state is actually unknown, not off. That's a real correctness bug the proposed fix would have introduced. Catching at the adjust_charging_power() call site instead (skip the tick, keep current rate) is the safer contract and fits that method's existing swallow-and-log responsibility — this is a local fix, not a structural one, and the PR body states the scope assessment clearly per rules.md step 9.

Minimal, no scope creep: Two narrow changes — one exception class added to an existing tuple, one None guard with early return — both fitting their target methods' existing contracts. No new classes, no new exception types (correctly, requests.RequestException is a stdlib type, doesn't belong in core/bess/exceptions.py), no hardcoded entity IDs, no exception-message string matching. set_grid_charge() (write-side) and backend/api.py:1064's use of grid_charge_enabled() are correctly left untouched — out of scope, per the analysis.

Tests — would catch a regression: Both new tests follow the existing test_unavailable_phase_sensor_does_not_crash_adjust_charging_power pattern (call-level, "must not raise"), which is appropriate here since the outcome under test is tick survival and there's no execution-model axis for a mid-tick HA outage. I independently verified both are RED without their fix:

• Reverting the requests.RequestException addition to battery_system_manager.py's except tuple → test_transient_ha_failure_does_not_crash_adjust_charging_power fails with requests.exceptions.RequestException: 502 Server Error... escaping, as claimed.
• Reverting the current_power is None guard in power_monitor.py → test_unreadable_charge_rate_does_not_crash_adjust_charging_power fails with TypeError: unsupported operand type(s) for -: 'int' and 'NoneType', as claimed.
  Restored tree: both pass. Full fast suite green (pytest -m "not slow": 2227 passed, 50 skipped, 0 failed). black, ruff, and the mypy ratchet (scripts/mypy-changed.sh --include-worktree) all pass clean on the touched files.

Checklist: No Optional[x], no hasattr/getattr-with-default, no silent fallbacks beyond the explicitly-logged degrade-and-skip this fix adds by design, all sensor access already routes through ha_api_controller (unchanged), convert_keys_to_camel_case n/a (no API layer touched). CHANGELOG entry correctly added once under ## [Unreleased] → ### Fixed.

No blockers. Approving.