From 8b89f35b951cab79790f21070eb19164e8efa222 Mon Sep 17 00:00:00 2001 From: Logan Rosen Date: Sun, 26 Apr 2026 20:13:04 -0400 Subject: [PATCH 1/3] Fall back to coarser cost reads on opower API errors When the Opower cost API returns an error (e.g., HTTP 500) for daily or hourly cost reads, fall back to the coarser-resolution data that was already fetched successfully, rather than re-raising the exception and crashing the coordinator. Previously, any cost read failure at the daily or hourly level would propagate up and crash the DataUpdateCoordinator. On retry, the coordinator would call async_login() again within the same 30-second TOTP window, causing the reused code to be rejected. This created a permanent reauth repair loop with no user-recoverable path. Now, if daily reads fail, the coordinator returns the monthly (BILL) data. If hourly reads fail, it returns the monthly+daily data. The monthly (BILL) handler still raises on failure since there is no coarser fallback. CostRead objects contain both consumption and cost, so returning the coarser reads preserves consumption statistics at reduced granularity rather than losing all data. Fixes #169223 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../components/opower/coordinator.py | 10 +++-- tests/components/opower/test_coordinator.py | 43 ++++++++++++++++++- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/opower/coordinator.py b/homeassistant/components/opower/coordinator.py index d53706b315c30..48f676309b17d 100644 --- a/homeassistant/components/opower/coordinator.py +++ b/homeassistant/components/opower/coordinator.py @@ -563,8 +563,9 @@ def _update_with_finer_cost_reads( account, AggregateType.DAY, start, end ) except ApiException as err: - _LOGGER.error("Error getting daily cost reads: %s", err) - raise + # Fall back to coarser monthly data rather than crashing the coordinator. + _LOGGER.warning("Error getting daily cost reads, using monthly: %s", err) + return cost_reads _LOGGER.debug("Got %s daily cost reads", len(daily_cost_reads)) _update_with_finer_cost_reads(cost_reads, daily_cost_reads) if account.read_resolution == ReadResolution.DAY: @@ -581,8 +582,9 @@ def _update_with_finer_cost_reads( account, AggregateType.HOUR, start, end ) except ApiException as err: - _LOGGER.error("Error getting hourly cost reads: %s", err) - raise + # Fall back to coarser data rather than crashing the coordinator. + _LOGGER.warning("Error getting hourly cost reads, using daily: %s", err) + return cost_reads _LOGGER.debug("Got %s hourly cost reads", len(hourly_cost_reads)) _update_with_finer_cost_reads(cost_reads, hourly_cost_reads) _LOGGER.debug("Got %s cost reads", len(cost_reads)) diff --git a/tests/components/opower/test_coordinator.py b/tests/components/opower/test_coordinator.py index 6c6624c35f763..0a06f1df52f0e 100644 --- a/tests/components/opower/test_coordinator.py +++ b/tests/components/opower/test_coordinator.py @@ -250,8 +250,6 @@ async def test_coordinator_migration( ("async_get_accounts", None), ("async_get_forecast", None), ("async_get_cost_reads", AggregateType.BILL), - ("async_get_cost_reads", AggregateType.DAY), - ("async_get_cost_reads", AggregateType.HOUR), ], ) async def test_coordinator_api_exceptions( @@ -290,6 +288,47 @@ async def side_effect(account, agg_type, start, end): await coordinator._async_update_data() +@pytest.mark.parametrize( + "failing_aggregate_type", + [AggregateType.DAY, AggregateType.HOUR], +) +async def test_coordinator_cost_reads_fallback_on_api_error( + recorder_mock: Recorder, + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_opower_api: AsyncMock, + failing_aggregate_type: AggregateType, +) -> None: + """Test that daily/hourly cost read failures fall back to coarser data.""" + coordinator = OpowerCoordinator(hass, mock_config_entry) + + # Use a single ELEC account with HOUR resolution so all read levels are attempted + account = mock_opower_api.async_get_accounts.return_value[0] + mock_opower_api.async_get_accounts.return_value = [account] + + t1 = dt_util.as_utc(datetime(2023, 1, 1, 0)) + t2 = dt_util.as_utc(datetime(2023, 1, 2, 0)) + bill_read = CostRead( + start_time=t1, end_time=t2, consumption=10.0, provided_cost=1.0 + ) + + async def side_effect( + acc: object, + agg_type: AggregateType, + start: object, + end: object, + ) -> list[CostRead]: + if agg_type == failing_aggregate_type: + raise ApiException(message="HTTP Error: 500", url="http://example.com") + return [bill_read] + + mock_opower_api.async_get_cost_reads.side_effect = side_effect + + # Should NOT raise — the coordinator should fall back to coarser data + result = await coordinator._async_update_data() + assert result is not None + + async def test_coordinator_updates_with_finer_grained_data( recorder_mock: Recorder, hass: HomeAssistant, From bc14baf0d3b791558f39eead07db03248a00fa8d Mon Sep 17 00:00:00 2001 From: Logan Rosen Date: Sun, 26 Apr 2026 20:19:31 -0400 Subject: [PATCH 2/3] Address PR review feedback - Fix hourly fallback log message to say 'using coarser reads' since cost_reads may be monthly-only if the daily call also failed - Add async_wait_recording_done after _async_update_data in fallback test to avoid flaky pending recorder writes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- homeassistant/components/opower/coordinator.py | 4 +++- tests/components/opower/test_coordinator.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/opower/coordinator.py b/homeassistant/components/opower/coordinator.py index 48f676309b17d..a55f2ae340345 100644 --- a/homeassistant/components/opower/coordinator.py +++ b/homeassistant/components/opower/coordinator.py @@ -583,7 +583,9 @@ def _update_with_finer_cost_reads( ) except ApiException as err: # Fall back to coarser data rather than crashing the coordinator. - _LOGGER.warning("Error getting hourly cost reads, using daily: %s", err) + _LOGGER.warning( + "Error getting hourly cost reads, using coarser reads: %s", err + ) return cost_reads _LOGGER.debug("Got %s hourly cost reads", len(hourly_cost_reads)) _update_with_finer_cost_reads(cost_reads, hourly_cost_reads) diff --git a/tests/components/opower/test_coordinator.py b/tests/components/opower/test_coordinator.py index 0a06f1df52f0e..3c688755f10d9 100644 --- a/tests/components/opower/test_coordinator.py +++ b/tests/components/opower/test_coordinator.py @@ -326,6 +326,7 @@ async def side_effect( # Should NOT raise — the coordinator should fall back to coarser data result = await coordinator._async_update_data() + await async_wait_recording_done(hass) assert result is not None From ace9b2dc84af65515ac3d50f4f05e6535a0da278 Mon Sep 17 00:00:00 2001 From: Logan Rosen Date: Wed, 29 Apr 2026 23:17:12 -0400 Subject: [PATCH 3/3] Fall back to usage-only reads instead of coarser data When the cost endpoint fails for daily/hourly reads, retry with usage_only=True to hit the usage-only endpoint instead. This preserves consumption data at the original granularity (daily/hourly) rather than falling back to monthly data. Some utilities like ConEd return HTTP 500 from the cost endpoint for daily/hourly aggregation but the usage-only endpoint works fine. If the usage-only endpoint also fails, fall back to the coarser data that was already fetched. --- .../components/opower/coordinator.py | 28 +++++++++++++++---- tests/components/opower/test_coordinator.py | 12 ++++++-- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/opower/coordinator.py b/homeassistant/components/opower/coordinator.py index a55f2ae340345..6d542663cdd2e 100644 --- a/homeassistant/components/opower/coordinator.py +++ b/homeassistant/components/opower/coordinator.py @@ -563,9 +563,17 @@ def _update_with_finer_cost_reads( account, AggregateType.DAY, start, end ) except ApiException as err: - # Fall back to coarser monthly data rather than crashing the coordinator. - _LOGGER.warning("Error getting daily cost reads, using monthly: %s", err) - return cost_reads + _LOGGER.warning( + "Error getting daily cost reads, falling back to usage-only: %s", + err, + ) + try: + daily_cost_reads = await self.api.async_get_cost_reads( + account, AggregateType.DAY, start, end, usage_only=True + ) + except ApiException: + _LOGGER.warning("Usage-only daily reads also failed, using monthly") + return cost_reads _LOGGER.debug("Got %s daily cost reads", len(daily_cost_reads)) _update_with_finer_cost_reads(cost_reads, daily_cost_reads) if account.read_resolution == ReadResolution.DAY: @@ -582,11 +590,19 @@ def _update_with_finer_cost_reads( account, AggregateType.HOUR, start, end ) except ApiException as err: - # Fall back to coarser data rather than crashing the coordinator. _LOGGER.warning( - "Error getting hourly cost reads, using coarser reads: %s", err + "Error getting hourly cost reads, falling back to usage-only: %s", + err, ) - return cost_reads + try: + hourly_cost_reads = await self.api.async_get_cost_reads( + account, AggregateType.HOUR, start, end, usage_only=True + ) + except ApiException: + _LOGGER.warning( + "Usage-only hourly reads also failed, using coarser reads" + ) + return cost_reads _LOGGER.debug("Got %s hourly cost reads", len(hourly_cost_reads)) _update_with_finer_cost_reads(cost_reads, hourly_cost_reads) _LOGGER.debug("Got %s cost reads", len(cost_reads)) diff --git a/tests/components/opower/test_coordinator.py b/tests/components/opower/test_coordinator.py index 3c688755f10d9..2012fcd4815f1 100644 --- a/tests/components/opower/test_coordinator.py +++ b/tests/components/opower/test_coordinator.py @@ -299,7 +299,7 @@ async def test_coordinator_cost_reads_fallback_on_api_error( mock_opower_api: AsyncMock, failing_aggregate_type: AggregateType, ) -> None: - """Test that daily/hourly cost read failures fall back to coarser data.""" + """Test that daily/hourly cost read failures fall back to usage-only reads.""" coordinator = OpowerCoordinator(hass, mock_config_entry) # Use a single ELEC account with HOUR resolution so all read levels are attempted @@ -311,20 +311,26 @@ async def test_coordinator_cost_reads_fallback_on_api_error( bill_read = CostRead( start_time=t1, end_time=t2, consumption=10.0, provided_cost=1.0 ) + usage_read = CostRead( + start_time=t1, end_time=t2, consumption=10.0, provided_cost=0.0 + ) async def side_effect( acc: object, agg_type: AggregateType, start: object, end: object, + usage_only: bool = False, ) -> list[CostRead]: - if agg_type == failing_aggregate_type: + if agg_type == failing_aggregate_type and not usage_only: raise ApiException(message="HTTP Error: 500", url="http://example.com") + if usage_only: + return [usage_read] return [bill_read] mock_opower_api.async_get_cost_reads.side_effect = side_effect - # Should NOT raise — the coordinator should fall back to coarser data + # Should NOT raise — the coordinator should fall back to usage-only reads result = await coordinator._async_update_data() await async_wait_recording_done(hass) assert result is not None