Avoid crashing due to unexpected entity category#71346
Avoid crashing due to unexpected entity category#71346elupus wants to merge 1 commit intohome-assistant:devfrom
Conversation
probot-home-assistant
bot
added
bugfix
core
small-pr
|
I'm not sure why this is needed? The system category was for internal use, documented as such and never used. How would this have caused issues? |
| try: | ||
| return cls(value) | ||
| except ValueError: | ||
| _LOGGER.warning("Ignoring unexpected value %s for enum %s", value, cls) |
There was a problem hiding this comment.
I don't think we should just ignore every unknown value.
There was a problem hiding this comment.
Breaking startup of home assistant due to a single errounous value here is not a good ide.
There was a problem hiding this comment.
An alternative would be to ignore the complete entity with bad data.
There was a problem hiding this comment.
Breaking startup of home assistant due to a single errounous value here is not a good ide.
There are thousands of ways custom integrations could break the registries 🤷
Have we pinpointed the root cause/code/integration?
There was a problem hiding this comment.
Nope, we have two reported issues that i know of so far.
|
|
||
| from .entity import EntityCategory # pylint: disable=import-outside-toplevel | ||
|
|
||
| def parse_enum( |
There was a problem hiding this comment.
We should not define that as inline function, let's place that as standalone function into that file
| "entity_id": "test.no_name", | ||
| "platform": "super_platform", | ||
| "unique_id": "without-name", | ||
| "disabled_by": "ignored-invalid-disabled-by", |
There was a problem hiding this comment.
We should also test the logger message
|
Any entity registry stored that happened to have system as it's category. Could have been a custom component or anything that leaves it behind. Any time we end up removing any enums, we need to handle loading a store with enums not in the class. |
Nothing should have been using that? |
|
It was available for 2 (?) releases. Since we removed it, we should just guard for |
|
Here is a PR that just filters out system #71361 |
|
Fine by me. I'd rather see a robust solution that avoids crashing if at all possible, but that fixes the immediate issue. |
|
Since we guard the data that enters into storage, we shouldn't have to guard for any other bad data |
5 participants
Proposed change
Avoid crashing load of entity registry due to unexpected enum value for disable_by and entity_category
Type of change
Additional information
Checklist
black --fast 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..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: