Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exception translations to ring integration #136468

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from
+98 −20
Draft

Add exception translations to ring integration #136468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions homeassistant/components/ring/camera.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from homeassistant.util import dt as dt_util

from . import RingConfigEntry
from .const import DOMAIN
from .coordinator import RingDataCoordinator
from .entity import RingDeviceT, RingEntity, exception_wrap

Expand Down Expand Up @@ -214,8 +215,13 @@ async def async_on_webrtc_candidate(
) -> None:
"""Handle a WebRTC candidate."""
if candidate.sdp_m_line_index is None:
msg = "The sdp_m_line_index is required for ring webrtc streaming"
raise HomeAssistantError(msg)
raise HomeAssistantError(
translation_domain=DOMAIN,
translation_key="sdp_m_line_index_required",
translation_placeholders={
"device": self._device.name,
},
)
await self._device.on_webrtc_candidate(
session_id, candidate.candidate, candidate.sdp_m_line_index
)
Expand Down
45 changes: 37 additions & 8 deletions homeassistant/components/ring/coordinator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
UpdateFailed,
)

from .const import SCAN_INTERVAL
from .const import DOMAIN, SCAN_INTERVAL

_LOGGER = logging.getLogger(__name__)

Expand All @@ -33,20 +33,45 @@ async def _call_api[*_Ts, _R](
hass: HomeAssistant,
target: Callable[[*_Ts], Coroutine[Any, Any, _R]],
*args: *_Ts,
msg_suffix: str = "",
func_name: str,
device_name: str | None = None,
) -> _R:
device_placeholder = {"device": device_name} if device_name else {}
translation_prefix = "device_api_" if device_name else "api_"
try:
return await target(*args)
except AuthenticationError as err:
# Raising ConfigEntryAuthFailed will cancel future updates
# and start a config flow with SOURCE_REAUTH (async_step_reauth)
raise ConfigEntryAuthFailed from err
raise ConfigEntryAuthFailed(
translation_domain=DOMAIN,
translation_key=f"{translation_prefix}authentication",
translation_placeholders={
"func": func_name,
Copy link
Member

Choose a reason for hiding this comment

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

Implementation details do not belong in user-facing error messages. If this is useful for diagnostics or troubleshooting, then log it instead.

"exc": str(err),
Comment on lines +50 to +51
Copy link
Member

Choose a reason for hiding this comment

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

I think it's OK to include the stringified exception IF it's something understandable and useful to the user and isn't already, or can't be, represented by the exception translation message itself. Can you give some examples of what might be added here?

**device_placeholder,
},
) from err
except RingTimeout as err:
raise UpdateFailed(
f"Timeout communicating with API{msg_suffix}: {err}"
translation_domain=DOMAIN,
translation_key=f"{translation_prefix}timeout",
translation_placeholders={
"func": func_name,
"exc": str(err),
**device_placeholder,
},
) from err
except RingError as err:
raise UpdateFailed(f"Error communicating with API{msg_suffix}: {err}") from err
raise UpdateFailed(
translation_domain=DOMAIN,
translation_key=f"{translation_prefix}error",
translation_placeholders={
"func": func_name,
"exc": str(err),
**device_placeholder,
},
) from err


