Handle integrations with empty services or failing to load during service description enumeration (#95911)

* wip

* tweaks

* tweaks

* add coverage

* complain loudly as we never execpt this to happen

* ensure not None

* comment it

homeassistant/helpers/service.py

@@ -566,7 +566,9 @@ async def async_get_all_descriptions(
     hass: HomeAssistant,
 ) -> dict[str, dict[str, Any]]:
     """Return descriptions (i.e. user documentation) for all service calls."""
-    descriptions_cache = hass.data.setdefault(SERVICE_DESCRIPTION_CACHE, {})
+    descriptions_cache: dict[
+        tuple[str, str], dict[str, Any] | None
+    ] = hass.data.setdefault(SERVICE_DESCRIPTION_CACHE, {})
     services = hass.services.async_services()
 
     # See if there are new services not seen before.
@@ -574,59 +576,60 @@ async def async_get_all_descriptions(
     missing = set()
     all_services = []
     for domain in services:
-        for service in services[domain]:
+        for service_name in services[domain]:
-            cache_key = (domain, service)
+            cache_key = (domain, service_name)
             all_services.append(cache_key)
             if cache_key not in descriptions_cache:
                 missing.add(domain)
 
     # If we have a complete cache, check if it is still valid
-    if ALL_SERVICE_DESCRIPTIONS_CACHE in hass.data:
+    if all_cache := hass.data.get(ALL_SERVICE_DESCRIPTIONS_CACHE):
-        previous_all_services, previous_descriptions_cache = hass.data[
+        previous_all_services, previous_descriptions_cache = all_cache
-            ALL_SERVICE_DESCRIPTIONS_CACHE
-        ]
         # If the services are the same, we can return the cache
         if previous_all_services == all_services:
             return cast(dict[str, dict[str, Any]], previous_descriptions_cache)
 
     # Files we loaded for missing descriptions
-    loaded = {}
+    loaded: dict[str, JSON_TYPE] = {}
 
     if missing:
         ints_or_excs = await async_get_integrations(hass, missing)
-        integrations = [
+        integrations: list[Integration] = []
-            int_or_exc
+        for domain, int_or_exc in ints_or_excs.items():
-            for int_or_exc in ints_or_excs.values()
+            if type(int_or_exc) is Integration:  # pylint: disable=unidiomatic-typecheck
-            if isinstance(int_or_exc, Integration)
+                integrations.append(int_or_exc)
-        ]
+                continue
-
+            if TYPE_CHECKING:
+                assert isinstance(int_or_exc, Exception)
+            _LOGGER.error("Failed to load integration: %s", domain, exc_info=int_or_exc)
         contents = await hass.async_add_executor_job(
             _load_services_files, hass, integrations
         )
-
+        loaded = dict(zip(missing, contents))
-        for domain, content in zip(missing, contents):
-            loaded[domain] = content
 
     # Build response
     descriptions: dict[str, dict[str, Any]] = {}
-    for domain in services:
+    for domain, services_map in services.items():
         descriptions[domain] = {}
+        domain_descriptions = descriptions[domain]
 
-        for service in services[domain]:
+        for service_name in services_map:
-            cache_key = (domain, service)
+            cache_key = (domain, service_name)
             description = descriptions_cache.get(cache_key)
-
             # Cache missing descriptions
             if description is None:
-                domain_yaml = loaded[domain]
+                domain_yaml = loaded.get(domain) or {}
+                # The YAML may be empty for dynamically defined
+                # services (ie shell_command) that never call
+                # service.async_set_service_schema for the dynamic
+                # service
 
                 yaml_description = domain_yaml.get(  # type: ignore[union-attr]
-                    service, {}
+                    service_name, {}
                 )
 
                 # Don't warn for missing services, because it triggers false
                 # positives for things like scripts, that register as a service
-
                 description = {
                     "name": yaml_description.get("name", ""),
                     "description": yaml_description.get("description", ""),
@@ -637,7 +640,7 @@ async def async_get_all_descriptions(
                     description["target"] = yaml_description["target"]
 
                 if (
-                    response := hass.services.supports_response(domain, service)
+                    response := hass.services.supports_response(domain, service_name)
                 ) != SupportsResponse.NONE:
                     description["response"] = {
                         "optional": response == SupportsResponse.OPTIONAL,
@@ -645,7 +648,7 @@ async def async_get_all_descriptions(
 
                 descriptions_cache[cache_key] = description
 
-            descriptions[domain][service] = description
+            domain_descriptions[service_name] = description
 
     hass.data[ALL_SERVICE_DESCRIPTIONS_CACHE] = (all_services, descriptions)
     return descriptions
@@ -667,7 +670,9 @@ def async_set_service_schema(
     hass: HomeAssistant, domain: str, service: str, schema: dict[str, Any]
 ) -> None:
     """Register a description for a service."""
-    hass.data.setdefault(SERVICE_DESCRIPTION_CACHE, {})
+    descriptions_cache: dict[
+        tuple[str, str], dict[str, Any] | None
+    ] = hass.data.setdefault(SERVICE_DESCRIPTION_CACHE, {})
 
     description = {
         "name": schema.get("name", ""),
@@ -686,7 +691,7 @@ def async_set_service_schema(
         }
 
     hass.data.pop(ALL_SERVICE_DESCRIPTIONS_CACHE, None)
-    hass.data[SERVICE_DESCRIPTION_CACHE][(domain, service)] = description
+    descriptions_cache[(domain, service)] = description
 
 
 @bind_hass

tests/helpers/test_service.py

@@ -622,6 +622,108 @@ async def test_async_get_all_descriptions(hass: HomeAssistant) -> None:
     assert await service.async_get_all_descriptions(hass) is descriptions
 
 
+async def test_async_get_all_descriptions_failing_integration(
+    hass: HomeAssistant, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Test async_get_all_descriptions when async_get_integrations returns an exception."""
+    group = hass.components.group
+    group_config = {group.DOMAIN: {}}
+    await async_setup_component(hass, group.DOMAIN, group_config)
+    descriptions = await service.async_get_all_descriptions(hass)
+
+    assert len(descriptions) == 1
+
+    assert "description" in descriptions["group"]["reload"]
+    assert "fields" in descriptions["group"]["reload"]
+
+    logger = hass.components.logger
+    logger_config = {logger.DOMAIN: {}}
+    await async_setup_component(hass, logger.DOMAIN, logger_config)
+    with patch(
+        "homeassistant.helpers.service.async_get_integrations",
+        return_value={"logger": ImportError},
+    ):
+        descriptions = await service.async_get_all_descriptions(hass)
+
+    assert len(descriptions) == 2
+    assert "Failed to load integration: logger" in caplog.text
+
+    # Services are empty defaults if the load fails but should
+    # not raise
+    assert descriptions[logger.DOMAIN]["set_level"] == {
+        "description": "",
+        "fields": {},
+        "name": "",
+    }
+
+    hass.services.async_register(logger.DOMAIN, "new_service", lambda x: None, None)
+    service.async_set_service_schema(
+        hass, logger.DOMAIN, "new_service", {"description": "new service"}
+    )
+    descriptions = await service.async_get_all_descriptions(hass)
+    assert "description" in descriptions[logger.DOMAIN]["new_service"]
+    assert descriptions[logger.DOMAIN]["new_service"]["description"] == "new service"
+
+    hass.services.async_register(
+        logger.DOMAIN, "another_new_service", lambda x: None, None
+    )
+    hass.services.async_register(
+        logger.DOMAIN,
+        "service_with_optional_response",
+        lambda x: None,
+        None,
+        SupportsResponse.OPTIONAL,
+    )
+    hass.services.async_register(
+        logger.DOMAIN,
+        "service_with_only_response",
+        lambda x: None,
+        None,
+        SupportsResponse.ONLY,
+    )
+
+    descriptions = await service.async_get_all_descriptions(hass)
+    assert "another_new_service" in descriptions[logger.DOMAIN]
+    assert "service_with_optional_response" in descriptions[logger.DOMAIN]
+    assert descriptions[logger.DOMAIN]["service_with_optional_response"][
+        "response"
+    ] == {"optional": True}
+    assert "service_with_only_response" in descriptions[logger.DOMAIN]
+    assert descriptions[logger.DOMAIN]["service_with_only_response"]["response"] == {
+        "optional": False
+    }
+
+    # Verify the cache returns the same object
+    assert await service.async_get_all_descriptions(hass) is descriptions
+
+
+async def test_async_get_all_descriptions_dynamically_created_services(
+    hass: HomeAssistant, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Test async_get_all_descriptions when async_get_integrations when services are dynamic."""
+    group = hass.components.group
+    group_config = {group.DOMAIN: {}}
+    await async_setup_component(hass, group.DOMAIN, group_config)
+    descriptions = await service.async_get_all_descriptions(hass)
+
+    assert len(descriptions) == 1
+
+    assert "description" in descriptions["group"]["reload"]
+    assert "fields" in descriptions["group"]["reload"]
+
+    shell_command = hass.components.shell_command
+    shell_command_config = {shell_command.DOMAIN: {"test_service": "ls /bin"}}
+    await async_setup_component(hass, shell_command.DOMAIN, shell_command_config)
+    descriptions = await service.async_get_all_descriptions(hass)
+
+    assert len(descriptions) == 2
+    assert descriptions[shell_command.DOMAIN]["test_service"] == {
+        "description": "",
+        "fields": {},
+        "name": "",
+    }
+
+
 async def test_call_with_required_features(hass: HomeAssistant, mock_entities) -> None:
     """Test service calls invoked only if entity has required features."""
     test_service_mock = AsyncMock(return_value=None)