Do not break Alexa sync when encounter bad entity (#39380)

homeassistant/components/alexa/capabilities.py

@@ -1,5 +1,6 @@
 """Alexa capabilities."""
 import logging
+from typing import List, Optional
 
 from homeassistant.components import (
     cover,
@@ -36,6 +37,7 @@ from homeassistant.const import (
     STATE_UNKNOWN,
     STATE_UNLOCKED,
 )
+from homeassistant.core import State
 import homeassistant.util.color as color_util
 import homeassistant.util.dt as dt_util
 
@@ -71,32 +73,32 @@ class AlexaCapability:
 
     supported_locales = {"en-US"}
 
-    def __init__(self, entity, instance=None):
+    def __init__(self, entity: State, instance: Optional[str] = None):
         """Initialize an Alexa capability."""
         self.entity = entity
         self.instance = instance
 
-    def name(self):
+    def name(self) -> str:
         """Return the Alexa API name of this interface."""
         raise NotImplementedError
 
     @staticmethod
-    def properties_supported():
+    def properties_supported() -> List[dict]:
         """Return what properties this entity supports."""
         return []
 
     @staticmethod
-    def properties_proactively_reported():
+    def properties_proactively_reported() -> bool:
         """Return True if properties asynchronously reported."""
         return False
 
     @staticmethod
-    def properties_retrievable():
+    def properties_retrievable() -> bool:
         """Return True if properties can be retrieved."""
         return False
 
     @staticmethod
-    def properties_non_controllable():
+    def properties_non_controllable() -> bool:
         """Return True if non controllable."""
         return None
 
@@ -237,20 +239,34 @@ class AlexaCapability:
         """Return properties serialized for an API response."""
         for prop in self.properties_supported():
             prop_name = prop["name"]
-            prop_value = self.get_property(prop_name)
-            if prop_value is not None:
-                result = {
-                    "name": prop_name,
-                    "namespace": self.name(),
-                    "value": prop_value,
-                    "timeOfSample": dt_util.utcnow().strftime(DATE_FORMAT),
-                    "uncertaintyInMilliseconds": 0,
-                }
-                instance = self.instance
-                if instance is not None:
-                    result["instance"] = instance
+            try:
+                prop_value = self.get_property(prop_name)
+            except UnsupportedProperty:
+                raise
+            except Exception:  # pylint: disable=broad-except
+                _LOGGER.exception(
+                    "Unexpected error getting %s.%s property from %s",
+                    self.name(),
+                    prop_name,
+                    self.entity,
+                )
+                prop_value = None
 
-                yield result
+            if prop_value is None:
+                continue
+
+            result = {
+                "name": prop_name,
+                "namespace": self.name(),
+                "value": prop_value,
+                "timeOfSample": dt_util.utcnow().strftime(DATE_FORMAT),
+                "uncertaintyInMilliseconds": 0,
+            }
+            instance = self.instance
+            if instance is not None:
+                result["instance"] = instance
+
+            yield result
 
 
 class Alexa(AlexaCapability):

homeassistant/components/alexa/entities.py

@@ -1,6 +1,6 @@
 """Alexa entity adapters."""
 import logging
-from typing import List
+from typing import TYPE_CHECKING, List
 
 from homeassistant.components import (
     alarm_control_panel,
@@ -34,7 +34,7 @@ from homeassistant.const import (
     TEMP_CELSIUS,
     TEMP_FAHRENHEIT,
 )
-from homeassistant.core import callback
+from homeassistant.core import HomeAssistant, State, callback
 from homeassistant.helpers import network
 from homeassistant.util.decorator import Registry
 
@@ -42,6 +42,7 @@ from .capabilities import (
     Alexa,
     AlexaBrightnessController,
     AlexaCameraStreamController,
+    AlexaCapability,
     AlexaChannelController,
     AlexaColorController,
     AlexaColorTemperatureController,
@@ -72,6 +73,9 @@ from .capabilities import (
 )
 from .const import CONF_DESCRIPTION, CONF_DISPLAY_CATEGORIES
 
+if TYPE_CHECKING:
+    from .config import AbstractConfig
+
 _LOGGER = logging.getLogger(__name__)
 
 ENTITY_ADAPTERS = Registry()
@@ -203,7 +207,7 @@ class AlexaEntity:
     The API handlers should manipulate entities only through this interface.
     """
 
-    def __init__(self, hass, config, entity):
+    def __init__(self, hass: HomeAssistant, config: "AbstractConfig", entity: State):
         """Initialize Alexa Entity."""
         self.hass = hass
         self.config = config
@@ -246,13 +250,13 @@ class AlexaEntity:
         """
         raise NotImplementedError
 
-    def get_interface(self, capability):
+    def get_interface(self, capability) -> AlexaCapability:
         """Return the given AlexaInterface.
 
         Raises _UnsupportedInterface.
         """
 
-    def interfaces(self):
+    def interfaces(self) -> List[AlexaCapability]:
         """Return a list of supported interfaces.
 
         Used for discovery. The list should contain AlexaInterface instances.
@@ -280,11 +284,18 @@ class AlexaEntity:
         }
 
         locale = self.config.locale
-        capabilities = [
-            i.serialize_discovery()
-            for i in self.interfaces()
-            if locale in i.supported_locales
-        ]
+        capabilities = []
+
+        for i in self.interfaces():
+            if locale not in i.supported_locales:
+                continue
+
+            try:
+                capabilities.append(i.serialize_discovery())
+            except Exception:  # pylint: disable=broad-except
+                _LOGGER.exception(
+                    "Error serializing %s discovery for %s", i.name(), self.entity
+                )
 
         result["capabilities"] = capabilities
 

tests/components/alexa/test_capabilities.py

@@ -33,6 +33,7 @@ from . import (
     reported_properties,
 )
 
+from tests.async_mock import patch
 from tests.common import async_mock_service
 
 
@@ -756,3 +757,25 @@ async def test_report_image_processing(hass):
         "humanPresenceDetectionState",
         {"value": "DETECTED"},
     )
+
+
+async def test_get_property_blowup(hass, caplog):
+    """Test we handle a property blowing up."""
+    hass.states.async_set(
+        "climate.downstairs",
+        climate.HVAC_MODE_AUTO,
+        {
+            "friendly_name": "Climate Downstairs",
+            "supported_features": 91,
+            climate.ATTR_CURRENT_TEMPERATURE: 34,
+            ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS,
+        },
+    )
+    with patch(
+        "homeassistant.components.alexa.capabilities.float",
+        side_effect=Exception("Boom Fail"),
+    ):
+        properties = await reported_properties(hass, "climate.downstairs")
+        properties.assert_not_has_property("Alexa.ThermostatController", "temperature")
+
+    assert "Boom Fail" in caplog.text

tests/components/alexa/test_entities.py

@@ -3,6 +3,8 @@ from homeassistant.components.alexa import smart_home
 
 from . import DEFAULT_CONFIG, get_new_request
 
+from tests.async_mock import patch
+
 
 async def test_unsupported_domain(hass):
     """Discovery ignores entities of unknown domains."""
@@ -16,3 +18,29 @@ async def test_unsupported_domain(hass):
     msg = msg["event"]
 
     assert not msg["payload"]["endpoints"]
+
+
+async def test_serialize_discovery_recovers(hass, caplog):
+    """Test we handle an interface raising unexpectedly during serialize discovery."""
+    request = get_new_request("Alexa.Discovery", "Discover")
+
+    hass.states.async_set("switch.bla", "on", {"friendly_name": "Boop Woz"})
+
+    with patch(
+        "homeassistant.components.alexa.capabilities.AlexaPowerController.serialize_discovery",
+        side_effect=TypeError,
+    ):
+        msg = await smart_home.async_handle_message(hass, DEFAULT_CONFIG, request)
+
+    assert "event" in msg
+    msg = msg["event"]
+
+    interfaces = {
+        ifc["interface"] for ifc in msg["payload"]["endpoints"][0]["capabilities"]
+    }
+
+    assert "Alexa.PowerController" not in interfaces
+    assert (
+        f"Error serializing Alexa.PowerController discovery for {hass.states.get('switch.bla')}"
+        in caplog.text
+    )