Skip to content

Commit

Permalink
Suppress stack trace on config errors
Browse files Browse the repository at this point in the history
  • Loading branch information
jbouwh committed Oct 20, 2023
1 parent 25ab622 commit b39616e
Show file tree
Hide file tree
Showing 8 changed files with 339 additions and 83 deletions.
16 changes: 6 additions & 10 deletions homeassistant/components/mqtt/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
SERVICE_RELOAD,
)
from homeassistant.core import HassJob, HomeAssistant, ServiceCall, callback
from homeassistant.exceptions import HomeAssistantError, TemplateError, Unauthorized
from homeassistant.exceptions import ServiceError, TemplateError, Unauthorized
from homeassistant.helpers import config_validation as cv, event as ev, template
from homeassistant.helpers.device_registry import DeviceEntry
from homeassistant.helpers.dispatcher import async_dispatcher_connect
Expand Down Expand Up @@ -245,7 +245,7 @@ async def async_check_config_schema(
message, _ = conf_util._format_config_error(
ex, domain, config, integration.documentation
)
raise HomeAssistantError(message) from ex
raise ServiceError(message) from ex


async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
Expand Down 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
5 changes: 5 additions & 0 deletions homeassistant/components/websocket_api/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from homeassistant.core import Context, Event, HomeAssistant, State, callback
from homeassistant.exceptions import (
HomeAssistantError,
ServiceError,
ServiceNotFound,
TemplateError,
Unauthorized,
Expand Down Expand Up @@ -236,6 +237,10 @@ async def handle_call_service(
connection.send_error(msg["id"], const.ERR_HOME_ASSISTANT_ERROR, str(err))
except vol.Invalid as err:
connection.send_error(msg["id"], const.ERR_INVALID_FORMAT, str(err))
except ServiceError as err:
connection.logger.error(err)
connection.logger.debug("", exc_info=err)
connection.send_error(msg["id"], const.ERR_HOME_ASSISTANT_ERROR, str(err))

Check warning on line 243 in homeassistant/components/websocket_api/commands.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/websocket_api/commands.py#L241-L243

Added lines #L241 - L243 were not covered by tests
except HomeAssistantError as err:
connection.logger.exception(err)
connection.send_error(msg["id"], const.ERR_HOME_ASSISTANT_ERROR, str(err))
Expand Down
74 changes: 65 additions & 9 deletions homeassistant/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
__version__,
)
from .core import DOMAIN as CONF_CORE, ConfigSource, HomeAssistant, callback
from .exceptions import HomeAssistantError
from .exceptions import HomeAssistantError, ServiceError
from .generated.currencies import HISTORIC_CURRENCIES
from .helpers import (
config_per_platform,
Expand Down 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 ServiceError(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,
) -> 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

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
4 changes: 4 additions & 0 deletions homeassistant/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ class HomeAssistantError(Exception):
"""General Home Assistant exception occurred."""


class ServiceError(HomeAssistantError):
"""An expected exception occurred when calling a service."""


class InvalidEntityFormatError(HomeAssistantError):
"""When an invalid formatted entity is encountered."""

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
24 changes: 23 additions & 1 deletion tests/components/websocket_api/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,12 @@ def ha_error_call(_):

hass.services.async_register("domain_test", "ha_error", ha_error_call)

@callback
def service_error_call(_):
raise HomeAssistantError("error_message")

hass.services.async_register("domain_test", "service_error", service_error_call)

async def unknown_error_call(_):
raise ValueError("value_error")

Expand All @@ -480,14 +486,30 @@ async def unknown_error_call(_):
"id": 6,
"type": "call_service",
"domain": "domain_test",
"service": "unknown_error",
"service": "service_error",
}
)

msg = await websocket_client.receive_json()
assert msg["id"] == 6
assert msg["type"] == const.TYPE_RESULT
assert msg["success"] is False
assert msg["error"]["code"] == "home_assistant_error"
assert msg["error"]["message"] == "error_message"

await websocket_client.send_json(
{
"id": 7,
"type": "call_service",
"domain": "domain_test",
"service": "unknown_error",
}
)

msg = await websocket_client.receive_json()
assert msg["id"] == 7
assert msg["type"] == const.TYPE_RESULT
assert msg["success"] is False
assert msg["error"]["code"] == "unknown_error"
assert msg["error"]["message"] == "value_error"

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, ServiceError
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(ServiceError):
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

0 comments on commit b39616e

Please sign in to comment.