From 03b3e4782610f4ef5745b6aee6c28585f051cf10 Mon Sep 17 00:00:00 2001 From: Matt Dennis Date: Tue, 20 Oct 2026 21:01:44 +0100 Subject: [PATCH 1/2] fix: log skipped discovery entities with context instead of a blank ERROR add_list caught KeyError and called _LOGGER.error(error). For a trvcontrol product with no linked TRVs, get_device_data logged its own ERROR and then raised a bare KeyError, whose str() is empty, so Home Assistant showed a second, blank ERROR entry. get_device_data now raises KeyError with a message and no longer logs, and add_list logs one WARNING naming the entity type, Hive product type and id, and the missing key. Fixes home-assistant/core#172819 Co-Authored-By: Claude Opus 5.5 --- src/helper/hive_helper.py | 5 +---- src/session/discovery.py | 11 +++++++++- tests/unit/test_discovery.py | 41 ++++++++++++++++++++++++++++++++++++ tests/unit/test_helpers.py | 13 +++++++++++- 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/helper/hive_helper.py b/src/helper/hive_helper.py index 8768f1af..aa59b456 100644 --- a/src/helper/hive_helper.py +++ b/src/helper/hive_helper.py @@ -168,10 +168,7 @@ def get_device_data(self, product: dict): if trv_present: device = self.session.data.devices[product["props"]["trvs"][0]] else: - _LOGGER.error( - "get_device_data - No TRVs found for product %s", product_id - ) - raise KeyError + raise KeyError(f"no TRVs linked to trvcontrol product {product_id}") elif ( product_type == "warmwhitelight" and product["props"]["model"] == "SIREN001" ): diff --git a/src/session/discovery.py b/src/session/discovery.py index ec835c90..003366f9 100644 --- a/src/session/discovery.py +++ b/src/session/discovery.py @@ -113,7 +113,16 @@ def add_list(self, entity_type: str, data: dict, **kwargs) -> Device | None: self.device_list[entity_type].append(device_obj) return device_obj except KeyError as error: - _LOGGER.error(error) + # str(KeyError()) is empty, so name the entity and key explicitly; + # a bare _LOGGER.error(error) showed up as a blank log entry. + missing = error.args[0] if error.args else "unknown key" + _LOGGER.warning( + "Skipping %s entity for Hive %s %s: missing data (%s)", + entity_type, + data.get("type", "device"), + data.get("id", "unknown"), + missing, + ) return None @staticmethod diff --git a/tests/unit/test_discovery.py b/tests/unit/test_discovery.py index f11e3ad9..9bad3583 100644 --- a/tests/unit/test_discovery.py +++ b/tests/unit/test_discovery.py @@ -2,6 +2,7 @@ # pylint: disable=protected-access,attribute-defined-outside-init +import logging from unittest.mock import MagicMock from apyhiveapi.helper.hivedataclasses import Device, SessionConfig @@ -216,3 +217,43 @@ def test_returns_device_instance(self): data = {"id": "heat-1", "type": "heating"} device = d.add_list("climate", data) assert isinstance(device, Device) + + +# --------------------------------------------------------------------------- +# add_list error logging (home-assistant/core#172819: blank ERROR log entry) +# --------------------------------------------------------------------------- + + +class TestAddListErrorLogging: + """A skipped entity is logged with enough context to act on.""" + + def test_bare_keyerror_logs_non_blank_warning(self, caplog): + """A KeyError with no args previously produced an empty log message.""" + d = _make_discovery() + d.helper.get_device_data.side_effect = KeyError() + with caplog.at_level(logging.WARNING): + result = d.add_list("climate", {"id": "trv-ctl-1", "type": "trvcontrol"}) + assert result is None + (record,) = caplog.records + assert record.levelno == logging.WARNING + message = record.getMessage() + assert message.strip() + assert "climate" in message + assert "trvcontrol" in message + assert "trv-ctl-1" in message + assert "unknown key" in message + + def test_keyerror_message_included(self, caplog): + d = _make_discovery() + d.helper.get_device_data.side_effect = KeyError("parent-0000") + with caplog.at_level(logging.WARNING): + d.add_list("binary_sensor", {"id": "sense-1", "type": "sense"}) + assert "parent-0000" in caplog.records[0].getMessage() + + def test_data_without_id_or_type(self, caplog): + d = _make_discovery() + d.helper.get_device_data.side_effect = KeyError("state") + with caplog.at_level(logging.WARNING): + d.add_list("sensor", {}) + message = caplog.records[0].getMessage() + assert "device unknown" in message diff --git a/tests/unit/test_helpers.py b/tests/unit/test_helpers.py index 8ee6b13c..8981aff7 100644 --- a/tests/unit/test_helpers.py +++ b/tests/unit/test_helpers.py @@ -2,6 +2,7 @@ # pylint: disable=protected-access +import logging from unittest.mock import MagicMock import pytest @@ -454,9 +455,19 @@ def test_trvcontrol_no_trvs_raises_key_error(self): """trvcontrol with an empty trvs list raises KeyError.""" helper, _ = _make_helper() product = {"id": "trv-1", "type": "trvcontrol", "props": {"trvs": []}} - with pytest.raises(KeyError): + with pytest.raises( + KeyError, match="no TRVs linked to trvcontrol product trv-1" + ): helper.get_device_data(product) + def test_trvcontrol_no_trvs_does_not_log_error(self, caplog): + """The caller logs the skip once; the helper must not log a second ERROR.""" + helper, _ = _make_helper() + product = {"id": "trv-1", "type": "trvcontrol", "props": {"trvs": []}} + with caplog.at_level(logging.DEBUG), pytest.raises(KeyError): + helper.get_device_data(product) + assert not [r for r in caplog.records if r.levelno >= logging.ERROR] + def test_trvcontrol_with_trv_returns_device(self): """trvcontrol with a valid TRV looks up the TRV device.""" devices = { From 1ea9737c9e82eeb6d912ce7fd80f1c0eed942cd8 Mon Sep 17 00:00:00 2001 From: Matt Dennis Date: Tue, 20 Oct 2026 21:05:07 +0100 Subject: [PATCH 2/2] test: drop local logging import now imported at module level Co-Authored-By: Claude Opus 5.5 --- tests/unit/test_helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_helpers.py b/tests/unit/test_helpers.py index 8981aff7..3f286f95 100644 --- a/tests/unit/test_helpers.py +++ b/tests/unit/test_helpers.py @@ -538,7 +538,6 @@ def test_zone_mismatch_keeps_product_as_device(self): def test_trv_without_zone_does_not_log_warning(self, caplog): """TRV devices that omit 'zone' from props are silently skipped (no warning).""" - import logging helper, _ = _make_helper( devices={