Repository: home-assistant/core
Pull request: Handle integrations with empty services or failing to load during service description enumeration




## Proposed change
<!--
  Describe the big picture of your changes here to communicate to the
  maintainers why we should accept this pull request. If it fixes a bug
  or resolves a feature request, be sure to link to that issue in the
  additional information section.
-->
fixes #95581

## Type of change
<!--
  What type of change does your PR introduce to Home Assistant?
  NOTE: Please, check only 1! box!
  If your PR requires multiple boxes to be checked, you'll most likely need to
  split it into multiple PRs. This makes things easier and faster to code review.
-->

- [ ] Dependency upgrade
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests

## Additional information
<!--
  Details are important, and help maintainers processing your PR.
  Please be sure to fill out additional details, if applicable.
-->

- This PR fixes or closes issue: fixes #
- This PR is related to issue: 
- Link to documentation pull request: 

## Checklist
<!--
  Put an `x` in the boxes that apply. You can also fill these out after
  creating the PR. If you're unsure about any of them, don't hesitate to ask.
  We're here to help! This is simply a reminder of what we are going to look
  for before merging your code.
-->

- [x] The code change is tested and works locally.
- [ ] Local tests pass. **Your PR cannot be merged unless tests pass**
- [ ] There is no commented out code in this PR.
- [ ] I have followed the [development checklist][dev-checklist]
- [ ] I have followed the [perfect PR recommendations][perfect-pr]
- [ ] The code has been formatted using Black (`black --fast homeassistant tests`)
- [x] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

- [ ] Documentation added/updated for [www.home-assistant.io][docs-repository]

If the code communicates with devices, web services, or third-party tools:

- [ ] The [manifest file][manifest-docs] has all fields filled out correctly.  
      Updated and included derived files by running: `python3 -m script.hassfest`.
- [ ] New or updated dependencies have been added to `requirements_all.txt`.  
      Updated by running `python3 -m script.gen_requirements_all`.
