Ensure that the ring integration always raises HomeAssistantError for user actions (#109893)

* Wrap library exceptions in HomeAssistantErrors

* Remove commented

* Update post review

* Update post second review
This commit is contained in:
Steven B 2024-03-11 19:23:49 +00:00 committed by GitHub
parent 5e94858821
commit eff0aac586
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 189 additions and 35 deletions

View File

@ -8,7 +8,6 @@ import logging
from typing import Optional
from haffmpeg.camera import CameraMjpeg
import requests
from homeassistant.components import ffmpeg
from homeassistant.components.camera import Camera
@ -20,7 +19,7 @@ from homeassistant.util import dt as dt_util
from .const import DOMAIN, RING_DEVICES, RING_DEVICES_COORDINATOR
from .coordinator import RingDataCoordinator
from .entity import RingEntity
from .entity import RingEntity, exception_wrap
FORCE_REFRESH_INTERVAL = timedelta(minutes=3)
@ -145,17 +144,11 @@ class RingCam(RingEntity, Camera):
if self._last_video_id != self._last_event["id"]:
self._image = None
try:
video_url = await self.hass.async_add_executor_job(
self._device.recording_url, self._last_event["id"]
)
except requests.Timeout:
_LOGGER.warning(
"Time out fetching recording url for camera %s", self.entity_id
)
video_url = None
self._video_url = await self.hass.async_add_executor_job(self._get_video)
if video_url:
self._last_video_id = self._last_event["id"]
self._video_url = video_url
self._expires_at = FORCE_REFRESH_INTERVAL + utcnow
self._last_video_id = self._last_event["id"]
self._expires_at = FORCE_REFRESH_INTERVAL + utcnow
@exception_wrap
def _get_video(self) -> str:
return self._device.recording_url(self._last_event["id"])

View File

@ -1,10 +1,11 @@
"""Base class for Ring entity."""
from collections.abc import Callable
from typing import Any, Concatenate, ParamSpec, TypeVar
from typing import TypeVar
from ring_doorbell.generic import RingGeneric
import ring_doorbell
from homeassistant.core import callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.device_registry import DeviceInfo
from homeassistant.helpers.update_coordinator import CoordinatorEntity
@ -19,6 +20,33 @@ _RingCoordinatorT = TypeVar(
"_RingCoordinatorT",
bound=(RingDataCoordinator | RingNotificationsCoordinator),
)
_T = TypeVar("_T", bound="RingEntity")
_P = ParamSpec("_P")
def exception_wrap(
func: Callable[Concatenate[_T, _P], Any],
) -> Callable[Concatenate[_T, _P], Any]:
"""Define a wrapper to catch exceptions and raise HomeAssistant errors."""
def _wrap(self: _T, *args: _P.args, **kwargs: _P.kwargs) -> None:
try:
return func(self, *args, **kwargs)
except ring_doorbell.AuthenticationError as err:
self.hass.loop.call_soon_threadsafe(
self.coordinator.config_entry.async_start_reauth, self.hass
)
raise HomeAssistantError(err) from err
except ring_doorbell.RingTimeout as err:
raise HomeAssistantError(
f"Timeout communicating with API {func}: {err}"
) from err
except ring_doorbell.RingError as err:
raise HomeAssistantError(
f"Error communicating with API{func}: {err}"
) from err
return _wrap
class RingEntity(CoordinatorEntity[_RingCoordinatorT]):
@ -30,7 +58,7 @@ class RingEntity(CoordinatorEntity[_RingCoordinatorT]):
def __init__(
self,
device: RingGeneric,
device: ring_doorbell.RingGeneric,
coordinator: _RingCoordinatorT,
) -> None:
"""Initialize a sensor for Ring device."""
@ -51,7 +79,7 @@ class RingEntity(CoordinatorEntity[_RingCoordinatorT]):
return device_data
return None
def _get_coordinator_device(self) -> RingGeneric | None:
def _get_coordinator_device(self) -> ring_doorbell.RingGeneric | None:
if (device_data := self._get_coordinator_device_data()) and (
device := device_data.device
):

View File

@ -4,7 +4,6 @@ from datetime import timedelta
import logging
from typing import Any
import requests
from ring_doorbell import RingStickUpCam
from ring_doorbell.generic import RingGeneric
@ -16,7 +15,7 @@ import homeassistant.util.dt as dt_util
from .const import DOMAIN, RING_DEVICES, RING_DEVICES_COORDINATOR
from .coordinator import RingDataCoordinator
from .entity import RingEntity
from .entity import RingEntity, exception_wrap
_LOGGER = logging.getLogger(__name__)
@ -76,13 +75,10 @@ class RingLight(RingEntity, LightEntity):
self._attr_is_on = device.lights == ON_STATE
super()._handle_coordinator_update()
@exception_wrap
def _set_light(self, new_state):
"""Update light state, and causes Home Assistant to correctly update."""
try:
self._device.lights = new_state
except requests.Timeout:
_LOGGER.error("Time out setting %s light to %s", self.entity_id, new_state)
return
self._device.lights = new_state
self._attr_is_on = new_state == ON_STATE
self._no_updates_until = dt_util.utcnow() + SKIP_UPDATES_DELAY

View File

@ -13,7 +13,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
from .const import DOMAIN, RING_DEVICES, RING_DEVICES_COORDINATOR
from .coordinator import RingDataCoordinator
from .entity import RingEntity
from .entity import RingEntity, exception_wrap
_LOGGER = logging.getLogger(__name__)
@ -49,6 +49,7 @@ class RingChimeSiren(RingEntity, SirenEntity):
# Entity class attributes
self._attr_unique_id = f"{self._device.id}-siren"
@exception_wrap
def turn_on(self, **kwargs: Any) -> None:
"""Play the test sound on a Ring Chime device."""
tone = kwargs.get(ATTR_TONE) or KIND_DING

View File

@ -4,7 +4,6 @@ from datetime import timedelta
import logging
from typing import Any
import requests
from ring_doorbell import RingStickUpCam
from ring_doorbell.generic import RingGeneric
@ -16,7 +15,7 @@ import homeassistant.util.dt as dt_util
from .const import DOMAIN, RING_DEVICES, RING_DEVICES_COORDINATOR
from .coordinator import RingDataCoordinator
from .entity import RingEntity
from .entity import RingEntity, exception_wrap
_LOGGER = logging.getLogger(__name__)
@ -83,13 +82,10 @@ class SirenSwitch(BaseRingSwitch):
self._attr_is_on = device.siren > 0
super()._handle_coordinator_update()
@exception_wrap
def _set_switch(self, new_state):
"""Update switch state, and causes Home Assistant to correctly update."""
try:
self._device.siren = new_state
except requests.Timeout:
_LOGGER.error("Time out setting %s siren to %s", self.entity_id, new_state)
return
self._device.siren = new_state
self._attr_is_on = new_state > 0
self._no_updates_until = dt_util.utcnow() + SKIP_UPDATES_DELAY

View File

@ -1,9 +1,14 @@
"""The tests for the Ring light platform."""
from unittest.mock import PropertyMock, patch
import pytest
import requests_mock
import ring_doorbell
from homeassistant.config_entries import SOURCE_REAUTH
from homeassistant.const import Platform
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import entity_registry as er
from .common import setup_platform
@ -90,3 +95,44 @@ async def test_updates_work(
state = hass.states.get("light.front_light")
assert state.state == "on"
@pytest.mark.parametrize(
("exception_type", "reauth_expected"),
[
(ring_doorbell.AuthenticationError, True),
(ring_doorbell.RingTimeout, False),
(ring_doorbell.RingError, False),
],
ids=["Authentication", "Timeout", "Other"],
)
async def test_light_errors_when_turned_on(
hass: HomeAssistant,
requests_mock: requests_mock.Mocker,
exception_type,
reauth_expected,
) -> None:
"""Tests the light turns on correctly."""
await setup_platform(hass, Platform.LIGHT)
config_entry = hass.config_entries.async_entries("ring")[0]
assert not any(config_entry.async_get_active_flows(hass, {SOURCE_REAUTH}))
with patch.object(
ring_doorbell.RingStickUpCam, "lights", new_callable=PropertyMock
) as mock_lights:
mock_lights.side_effect = exception_type
with pytest.raises(HomeAssistantError):
await hass.services.async_call(
"light", "turn_on", {"entity_id": "light.front_light"}, blocking=True
)
await hass.async_block_till_done()
assert mock_lights.call_count == 1
assert (
any(
flow
for flow in config_entry.async_get_active_flows(hass, {SOURCE_REAUTH})
if flow["handler"] == "ring"
)
== reauth_expected
)

View File

@ -1,9 +1,14 @@
"""The tests for the Ring button platform."""
from unittest.mock import patch
import pytest
import requests_mock
import ring_doorbell
from homeassistant.config_entries import SOURCE_REAUTH
from homeassistant.const import Platform
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import entity_registry as er
from .common import setup_platform
@ -145,3 +150,46 @@ async def test_motion_chime_can_be_played(
state = hass.states.get("siren.downstairs_siren")
assert state.state == "unknown"
@pytest.mark.parametrize(
("exception_type", "reauth_expected"),
[
(ring_doorbell.AuthenticationError, True),
(ring_doorbell.RingTimeout, False),
(ring_doorbell.RingError, False),
],
ids=["Authentication", "Timeout", "Other"],
)
async def test_siren_errors_when_turned_on(
hass: HomeAssistant,
requests_mock: requests_mock.Mocker,
exception_type,
reauth_expected,
) -> None:
"""Tests the siren turns on correctly."""
await setup_platform(hass, Platform.SIREN)
config_entry = hass.config_entries.async_entries("ring")[0]
assert not any(config_entry.async_get_active_flows(hass, {SOURCE_REAUTH}))
with patch.object(
ring_doorbell.RingChime, "test_sound", side_effect=exception_type
) as mock_siren:
with pytest.raises(HomeAssistantError):
await hass.services.async_call(
"siren",
"turn_on",
{"entity_id": "siren.downstairs_siren", "tone": "motion"},
blocking=True,
)
await hass.async_block_till_done()
assert mock_siren.call_count == 1
assert (
any(
flow
for flow in config_entry.async_get_active_flows(hass, {SOURCE_REAUTH})
if flow["handler"] == "ring"
)
== reauth_expected
)

View File

@ -1,9 +1,14 @@
"""The tests for the Ring switch platform."""
from unittest.mock import PropertyMock, patch
import pytest
import requests_mock
import ring_doorbell
from homeassistant.config_entries import SOURCE_REAUTH
from homeassistant.const import ATTR_ENTITY_ID, Platform
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import entity_registry as er
from homeassistant.setup import async_setup_component
@ -97,3 +102,44 @@ async def test_updates_work(
state = hass.states.get("switch.front_siren")
assert state.state == "on"
@pytest.mark.parametrize(
("exception_type", "reauth_expected"),
[
(ring_doorbell.AuthenticationError, True),
(ring_doorbell.RingTimeout, False),
(ring_doorbell.RingError, False),
],
ids=["Authentication", "Timeout", "Other"],
)
async def test_switch_errors_when_turned_on(
hass: HomeAssistant,
requests_mock: requests_mock.Mocker,
exception_type,
reauth_expected,
) -> None:
"""Tests the switch turns on correctly."""
await setup_platform(hass, Platform.SWITCH)
config_entry = hass.config_entries.async_entries("ring")[0]
assert not any(config_entry.async_get_active_flows(hass, {SOURCE_REAUTH}))
with patch.object(
ring_doorbell.RingStickUpCam, "siren", new_callable=PropertyMock
) as mock_switch:
mock_switch.side_effect = exception_type
with pytest.raises(HomeAssistantError):
await hass.services.async_call(
"switch", "turn_on", {"entity_id": "switch.front_siren"}, blocking=True
)
await hass.async_block_till_done()
assert mock_switch.call_count == 1
assert (
any(
flow
for flow in config_entry.async_get_active_flows(hass, {SOURCE_REAUTH})
if flow["handler"] == "ring"
)
== reauth_expected
)