home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.63k stars 30.78k forks source link

Service "Reload themes" (or a related core method) does not return an error if a theme is invalid #98708

Open ildar170975 opened 1 year ago

ildar170975 commented 1 year ago

The problem

Originally reported for Frontend: https://github.com/home-assistant/frontend/issues/17610 Was advised to move the issue to the Core.

A detailed description is provided here: https://github.com/home-assistant/frontend/issues/17610. In short: -- when a theme is invalid, a user must be informed (an error message in "Dev tools -> Services" or in a Log); -- it should happen either when calling the "Reload themes" service or at HA startup.

Example of an invalid theme:

my_poor_theme:

i.e. w/o any variables.

What version of Home Assistant Core has the issue?

2023.8.0

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

No response

Link to integration documentation on our website

https://www.home-assistant.io/integrations/frontend/

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 1 year ago

Hey there @home-assistant/frontend, mind taking a look at this issue as it has been labeled with an integration (frontend) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `frontend` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign frontend` Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


frontend documentation frontend source (message by IssueLinks)

joostlek commented 1 year ago

https://github.com/home-assistant/core/blob/dev/homeassistant/components/frontend/__init__.py#L447

ildar170975 commented 1 year ago

@joostlek In my scenario no messages in Log.

joostlek commented 1 year ago

I just posted the link as a starting point, but the fact that you don't even get a warning makes it even stranger

joostlek commented 1 year ago

Oh wait, maybe it only does this when you don't have the theme at all (so at this point it just checks if the name is present, not if the theme is valid)

ildar170975 commented 1 year ago

Well, we need to either consider "empty" themes as invalid (with dispalying an error in a Log & Devtools) or consider "empty" themes as valid (as a user see no harm for this). Usecase: a person playing with a custom theme, adding/editing/removing vars (or card-mod code), and an empty theme (i.e. w/o vars) is harmless from a user pov.

joostlek commented 1 year ago

Can you confirm that the error does get logged if you want to change to a nonexistent theme?

ildar170975 commented 1 year ago

Never selected a theme via yaml, only from UI (where a selector contains only existing themes). So created a card and selected an existing theme: image Then selected a non-existing theme: image No messages in Log.

joostlek commented 1 year ago

I meant via the service call

Screenshot_20230820-164433.png

Screenshot_20230820-164428.png

karwosts commented 1 year ago

@joostlek - curious why you're looking at set_theme code?

This issue is related to reload_themes service, not set_theme.

reload_themes should return an error when the theme yaml is invalid. Instead it runs and leaves the themes in a bad state.

joostlek commented 1 year ago

Oh good point, I oversaw that part.

ildar170975 commented 1 year ago

Well, the "call service" became green (like everything is OK), but there is an error in Log:

023-08-20 17:46:20.344 WARNING (MainThread) [homeassistant.components.frontend] Theme xyz not found

image

ildar170975 commented 1 year ago

Anyway , imho these are diff things: -- missing theme is found in a code -- a theme is empty (and I tend to think this should not be an error)

joostlek commented 1 year ago

Yea, I misread it, sorry

issue-triage-workflows[bot] commented 12 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

karwosts commented 11 months ago

still valid

ildar170975 commented 9 months ago

2024.1.1 - still there

issue-triage-workflows[bot] commented 6 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

ildar170975 commented 6 months ago

up

issue-triage-workflows[bot] commented 3 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

ildar170975 commented 3 months ago

up

issue-triage-workflows[bot] commented 3 weeks ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

ildar170975 commented 2 weeks ago

up