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.

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

Already on GitHub? Sign in to your account

Allow async_integration_yaml_config to raise if config fails on reload #101037

Closed
wants to merge 15 commits into from
12 changes: 4 additions & 8 deletions homeassistant/components/mqtt/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,14 +404,10 @@ async def async_setup_reload_service() -> None:
async def _reload_config(call: ServiceCall) -> None:
"""Reload the platforms."""
# Fetch updated manually configured items and validate
if (
config_yaml := await async_integration_yaml_config(hass, DOMAIN)
) is None:
# Raise in case we have an invalid configuration
raise HomeAssistantError(
"Error reloading manually configured MQTT items, "
"check your configuration.yaml"
)
config_yaml = await async_integration_yaml_config(
hass, DOMAIN, raise_on_failure=True
)

# Check the schema before continuing reload
await async_check_config_schema(hass, config_yaml)

Expand Down
72 changes: 64 additions & 8 deletions homeassistant/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,23 @@ def process_ha_config_upgrade(hass: HomeAssistant) -> None:
outp.write(__version__)


@callback
def async_raise_exception(
ex: Exception,
domain: str,
config: dict,
hass: HomeAssistant,
link: str | None = None,
) -> None:
"""Raise an error for configuration validation.

This method must be run in the event loop.
"""
async_notify_setup_error(hass, domain, link)
message, _ = _format_config_error(ex, domain, config, link)
raise HomeAssistantError(message) from ex