- [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [ ] Untested files have been added to `.coveragerc`.

<!--
  This project is very active and we have a high turnover of pull requests.

  Unfortunately, the number of incoming pull requests is higher than what our
  reviewers can review and merge so there is a long backlog of pull requests
  waiting for review. You can help here!
  
  By reviewing another pull request, you will help raise the code quality of
  that pull request and the final review will be faster. This way the general
  pace of pull request reviews will go up and your wait time will go down.
  
  When picking a pull request to review, try to choose one that hasn't yet
  been reviewed.

  Thanks for helping out!
-->

To help with the load of incoming pull requests:

- [ ] I have reviewed two other [open pull requests][prs] in this repository.

[prs]: https://github.com/home-assistant/core/pulls?q=is%3Aopen+is%3Apr+-author%3A%40me+-draft%3Atrue+-label%3Awaiting-for-upstream+sort%3Acreated-desc+review%3Anone+-status%3Afailure

<!--
  Thank you for contributing <3

  Below, some useful links you could explore:
-->
[dev-checklist]: https://developers.home-assistant.io/docs/development_checklist/
[manifest-docs]: https://developers.home-assistant.io/docs/creating_integration_manifest/
[quality-scale]: https://developers.home-assistant.io/docs/integration_quality_scale_index/
[docs-repository]: https://github.com/home-assistant/home-assistant.io
[perfect-pr]: https://developers.home-assistant.io/docs/review-process/#creating-the-perfect-pr


Problem statement:
ERROR (MainThread) [homeassistant.components.websocket_api.http.connection]
### The problem

I am experiencing a issue, where the web interface continuously displays the message "Loading data" without fully loading the dashboard. This problem is not limited to a specific device, as multiple devices on the network are affected. The issue occurs both when accessing Home Assistant through an nginx redirect and directly via IP and port.

In shared log, the KeyError is related to the "shell_command" configuration, but it has randomly changed to other components, such as "shopping_list, input_number, climate..." 

Restarting the Home Assistant service temporarily resolves the issue, although it sometimes requires multiple restarts. However, the problem resurfaces as soon as I refresh the web page, causing it to get stuck on the perpetual "Loading data" state.

Below is debug error, that appear when I try to access from any device.

I have no idea what I could try so any help would be much appreciated.

### What version of Home Assistant Core has the issue?

2023.6.3

### What was the last working version of Home Assistant Core?

unknown

### What type of installation are you running?

Home Assistant OS

### Integration causing the issue

unknown

### Link to integration documentation on our website

_No response_

### Diagnostics information

_No response_

### Example YAML snippet

_No response_

### Anything in the logs that might be useful for us?

```txt
2023-06-29 08:29:04.507 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection] [139825427603792] Error handling message: Unknown error (unknown_error) User from 10.101.0.254 (Mozilla/5.0 (Linux; Android 12; M2007J3SG Build/SKQ1.211006.001; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/114.0.5735.130 Mobile Safari/537.36 Home Assistant/2023.6.0-10170 (Android 12; M2007J3SG))
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/websocket_api/decorators.py", line 26, in _handle_async_response
    await func(hass, connection, msg)
  File "/usr/src/homeassistant/homeassistant/components/websocket_api/commands.py", line 382, in handle_get_services
    payload = await _async_get_all_descriptions_json(hass)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/websocket_api/commands.py", line 363, in _async_get_all_descriptions_json
    descriptions = await async_get_all_descriptions(hass)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 614, in async_get_all_descriptions
    domain_yaml = loaded[domain]
                  ~~~~~~^^^^^^^^
KeyError: 'shell_command'
2023-06-29 08:29:04.510 DEBUG (MainThread) [homeassistant.components.websocket_api.http.connection] [139825427603792] Sending {"id":9,"type":"result","success":false,"error":{"code":"unknown_error","message":"Unknown error"}}
2023-06-29 08:29:04.560 DEBUG (MainThread) [zeroconf] IPv6 scope_id 0 associated to the receiving interface
2023-06-29 08:29:04.560 DEBUG (MainThread) [zeroconf] Ignoring duplicate message with no unicast questions received from '::ffff:10.0.11.170':5353 [socket 14 (('::', 5353, 0, 0))] (169 ytes) as [b'\x00\x00\x00\x00\x00\x03\x00\x00\x00\x03\x00\x00\x07Android\x05local\x00\x00\xff\x00\x01\xc0\x0c\x00\xff\x00\x01,{"nm":"Mi 10T Pro","as":"[8194]","ip":"170"}\x0b_mi-connect\x04_udp\xc0\x14\x00\xff\x00\x01\xc0\x0c\x00\x01\x00\x01\x00\x00\x00x\x00\x04\n\x00\x0b\xaa\xc0\x0c\x00\x1c\x00\x01\x00\x00\x00x\x00\x10\xfe\x80\x00\x00\x00\x00\x00\x00\xcc\x8a9\x8d\xcf\xd7\xb1m\xc0%\x00!\x00\x01\x00\x00\x00x\x00\x08\x00\x00\x00\x00\xddZ\xc0\x0c']
2023-06-29 08:29:04.560 DEBUG (MainThread) [zeroconf] IPv6 scope_id 2 associated to the receiving interface
2023-06-29 08:29:04.561 DEBUG (MainThread) [zeroconf] Ignoring duplicate message with no unicast questions received from 'fe80::cc8a:398d:cfd7:b16d':5353 [socket 14 (('::', 5353, 0, 0))] (169 bytes) as [b'\x00\x00\x00\x00\x00\x03\x00\x00\x00\x03\x00\x00\x07Android\x05local\x00\x00\xff\x00\x01\xc0\x0c\x00\xff\x00\x01,{"nm":"Mi 10T Pro","as":"[8194]","ip":"170"}\x0b_mi-connect\x04_udp\xc0\x14\x00\xff\x00\x01\xc0\x0c\x00\x01\x00\x01\x00\x00\x00x\x00\x04\n\x00\x0b\xaa\xc0\x0c\x00\x1c\x00\x01\x00\x00\x00x\x00\x10\xfe\x80\x00\x00\x00\x00\x00\x00\xcc\x8a9\x8d\xcf\xd7\xb1m\xc0%\x00!\x00\x01\x00\x00\x00x\x00\x08\x00\x00\x00\x00\xddZ\xc0\x0c']
2023-06-29 08:29:04.641 DEBUG (MainThread) [httpcore] http11.send_request_headers.started request=<Request [b'GET']>
2023-06-29 08:29:04.642 DEBUG (MainThread) [httpcore] http11.send_request_headers.complete
2023-06-29 08:29:04.642 DEBUG (MainThread) [httpcore] http11.send_request_body.started request=<Request [b'GET']>
2023-06-29 08:29:04.642 DEBUG (MainThread) [httpcore] http11.send_request_body.complete
2023-06-29 08:29:04.642 DEBUG (MainThread) [httpcore] http11.receive_response_headers.started request=<Request [b'GET']>
2023-06-29 08:29:04.646 DEBUG (MainThread) [httpcore] http11.receive_response_headers.complete return_value=(b'HTTP/1.1', 200, b'OK', [(b'Cache-Control', b'public, max-age=2678400'), (b'Content-Type', b'image/jpeg'), (b'Etag', b'"176d0a446b8e47b8-4086"'), (b'Last-Modified', b'Thu, 29 Jun 2023 05:32:05 GMT'), (b'Content-Length', b'16518'), (b'Accept-Ranges', b'bytes'), (b'Date', b'Thu, 29 Jun 2023 06:29:04 GMT'), (b'Server', b'Python/3.11 aiohttp/3.8.4')])
2023-06-29 08:29:04.647 DEBUG (MainThread) [httpcore] http11.receive_response_body.started request=<Request [b'GET']>
2023-06-29 08:29:04.647 DEBUG (MainThread) [httpcore] http11.receive_response_body.complete
2023-06-29 08:29:04.647 DEBUG (MainThread) [httpcore] http11.response_closed.started
2023-06-29 08:29:04.649 DEBUG (MainThread) [httpcore] http11.response_closed.complete
```


### Additional information

_No response_

Hints from issue discussion:
This probably is due to an external service requesting service descriptions over the API. One service which can do that is the remote home-assistant integration, do you use that?

I have the same problem, as soon as my remote instance of HA sends a request to my 'main' HA for the service descriptions, the API call generates an error and afterwards, HA freezes.

This issue is (I think) due to this change https://github.com/home-assistant/core/pull/93131. This PR is merged in 2023.6.x which does also align with my observation that everything works fine on versions < 2023.6.0. @bdraco can you confirm if this is related to the change you implemented?
Yes, I use remote home-assistant integration, and I have two hass instances connected together. I can try downgrading to a version earlier than 2023.6.0, but I'm fairly certain that's around the time when the problems started. Do you have any suggestions on what we can do to resolve this?
I added the PR in which it changed and notified the owner of the PR if this could be related. Hope that he might have a thought. I will try to debug the issue as soon as I have time and come with code changes to fix it but can't guarantee that that will be in the upcoming days. I really want to upgrade to 2023.7 because of the new interesting features but this bug avoids that and that is a pity... 
Traveling today but will take a look at this one when I can

@Wesley-Vos 
Please post the backtrace you see.
Looking at the code, the only way I can see this happening is if fetching the integration returned an exception. 

We currently don't log that.

We could refactor this to log the exception and exclude that integration from the load

The real question is why would the integration be failing to load if there are services for it registered

Commit message: add coverage

Patch to review:
diff --git a/homeassistant/helpers/service.py b/homeassistant/helpers/service.py
index 715a960de5dc0..90d3668b9f4fa 100644
--- a/homeassistant/helpers/service.py
+++ b/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,23 +576,21 @@ async def async_get_all_descriptions(
     missing = set()
     all_services = []
     for domain in services:
-        for service in services[domain]:
-            cache_key = (domain, service)
+        for service_name in services[domain]:
+            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:
-        previous_all_services, previous_descriptions_cache = hass.data[
-            ALL_SERVICE_DESCRIPTIONS_CACHE
-        ]
+    if all_cache := hass.data.get(ALL_SERVICE_DESCRIPTIONS_CACHE):
+        previous_all_services, previous_descriptions_cache = all_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)
@@ -599,34 +599,28 @@ async def async_get_all_descriptions(
             for int_or_exc in ints_or_excs.values()
             if isinstance(int_or_exc, Integration)
         ]
-
         contents = await hass.async_add_executor_job(
             _load_services_files, hass, integrations
         )
-
-        for domain, content in zip(missing, contents):
-            loaded[domain] = content
+        loaded = dict(zip(missing, contents))
 
     # 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]:
-            cache_key = (domain, service)
+        for service_name in services_map:
+            cache_key = (domain, service_name)
             description = descriptions_cache.get(cache_key)
-
             # Cache missing descriptions
-            if description is None:
-                domain_yaml = loaded[domain]
-
+            if description is None and (domain_yaml := loaded.get(domain)):
                 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 +631,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 +639,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 +661,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", ""),
@@ -679,7 +675,7 @@ def async_set_service_schema(
         description["target"] = schema["target"]
 
     hass.data.pop(ALL_SERVICE_DESCRIPTIONS_CACHE, None)
-    hass.data[SERVICE_DESCRIPTION_CACHE][(domain, service)] = description
+    descriptions_cache[(domain, service)] = description
 
 
 @bind_hass
diff --git a/tests/helpers/test_service.py b/tests/helpers/test_service.py
index 291a1744d2097..652f2f8ae78c0 100644
--- a/tests/helpers/test_service.py
+++ b/tests/helpers/test_service.py
@@ -605,6 +605,76 @@ 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,
+) -> 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
+
+    # Services are None if the load fails but should
+    # not raise
+    assert descriptions[logger.DOMAIN]["set_level"] is None
+
+    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_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)


Code review comments:
diff_hunk: @@ -599,34 +599,28 @@ async def async_get_all_descriptions(
             for int_or_exc in ints_or_excs.values()
path: homeassistant/helpers/service.py
text: @author:
We should warn about the failure here instead of swallowing it