Return early when setting cloud ai_task and conversation and not logged in to cloud #157402
Conversation
|
Hey there @home-assistant/cloud, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the cloud integration would throw errors when setting up ai_task and conversation entities for users not logged into Home Assistant Cloud. The changes add early return guards to prevent entity setup when users aren't logged in and expand error handling to catch NabuCasaBaseError exceptions.
Key changes:
- Added early return checks for
cloud.is_logged_instatus in bothai_taskandconversationsetup functions - Expanded exception handling to catch
NabuCasaBaseErrorin addition toLLMError - Added comprehensive test coverage for the new login check logic
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| homeassistant/components/cloud/conversation.py | Added login check and expanded exception handling to prevent setup errors |
| homeassistant/components/cloud/ai_task.py | Added login check and expanded exception handling to prevent setup errors |
| tests/components/cloud/test_conversation.py | Added test to verify early return when not logged in |
| tests/components/cloud/test_ai_task.py | Added test to verify early return when not logged in |
| await cloud.llm.async_ensure_token() | ||
| except LLMError: | ||
| except (LLMError, NabuCasaBaseError): | ||
| return |
There was a problem hiding this comment.
Side note: It doesn't seem optimal to handle these cases here in the platform setup as there are more than one platform that needs the same checks. The state of the checks can also change after platform setup, right?
I'd only have the login checks as part of the entity available and handle the missing token as part of service action calls error cases in the entity.
There was a problem hiding this comment.
At this point in time, these 2 entities are only for internal testing, and we should not expose/set up these entities for everyone.
Once it has been decided that we are moving ahead with this for a broader audience, these setup checks will be removed as they no longer serve a purpose.
There was a problem hiding this comment.
Ah, ok. Then maybe move the checks to async_setup_entry in the init module and don't set up the platforms at all if the checks fail?
| hass.data[DATA_CLOUD] = cloud | ||
|
|
||
| async_add_entities = AsyncMock() | ||
| await async_setup_entry(hass, entry, async_add_entities) |
There was a problem hiding this comment.
We don't test platform details directly like this.
Set up the config entry and assert (missing) state in the core state machine.
Proposed change
This PR fixes a bug where the cloud integration was throwing an error when setting up
ai_taskandconversation, and users were not logged in to HomeAssistant Cloud.It guards against not logged in users and handles
NabuCasaBaseErrorsType of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: