Skip to content

Commit

Permalink
Refactor config.async_log_exception (#104034)
Browse files Browse the repository at this point in the history
* Refactor config.async_log_exception

* Improve test coverage

* Make functions public
  • Loading branch information
emontnemery committed Nov 15, 2023
1 parent c132900 commit 5b37096
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 43 deletions.
2 changes: 1 addition & 1 deletion homeassistant/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ async def async_from_config_dict(
try:
await conf_util.async_process_ha_core_config(hass, core_config)
except vol.Invalid as config_err:
conf_util.async_log_exception(config_err, "homeassistant", core_config, hass)
conf_util.async_log_schema_error(config_err, "homeassistant", core_config, hass)
return None
except HomeAssistantError:
_LOGGER.error(
Expand Down
4 changes: 2 additions & 2 deletions homeassistant/components/device_tracker/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from homeassistant import util
from homeassistant.backports.functools import cached_property
from homeassistant.components import zone
from homeassistant.config import async_log_exception, load_yaml_config_file
from homeassistant.config import async_log_schema_error, load_yaml_config_file
from homeassistant.const import (
ATTR_ENTITY_ID,
ATTR_GPS_ACCURACY,
Expand Down Expand Up @@ -1006,7 +1006,7 @@ async def async_load_config(
device = dev_schema(device)
device["dev_id"] = cv.slugify(dev_id)
except vol.Invalid as exp:
async_log_exception(exp, dev_id, devices, hass)
async_log_schema_error(exp, dev_id, devices, hass)
else:
result.append(Device(hass, **device))
return result
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/mqtt/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ async def async_check_config_schema(
except vol.Invalid as ex:
integration = await async_get_integration(hass, DOMAIN)
# pylint: disable-next=protected-access
message, _ = conf_util._format_config_error(
message = conf_util.format_schema_error(
ex, domain, config, integration.documentation
)
raise ServiceValidationError(
Expand Down
4 changes: 2 additions & 2 deletions homeassistant/components/template/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from homeassistant.components.select import DOMAIN as SELECT_DOMAIN
from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN
from homeassistant.components.weather import DOMAIN as WEATHER_DOMAIN
from homeassistant.config import async_log_exception, config_without_domain
from homeassistant.config import async_log_schema_error, config_without_domain
from homeassistant.const import CONF_BINARY_SENSORS, CONF_SENSORS, CONF_UNIQUE_ID
from homeassistant.helpers import config_validation as cv
from homeassistant.helpers.trigger import async_validate_trigger_config
Expand Down Expand Up @@ -80,7 +80,7 @@ async def async_validate_config(hass, config):
hass, cfg[CONF_TRIGGER]
)
except vol.Invalid as err:
async_log_exception(err, DOMAIN, cfg, hass)
async_log_schema_error(err, DOMAIN, cfg, hass)
continue

legacy_warn_printed = False
Expand Down
71 changes: 45 additions & 26 deletions homeassistant/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,21 +490,37 @@ def process_ha_config_upgrade(hass: HomeAssistant) -> None:


@callback
def async_log_exception(
ex: Exception,
def async_log_schema_error(
ex: vol.Invalid,
domain: str,
config: dict,
hass: HomeAssistant,
link: str | None = None,
) -> None:
"""Log an error for configuration validation.
"""Log a schema validation error."""
if hass is not None:
async_notify_setup_error(hass, domain, link)
message = format_schema_error(ex, domain, config, link)
_LOGGER.error(message)


@callback
def async_log_config_validator_error(
ex: vol.Invalid | HomeAssistantError,
domain: str,
config: dict,
hass: HomeAssistant,
link: str | None = None,
) -> None:
"""Log an error from a custom config validator."""
if isinstance(ex, vol.Invalid):
async_log_schema_error(ex, domain, config, hass, link)
return

This method must be run in the event loop.
"""
if hass is not None:
async_notify_setup_error(hass, domain, link)
message, is_friendly = _format_config_error(ex, domain, config, link)
_LOGGER.error(message, exc_info=not is_friendly and ex)
message = format_homeassistant_error(ex, domain, config, link)
_LOGGER.error(message, exc_info=ex)


def _get_annotation(item: Any) -> tuple[str, int | str] | None:
Expand Down Expand Up @@ -655,25 +671,24 @@ def humanize_error(


@callback
def _format_config_error(
ex: Exception, domain: str, config: dict, link: str | None = None
) -> tuple[str, bool]:
"""Generate log exception for configuration validation.
def format_homeassistant_error(
ex: HomeAssistantError, domain: str, config: dict, link: str | None = None
) -> str:
"""Format HomeAssistantError thrown by a custom config validator."""
message = f"Invalid config for [{domain}]: {str(ex) or repr(ex)}"

This method must be run in the event loop.
"""
is_friendly = False
if domain != CONF_CORE and link:
message += f" Please check the docs at {link}."

if isinstance(ex, vol.Invalid):
message = humanize_error(ex, domain, config, link)
is_friendly = True
else:
message = f"Invalid config for [{domain}]: {str(ex) or repr(ex)}"
return message

if domain != CONF_CORE and link:
message += f" Please check the docs at {link}."

return message, is_friendly
@callback
def format_schema_error(
ex: vol.Invalid, domain: str, config: dict, link: str | None = None
) -> str:
"""Format configuration validation error."""
return humanize_error(ex, domain, config, link)


async def async_process_ha_core_config(hass: HomeAssistant, config: dict) -> None:
Expand Down Expand Up @@ -995,7 +1010,9 @@ async def async_process_component_config( # noqa: C901
await config_validator.async_validate_config(hass, config)
)
except (vol.Invalid, HomeAssistantError) as ex:
async_log_exception(ex, domain, config, hass, integration.documentation)
async_log_config_validator_error(
ex, domain, config, hass, integration.documentation
)
return None
except Exception: # pylint: disable=broad-except
_LOGGER.exception("Unknown error calling %s config validator", domain)
Expand All @@ -1006,7 +1023,7 @@ async def async_process_component_config( # noqa: C901
try:
return component.CONFIG_SCHEMA(config) # type: ignore[no-any-return]
except vol.Invalid as ex:
async_log_exception(ex, domain, config, hass, integration.documentation)
async_log_schema_error(ex, domain, config, hass, integration.documentation)
return None
except Exception: # pylint: disable=broad-except
_LOGGER.exception("Unknown error calling %s CONFIG_SCHEMA", domain)
Expand All @@ -1025,7 +1042,9 @@ async def async_process_component_config( # noqa: C901
try:
p_validated = component_platform_schema(p_config)
except vol.Invalid as ex:
async_log_exception(ex, domain, p_config, hass, integration.documentation)
async_log_schema_error(
ex, domain, p_config, hass, integration.documentation
)
continue
except Exception: # pylint: disable=broad-except
_LOGGER.exception(
Expand Down Expand Up @@ -1062,7 +1081,7 @@ async def async_process_component_config( # noqa: C901
try:
p_validated = platform.PLATFORM_SCHEMA(p_config)
except vol.Invalid as ex:
async_log_exception(
async_log_schema_error(
ex,
f"{domain}.{p_name}",
p_config,
Expand Down
18 changes: 12 additions & 6 deletions homeassistant/helpers/check_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
CONF_PACKAGES,
CORE_CONFIG_SCHEMA,
YAML_CONFIG_FILE,
_format_config_error,
config_per_platform,
extract_domain_configs,
format_homeassistant_error,
format_schema_error,
load_yaml_config_file,
merge_packages_config,
)
Expand Down Expand Up @@ -94,15 +95,20 @@ async def async_check_ha_config_file( # noqa: C901
def _pack_error(
package: str, component: str, config: ConfigType, message: str
) -> None:
"""Handle errors from packages: _log_pkg_error."""
"""Handle errors from packages."""
message = f"Package {package} setup failed. Component {component} {message}"
domain = f"homeassistant.packages.{package}.{component}"
pack_config = core_config[CONF_PACKAGES].get(package, config)
result.add_warning(message, domain, pack_config)

def _comp_error(ex: Exception, domain: str, component_config: ConfigType) -> None:
"""Handle errors from components: async_log_exception."""
message = _format_config_error(ex, domain, component_config)[0]
def _comp_error(
ex: vol.Invalid | HomeAssistantError, domain: str, component_config: ConfigType
) -> None:
"""Handle errors from components."""
if isinstance(ex, vol.Invalid):
message = format_schema_error(ex, domain, component_config)
else:
message = format_homeassistant_error(ex, domain, component_config)
if domain in frontend_dependencies:
result.add_error(message, domain, component_config)
else:
Expand Down Expand Up @@ -149,7 +155,7 @@ async def _get_integration(
result[CONF_CORE] = core_config
except vol.Invalid as err:
result.add_error(
_format_config_error(err, CONF_CORE, core_config)[0], CONF_CORE, core_config
format_schema_error(err, CONF_CORE, core_config), CONF_CORE, core_config
)
core_config = {}

Expand Down
37 changes: 32 additions & 5 deletions tests/helpers/test_check_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from homeassistant.config import YAML_CONFIG_FILE
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.check_config import (
CheckConfigError,
HomeAssistantConfig,
Expand Down Expand Up @@ -440,12 +441,38 @@ async def test_automation_config_platform(hass: HomeAssistant) -> None:
assert "input_datetime" in res


async def test_config_platform_raise(hass: HomeAssistant) -> None:
@pytest.mark.parametrize(
("exception", "errors", "warnings", "message", "config"),
[
(
Exception("Broken"),
1,
0,
"Unexpected error calling config validator: Broken",
{"value": 1},
),
(
HomeAssistantError("Broken"),
0,
1,
"Invalid config for [bla]: Broken",
{"bla": {"value": 1}},
),
],
)
async def test_config_platform_raise(
hass: HomeAssistant,
exception: Exception,
errors: int,
warnings: int,
message: str,
config: dict,
) -> None:
"""Test bad config validation platform."""
mock_platform(
hass,
"bla.config",
Mock(async_validate_config=Mock(side_effect=Exception("Broken"))),
Mock(async_validate_config=Mock(side_effect=exception)),
)
files = {
YAML_CONFIG_FILE: BASE_CONFIG
Expand All @@ -457,11 +484,11 @@ async def test_config_platform_raise(hass: HomeAssistant) -> None:
with patch("os.path.isfile", return_value=True), patch_yaml_files(files):
res = await async_check_ha_config_file(hass)
error = CheckConfigError(
"Unexpected error calling config validator: Broken",
message,
"bla",
{"value": 1},
config,
)
_assert_warnings_errors(res, [], [error])
_assert_warnings_errors(res, [error] * warnings, [error] * errors)


async def test_removed_yaml_support(hass: HomeAssistant) -> None:
Expand Down
3 changes: 3 additions & 0 deletions tests/snapshots/test_config.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,9 @@
Invalid config for [adr_0007_5] at <BASE_PATH>/fixtures/core/config/component_validation/basic/configuration.yaml, line 44: 'no_such_option' is an invalid option for [adr_0007_5], check: adr_0007_5->no_such_option. Please check the docs at https://www.home-assistant.io/integrations/adr_0007_5
Invalid config for [adr_0007_5] at <BASE_PATH>/fixtures/core/config/component_validation/basic/configuration.yaml, line 45: expected int for dictionary value 'adr_0007_5->port', got 'foo'. Please check the docs at https://www.home-assistant.io/integrations/adr_0007_5.
''',
"Invalid config for [custom_validator_ok_2] at <BASE_PATH>/fixtures/core/config/component_validation/basic/configuration.yaml, line 52: required key 'host' not provided. Please check the docs at https://www.home-assistant.io/integrations/custom_validator_ok_2.",
'Invalid config for [custom_validator_bad_1]: broken Please check the docs at https://www.home-assistant.io/integrations/custom_validator_bad_1.',
'Unknown error calling custom_validator_bad_2 config validator',
])
# ---
# name: test_package_merge_error[packages]
Expand Down
74 changes: 74 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,75 @@ async def async_validate_config(
)


@pytest.fixture
async def mock_custom_validator_integrations_with_docs(
hass: HomeAssistant,
) -> list[Integration]:
"""Mock integrations with custom validator."""
integrations = []

for domain in ("custom_validator_ok_1", "custom_validator_ok_2"):

def gen_async_validate_config(domain):
schema = vol.Schema(
{
domain: vol.Schema(
{
vol.Required("host"): str,
vol.Optional("port", default=8080): int,
}
)
},
extra=vol.ALLOW_EXTRA,
)

async def async_validate_config(
hass: HomeAssistant, config: ConfigType
) -> ConfigType:
"""Validate config."""
return schema(config)

return async_validate_config

integrations.append(
mock_integration(
hass,
MockModule(
domain,
partial_manifest={
"documentation": f"https://www.home-assistant.io/integrations/{domain}"
},
),
)
)
mock_platform(
hass,
f"{domain}.config",
Mock(async_validate_config=gen_async_validate_config(domain)),
)

for domain, exception in [
("custom_validator_bad_1", HomeAssistantError("broken")),
("custom_validator_bad_2", ValueError("broken")),
]:
integrations.append(
mock_integration(
hass,
MockModule(
domain,
partial_manifest={
"documentation": f"https://www.home-assistant.io/integrations/{domain}"
},
),
)
)
mock_platform(
hass,
f"{domain}.config",
Mock(async_validate_config=AsyncMock(side_effect=exception)),
)


async def test_create_default_config(hass: HomeAssistant) -> None:
"""Test creation of default config."""
assert not os.path.isfile(YAML_PATH)
Expand Down Expand Up @@ -1683,6 +1752,7 @@ async def test_component_config_validation_error_with_docs(
mock_iot_domain_integration_with_docs: Integration,
mock_non_adr_0007_integration_with_docs: None,
mock_adr_0007_integrations_with_docs: list[Integration],
mock_custom_validator_integrations_with_docs: list[Integration],
snapshot: SnapshotAssertion,
) -> None:
"""Test schema error in component."""
Expand All @@ -1700,6 +1770,10 @@ async def test_component_config_validation_error_with_docs(
"adr_0007_3",
"adr_0007_4",
"adr_0007_5",
"custom_validator_ok_1",
"custom_validator_ok_2",
"custom_validator_bad_1",
"custom_validator_bad_2",
]:
integration = await async_get_integration(hass, domain)
await config_util.async_process_component_config(
Expand Down

0 comments on commit 5b37096

Please sign in to comment.