@callback
def async_log_exception(
ex: Exception,
Expand Down Expand Up @@ -826,18 +843,35 @@ async def merge_packages_config(


async def async_process_component_config( # noqa: C901
hass: HomeAssistant, config: ConfigType, integration: Integration
hass: HomeAssistant,
config: ConfigType,
integration: Integration,
*,
raise_on_failure: bool = False,
jbouwh marked this conversation as resolved.
Show resolved Hide resolved
) -> ConfigType | None:
"""Check component configuration and return processed configuration.

Returns None on error.
Returns None on error,
or an exception is raised if the raise_on_failure is set.

This method must be run in the event loop.
"""
domain = integration.domain

def _raise_on_config_fail(ex: Exception, domain: str, config: ConfigType) -> None:
"""Raise an exception if the config fails instead of logging."""
if raise_on_failure:
async_raise_exception(ex, domain, config, hass, integration.documentation)

def _raise_on_fail(ex: Exception, message: str) -> None:
"""Raise an exception instead of logging."""
if raise_on_failure:
raise HomeAssistantError(message) from ex

try:
component = integration.get_component()
except LOAD_EXCEPTIONS as ex:
_raise_on_fail(ex, f"Unable to import {domain}: {ex}")
_LOGGER.error("Unable to import %s: %s", domain, ex)
return None

Expand All @@ -850,6 +884,7 @@ async def async_process_component_config( # noqa: C901
# If the config platform contains bad imports, make sure
# that still fails.
if err.name != f"{integration.pkg_path}.config":
_raise_on_fail(err, f"Error importing config platform {domain}: {err}")
_LOGGER.error("Error importing config platform %s: %s", domain, err)
return None

Expand All @@ -861,9 +896,11 @@ async def async_process_component_config( # noqa: C901
await config_validator.async_validate_config(hass, config)
)
except (vol.Invalid, HomeAssistantError) as ex:
_raise_on_config_fail(ex, domain, config)
async_log_exception(ex, domain, config, hass, integration.documentation)
return None
except Exception: # pylint: disable=broad-except
except Exception as ex: # pylint: disable=broad-except
_raise_on_fail(ex, f"Unknown error calling {domain} config validator")
_LOGGER.exception("Unknown error calling %s config validator", domain)
return None

Expand All @@ -872,9 +909,11 @@ async def async_process_component_config( # noqa: C901
try:
return component.CONFIG_SCHEMA(config) # type: ignore[no-any-return]
except vol.Invalid as ex:
_raise_on_config_fail(ex, domain, config)
async_log_exception(ex, domain, config, hass, integration.documentation)
return None
except Exception: # pylint: disable=broad-except
except Exception as ex: # pylint: disable=broad-except
_raise_on_fail(ex, f"Unknown error calling {domain} CONFIG_SCHEMA")
_LOGGER.exception("Unknown error calling %s CONFIG_SCHEMA", domain)
return None

Expand All @@ -891,9 +930,17 @@ async def async_process_component_config( # noqa: C901
try:
p_validated = component_platform_schema(p_config)
except vol.Invalid as ex:
_raise_on_config_fail(ex, domain, p_config)
async_log_exception(ex, domain, p_config, hass, integration.documentation)
continue
except Exception: # pylint: disable=broad-except
except Exception as ex: # pylint: disable=broad-except
_raise_on_fail(
ex,
(
f"Unknown error validating {p_name} platform config with {domain} component"
" platform schema"
),
)
_LOGGER.exception(
(
"Unknown error validating %s platform config with %s component"
Expand All @@ -914,12 +961,14 @@ async def async_process_component_config( # noqa: C901
try:
p_integration = await async_get_integration_with_requirements(hass, p_name)
except (RequirementsNotFound, IntegrationNotFound) as ex:
_raise_on_fail(ex, f"Platform error: {domain} - {ex}")
_LOGGER.error("Platform error: %s - %s", domain, ex)
continue

try:
platform = p_integration.get_platform(domain)
except LOAD_EXCEPTIONS:
except LOAD_EXCEPTIONS as ex:
_raise_on_fail(ex, f"Platform error: {domain}")
_LOGGER.exception("Platform error: %s", domain)
continue
Comment on lines +970 to 973
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code part did not have tests for the current code


Expand All @@ -928,15 +977,22 @@ async def async_process_component_config( # noqa: C901
try:
p_validated = platform.PLATFORM_SCHEMA(p_config)
except vol.Invalid as ex:
platform_name = f"{domain}.{p_name}"
_raise_on_config_fail(ex, platform_name, p_config)
async_log_exception(
ex,
f"{domain}.{p_name}",
platform_name,
p_config,
hass,
p_integration.documentation,
)
continue
except Exception: # pylint: disable=broad-except
except Exception as ex: # pylint: disable=broad-except
_raise_on_fail(
ex,
f"Unknown error validating config for {p_name} platform for {domain}"
" component with PLATFORM_SCHEMA",
)
_LOGGER.exception(
(
"Unknown error validating config for %s platform for %s"
Expand Down
26 changes: 23 additions & 3 deletions homeassistant/helpers/reload.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import asyncio
from collections.abc import Iterable
import logging
from typing import Any
from typing import Any, Literal, overload

from homeassistant import config as conf_util
from homeassistant.const import SERVICE_RELOAD
Expand Down Expand Up @@ -138,14 +138,34 @@ async def _async_reconfig_platform(
await asyncio.gather(*tasks)


@overload
async def async_integration_yaml_config(
hass: HomeAssistant, integration_name: str
hass: HomeAssistant, integration_name: str, *, raise_on_failure: Literal[True]
) -> ConfigType:
...


@overload
async def async_integration_yaml_config(
hass: HomeAssistant,
integration_name: str,
*,
raise_on_failure: Literal[False] = False,
) -> ConfigType | None:
...


async def async_integration_yaml_config(
hass: HomeAssistant, integration_name: str, *, raise_on_failure: bool = False
) -> ConfigType | None:
"""Fetch the latest yaml configuration for an integration."""
integration = await async_get_integration(hass, integration_name)

return await conf_util.async_process_component_config(
hass, await conf_util.async_hass_config_yaml(hass), integration
hass,
await conf_util.async_hass_config_yaml(hass),
integration,
raise_on_failure=raise_on_failure,
)


Expand Down
45 changes: 44 additions & 1 deletion tests/helpers/test_reload.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
from unittest.mock import AsyncMock, Mock, patch

import pytest
import voluptuous as vol

from homeassistant import config
from homeassistant.const import SERVICE_RELOAD
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.entity_component import EntityComponent
from homeassistant.helpers.entity_platform import async_get_platforms
from homeassistant.helpers.reload import (
Expand Down Expand Up @@ -208,8 +210,49 @@ async def test_async_integration_yaml_config(hass: HomeAssistant) -> None:
yaml_path = get_fixture_path(f"helpers/{DOMAIN}_configuration.yaml")
with patch.object(config, "YAML_CONFIG_FILE", yaml_path):
processed_config = await async_integration_yaml_config(hass, DOMAIN)
assert processed_config == {DOMAIN: [{"name": "one"}, {"name": "two"}]}
# Test fetching yaml config does not raise when the raise_on_failure option is set
processed_config = await async_integration_yaml_config(
hass, DOMAIN, raise_on_failure=True
)
assert processed_config == {DOMAIN: [{"name": "one"}, {"name": "two"}]}


async def test_async_integration_failing_yaml_config(hass: HomeAssistant) -> None:
"""Test reloading yaml config for an integration fails.

In case an integration reloads its yaml configuration it should throw when
the new config failed to load.
"""
schema_without_name_attr = vol.Schema({vol.Required("some_option"): str})

mock_integration(hass, MockModule(DOMAIN, config_schema=schema_without_name_attr))

yaml_path = get_fixture_path(f"helpers/{DOMAIN}_configuration.yaml")
with patch.object(config, "YAML_CONFIG_FILE", yaml_path):
# Test fetching yaml config does not raise without raise_on_failure option
processed_config = await async_integration_yaml_config(hass, DOMAIN)
assert processed_config is None
# Test fetching yaml config does not raise when the raise_on_failure option is set
with pytest.raises(HomeAssistantError):
await async_integration_yaml_config(hass, DOMAIN, raise_on_failure=True)


async def test_async_integration_failing_on_reload(hass: HomeAssistant) -> None:
"""Test reloading yaml config for an integration fails with other exception.

assert processed_config == {DOMAIN: [{"name": "one"}, {"name": "two"}]}
In case an integration reloads its yaml configuration it should throw when
the new config failed to load.
"""
mock_integration(hass, MockModule(DOMAIN))

yaml_path = get_fixture_path(f"helpers/{DOMAIN}_configuration.yaml")
with patch.object(config, "YAML_CONFIG_FILE", yaml_path), patch(
"homeassistant.config.async_process_component_config",
side_effect=HomeAssistantError(),
), pytest.raises(HomeAssistantError):
# Test fetching yaml config does raise when the raise_on_failure option is set
await async_integration_yaml_config(hass, DOMAIN, raise_on_failure=True)


async def test_async_integration_missing_yaml_config(hass: HomeAssistant) -> None:
Expand Down
Loading
Loading