Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add debug more information when splitting entity ID failed #49944

Closed
wants to merge 1 commit into from
Closed

Add debug more information when splitting entity ID failed #49944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion homeassistant/core.py
View file
Open in desktop
Expand Up @@ -114,7 +114,10 @@

def split_entity_id(entity_id: str) -> list[str]:
"""Split a state entity ID into domain and object ID."""
return entity_id.split(".", 1)
try:
return entity_id.split(".", 1)
except AttributeError as err:
raise HomeAssistantError("Unexpected entity ID format", entity_id) from err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you expect to include the entity ID in the message? I don't see how the second arg is used?

Also did you verify every place where this is used to make sure no one was catching attribute error?

Copy link
Contributor

@hmmbob hmmbob May 1, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, it is adding the entity_ids. Patched my running container and got this error after a long wait:

homeassistant.exceptions.HomeAssistantError: ('Unexpected entity ID format', ['switch.grijze_lamp', 'light.window_level', 'switch.staande_lamp', 'switch.zoutlamp', 'climate.woonkamer'])

Full trace in the linked issue.

edit: it's google_assitant, I was able to reproduce. See issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you expect to include the entity ID in the message?

Yes, see, above. In example linked by @hmmbob

Also did you verify every place where this is used to make sure no one was catching attribute error?

No, but the PR can be closed as the goal of this PR was to find the issue... which has been found. It is caused by this PR:

Send only a single event per incoming Google command
#49449

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL we can just pass tuples to exceptions 馃槅



VALID_ENTITY_ID = re.compile(r"^(?!.+__)(?!_)[\da-z_]+(?<!_)\.(?!_)[\da-z_]+(?<!_)$")
Expand Down
6 changes: 6 additions & 0 deletions tests/test_core.py
View file
Open in desktop
Expand Up @@ -52,6 +52,12 @@ def test_split_entity_id():
assert ha.split_entity_id("domain.object_id") == ["domain", "object_id"]


def test_raise_split_invalid_entity_id():
"""Test split_entity_id."""
with pytest.raises(ha.HomeAssistantError, match="Unexpected entity ID format"):
assert ha.split_entity_id({"this": "is", "invalid": ["stuff", "homie"]})


def test_async_add_hass_job_schedule_callback():
"""Test that we schedule coroutines and add jobs to the job pool."""
hass = MagicMock()
Expand Down