class RingDataCoordinator(DataUpdateCoordinator[RingDevices]):
Expand All @@ -72,7 +97,9 @@ async def _async_update_data(self) -> RingDevices:
update_method: str = (
"async_update_data" if self.first_call else "async_update_devices"
)
await _call_api(self.hass, getattr(self.ring_api, update_method))
await _call_api(
self.hass, getattr(self.ring_api, update_method), func_name=update_method
)
self.first_call = False
devices: RingDevices = self.ring_api.devices()
subscribed_device_ids = set(self.async_contexts())
Expand All @@ -88,14 +115,16 @@ async def _async_update_data(self) -> RingDevices:
self.hass,
lambda device: device.async_history(limit=10),
device,
msg_suffix=f" for device {device.name}", # device_id is the mac
func_name="async_history",
device_name=device.name,
)
)
tg.create_task(
_call_api(
self.hass,
device.async_update_health_data,
msg_suffix=f" for device {device.name}",
func_name="async_update_health_data",
device_name=device.name,
)
)
except ExceptionGroup as eg:
Expand Down
26 changes: 23 additions & 3 deletions homeassistant/components/ring/entity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,34 @@ async def _wrap(self: _RingBaseEntityT, *args: _P.args, **kwargs: _P.kwargs) ->
return await async_func(self, *args, **kwargs)
except AuthenticationError as err:
self.coordinator.config_entry.async_start_reauth(self.hass)
raise HomeAssistantError(err) from err
raise HomeAssistantError(
translation_domain=DOMAIN,
translation_key="device_api_authentication",
translation_placeholders={
"func": async_func.__name__,
"exc": str(err),
"device": self._device.name,
},
) from err
except RingTimeout as err:
raise HomeAssistantError(
f"Timeout communicating with API {async_func}: {err}"
translation_domain=DOMAIN,
translation_key="device_api_timeout",
translation_placeholders={
"func": async_func.__name__,
"exc": str(err),
"device": self._device.name,
},
) from err
except RingError as err:
raise HomeAssistantError(
f"Error communicating with API{async_func}: {err}"
translation_domain=DOMAIN,
translation_key="device_api_error",
translation_placeholders={
"func": async_func.__name__,
"exc": str(err),
"device": self._device.name,
},
) from err

return _wrap
Expand Down
23 changes: 23 additions & 0 deletions homeassistant/components/ring/strings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,29 @@
}
}
},
"exceptions": {
"api_authentication": {
"message": "Authentication error communicating with Ring API - {func}: {exc}"
},
"api_timeout": {
"message": "Timeout communicating with Ring API - {func}: {exc}"
},
"api_error": {
"message": "Error communicating with Ring API - {func}: {exc}"
},
"device_api_authentication": {
"message": "Authentication error communicating with Ring API for device {device} - {func}: {exc}"
},
"device_api_timeout": {
"message": "Timeout communicating with Ring API for device {device} - {func}: {exc}"
},
"device_api_error": {
"message": "Error communicating with Ring API for device {device} - {func}: {exc}"
},
"sdp_m_line_index_required": {
"message": "The sdp_m_line_index is required for ring webrtc streaming from {device}"
}
},
"issues": {
"deprecated_entity": {
"title": "Detected deprecated {platform} entity usage",
Expand Down
2 changes: 1 addition & 1 deletion tests/components/ring/test_camera.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ async def test_camera_webrtc(
assert response
assert response.get("success") is False
assert response["error"]["code"] == "home_assistant_error"
msg = "The sdp_m_line_index is required for ring webrtc streaming"
msg = f"The sdp_m_line_index is required for ring webrtc streaming from {front_camera_mock.name}"
assert msg in response["error"].get("message")
assert msg in caplog.text
front_camera_mock.on_webrtc_candidate.assert_called_once()
Expand Down
12 changes: 6 additions & 6 deletions tests/components/ring/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ async def test_auth_failed_on_setup(
[
(
RingTimeout,
"Timeout communicating with API: ",
"Timeout communicating with Ring API - async_update_data: ",
),
(
RingError,
"Error communicating with API: ",
"Error communicating with Ring API - async_update_data: ",
),
],
ids=["timeout-error", "other-error"],
Expand Down Expand Up @@ -166,11 +166,11 @@ async def test_auth_failure_on_device_update(
[
(
RingTimeout,
"Error fetching devices data: Timeout communicating with API: ",
"Error fetching devices data: Timeout communicating with Ring API - async_update_devices: ",
),
(
RingError,
"Error fetching devices data: Error communicating with API: ",
"Error fetching devices data: Error communicating with Ring API - async_update_devices: ",
),
],
ids=["timeout-error", "other-error"],
Expand Down Expand Up @@ -205,11 +205,11 @@ async def test_error_on_global_update(
[
(
RingTimeout,
"Error fetching devices data: Timeout communicating with API for device Front: ",
"Error fetching devices data: Timeout communicating with Ring API for device Front - async_history: ",
),
(
RingError,
"Error fetching devices data: Error communicating with API for device Front: ",
"Error fetching devices data: Error communicating with Ring API for device Front - async_history: ",
),
],
ids=["timeout-error", "other-error"],
Expand Down