Improve invalid error messages in the config flows (#108075)

This commit is contained in:
Robert Resch 2024-01-30 12:24:19 +01:00 committed by GitHub
parent 8ad0226241
commit 6fdad44941
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 162 additions and 23 deletions

View file

@ -104,6 +104,23 @@ class UnknownStep(FlowError):
"""Unknown step specified.""" """Unknown step specified."""
# ignore misc is required as vol.Invalid is not typed
# mypy error: Class cannot subclass "Invalid" (has type "Any")
class InvalidData(vol.Invalid): # type: ignore[misc]
"""Invalid data provided."""
def __init__(
self,
message: str,
path: list[str | vol.Marker] | None,
error_message: str | None,
schema_errors: dict[str, Any],
**kwargs: Any,
) -> None:
super().__init__(message, path, error_message, **kwargs)
self.schema_errors = schema_errors
class AbortFlow(FlowError): class AbortFlow(FlowError):
"""Exception to indicate a flow needs to be aborted.""" """Exception to indicate a flow needs to be aborted."""
@ -165,6 +182,29 @@ def _async_flow_handler_to_flow_result(
return results return results
def _map_error_to_schema_errors(
schema_errors: dict[str, Any],
error: vol.Invalid,
data_schema: vol.Schema,
) -> None:
"""Map an error to the correct position in the schema_errors.
Raises ValueError if the error path could not be found in the schema.
Limitation: Nested schemas are not supported and a ValueError will be raised.
"""
schema = data_schema.schema
error_path = error.path
if not error_path or (path_part := error_path[0]) not in schema:
raise ValueError("Could not find path in schema")
if len(error_path) > 1:
raise ValueError("Nested schemas are not supported")
# path_part can also be vol.Marker, but we need a string key
path_part_str = str(path_part)
schema_errors[path_part_str] = error.error_message
class FlowManager(abc.ABC): class FlowManager(abc.ABC):
"""Manage all the flows that are in progress.""" """Manage all the flows that are in progress."""
@ -334,7 +374,26 @@ class FlowManager(abc.ABC):
if ( if (
data_schema := cur_step.get("data_schema") data_schema := cur_step.get("data_schema")
) is not None and user_input is not None: ) is not None and user_input is not None:
try:
user_input = data_schema(user_input) user_input = data_schema(user_input)
except vol.Invalid as ex:
raised_errors = [ex]
if isinstance(ex, vol.MultipleInvalid):
raised_errors = ex.errors
schema_errors: dict[str, Any] = {}
for error in raised_errors:
try:
_map_error_to_schema_errors(schema_errors, error, data_schema)
except ValueError:
# If we get here, the path in the exception does not exist in the schema.
schema_errors.setdefault("base", []).append(str(error))
raise InvalidData(
"Schema validation failed",
path=ex.path,
error_message=ex.error_message,
schema_errors=schema_errors,
) from ex
# Handle a menu navigation choice # Handle a menu navigation choice
if cur_step["type"] == FlowResultType.MENU and user_input: if cur_step["type"] == FlowResultType.MENU and user_input:

View file

@ -110,10 +110,8 @@ class FlowManagerResourceView(_BaseFlowManagerView):
result = await self._flow_mgr.async_configure(flow_id, data) result = await self._flow_mgr.async_configure(flow_id, data)
except data_entry_flow.UnknownFlow: except data_entry_flow.UnknownFlow:
return self.json_message("Invalid flow specified", HTTPStatus.NOT_FOUND) return self.json_message("Invalid flow specified", HTTPStatus.NOT_FOUND)
except vol.Invalid as ex: except data_entry_flow.InvalidData as ex:
return self.json_message( return self.json({"errors": ex.schema_errors}, HTTPStatus.BAD_REQUEST)
f"User input malformed: {ex}", HTTPStatus.BAD_REQUEST
)
result = self._prepare_result_json(result) result = self._prepare_result_json(result)

View file

@ -9,6 +9,7 @@ import voluptuous as vol
from homeassistant import config_entries as core_ce, data_entry_flow from homeassistant import config_entries as core_ce, data_entry_flow
from homeassistant.components.config import config_entries from homeassistant.components.config import config_entries
from homeassistant.config_entries import HANDLERS, ConfigFlow from homeassistant.config_entries import HANDLERS, ConfigFlow
from homeassistant.const import CONF_LATITUDE, CONF_LONGITUDE, CONF_RADIUS
from homeassistant.core import HomeAssistant, callback from homeassistant.core import HomeAssistant, callback
from homeassistant.generated import config_flows from homeassistant.generated import config_flows
from homeassistant.helpers import config_entry_flow, config_validation as cv from homeassistant.helpers import config_entry_flow, config_validation as cv
@ -1019,12 +1020,7 @@ async def test_options_flow_with_invalid_data(hass: HomeAssistant, client) -> No
) )
assert resp.status == HTTPStatus.BAD_REQUEST assert resp.status == HTTPStatus.BAD_REQUEST
data = await resp.json() data = await resp.json()
assert data == { assert data == {"errors": {"choices": "invalid is not a valid option"}}
"message": (
"User input malformed: invalid is not a valid option for "
"dictionary value @ data['choices']"
)
}
async def test_get_single( async def test_get_single(
@ -2027,3 +2023,89 @@ async def test_subscribe_entries_ws_filtered(
"type": "added", "type": "added",
} }
] ]
async def test_flow_with_multiple_schema_errors(hass: HomeAssistant, client) -> None:
"""Test an config flow with multiple schema errors."""
mock_integration(
hass, MockModule("test", async_setup_entry=AsyncMock(return_value=True))
)
mock_platform(hass, "test.config_flow", None)
class TestFlow(core_ce.ConfigFlow):
async def async_step_user(self, user_input=None):
return self.async_show_form(
step_id="user",
data_schema=vol.Schema(
{
vol.Required(CONF_LATITUDE): cv.latitude,
vol.Required(CONF_LONGITUDE): cv.longitude,
vol.Required(CONF_RADIUS): vol.All(int, vol.Range(min=5)),
}
),
)
with patch.dict(HANDLERS, {"test": TestFlow}):
resp = await client.post(
"/api/config/config_entries/flow", json={"handler": "test"}
)
assert resp.status == HTTPStatus.OK
flow_id = (await resp.json())["flow_id"]
resp = await client.post(
f"/api/config/config_entries/flow/{flow_id}",
json={"latitude": 30000, "longitude": 30000, "radius": 1},
)
assert resp.status == HTTPStatus.BAD_REQUEST
data = await resp.json()
assert data == {
"errors": {
"latitude": "invalid latitude",
"longitude": "invalid longitude",
"radius": "value must be at least 5",
}
}
async def test_flow_with_multiple_schema_errors_base(
hass: HomeAssistant, client
) -> None:
"""Test an config flow with multiple schema errors where fields are not in the schema."""
mock_integration(
hass, MockModule("test", async_setup_entry=AsyncMock(return_value=True))
)
mock_platform(hass, "test.config_flow", None)
class TestFlow(core_ce.ConfigFlow):
async def async_step_user(self, user_input=None):
return self.async_show_form(
step_id="user",
data_schema=vol.Schema(
{
vol.Required(CONF_LATITUDE): cv.latitude,
}
),
)
with patch.dict(HANDLERS, {"test": TestFlow}):
resp = await client.post(
"/api/config/config_entries/flow", json={"handler": "test"}
)
assert resp.status == HTTPStatus.OK
flow_id = (await resp.json())["flow_id"]
resp = await client.post(
f"/api/config/config_entries/flow/{flow_id}",
json={"invalid": 30000, "invalid_2": 30000},
)
assert resp.status == HTTPStatus.BAD_REQUEST
data = await resp.json()
assert data == {
"errors": {
"base": [
"extra keys not allowed @ data['invalid']",
"extra keys not allowed @ data['invalid_2']",
],
"latitude": "required key not provided",
}
}

View file

@ -2,7 +2,7 @@
from unittest.mock import patch from unittest.mock import patch
import pytest import pytest
from voluptuous.error import MultipleInvalid from voluptuous.error import Invalid
from homeassistant import config_entries from homeassistant import config_entries
from homeassistant.components.eafm import const from homeassistant.components.eafm import const
@ -32,7 +32,7 @@ async def test_flow_invalid_station(hass: HomeAssistant, mock_get_stations) -> N
) )
assert result["type"] == "form" assert result["type"] == "form"
with pytest.raises(MultipleInvalid): with pytest.raises(Invalid):
result = await hass.config_entries.flow.async_configure( result = await hass.config_entries.flow.async_configure(
result["flow_id"], user_input={"station": "My other station"} result["flow_id"], user_input={"station": "My other station"}
) )

View file

@ -1444,7 +1444,7 @@ async def test_options_flow_exclude_mode_skips_category_entities(
# sonos_config_switch.entity_id is a config category entity # sonos_config_switch.entity_id is a config category entity
# so it should not be selectable since it will always be excluded # so it should not be selectable since it will always be excluded
with pytest.raises(vol.error.MultipleInvalid): with pytest.raises(vol.error.Invalid):
await hass.config_entries.options.async_configure( await hass.config_entries.options.async_configure(
result2["flow_id"], result2["flow_id"],
user_input={"entities": [sonos_config_switch.entity_id]}, user_input={"entities": [sonos_config_switch.entity_id]},
@ -1539,7 +1539,7 @@ async def test_options_flow_exclude_mode_skips_hidden_entities(
# sonos_hidden_switch.entity_id is a hidden entity # sonos_hidden_switch.entity_id is a hidden entity
# so it should not be selectable since it will always be excluded # so it should not be selectable since it will always be excluded
with pytest.raises(vol.error.MultipleInvalid): with pytest.raises(vol.error.Invalid):
await hass.config_entries.options.async_configure( await hass.config_entries.options.async_configure(
result2["flow_id"], result2["flow_id"],
user_input={"entities": [sonos_hidden_switch.entity_id]}, user_input={"entities": [sonos_hidden_switch.entity_id]},

View file

@ -465,7 +465,7 @@ async def test_advanced_options_form(
# Check if entry was updated # Check if entry was updated
for key, value in new_config.items(): for key, value in new_config.items():
assert entry.data[key] == value assert entry.data[key] == value
except vol.MultipleInvalid: except vol.Invalid:
# Check if form was expected with these options # Check if form was expected with these options
assert assert_result == data_entry_flow.FlowResultType.FORM assert assert_result == data_entry_flow.FlowResultType.FORM

View file

@ -48,7 +48,7 @@ async def test_user_step_discovered_devices(
assert result["type"] == FlowResultType.FORM assert result["type"] == FlowResultType.FORM
assert result["step_id"] == "pick_device" assert result["step_id"] == "pick_device"
with pytest.raises(vol.MultipleInvalid): with pytest.raises(vol.Invalid):
await hass.config_entries.flow.async_configure( await hass.config_entries.flow.async_configure(
result["flow_id"], user_input={CONF_ADDRESS: "wrong_address"} result["flow_id"], user_input={CONF_ADDRESS: "wrong_address"}
) )
@ -95,7 +95,7 @@ async def test_user_step_with_existing_device(
assert result["type"] == FlowResultType.FORM assert result["type"] == FlowResultType.FORM
with pytest.raises(vol.MultipleInvalid): with pytest.raises(vol.Invalid):
await hass.config_entries.flow.async_configure( await hass.config_entries.flow.async_configure(
result["flow_id"], user_input={CONF_ADDRESS: FAKE_ADDRESS_1} result["flow_id"], user_input={CONF_ADDRESS: FAKE_ADDRESS_1}
) )

View file

@ -672,7 +672,7 @@ async def test_keepalive_validation(
assert result["step_id"] == "broker" assert result["step_id"] == "broker"
if error: if error:
with pytest.raises(vol.MultipleInvalid): with pytest.raises(vol.Invalid):
result = await hass.config_entries.options.async_configure( result = await hass.config_entries.options.async_configure(
result["flow_id"], result["flow_id"],
user_input=test_input, user_input=test_input,

View file

@ -3,7 +3,7 @@ from unittest.mock import patch
from peco import HttpError, IncompatibleMeterError, UnresponsiveMeterError from peco import HttpError, IncompatibleMeterError, UnresponsiveMeterError
import pytest import pytest
from voluptuous.error import MultipleInvalid from voluptuous.error import Invalid
from homeassistant import config_entries from homeassistant import config_entries
from homeassistant.components.peco.const import DOMAIN from homeassistant.components.peco.const import DOMAIN
@ -51,7 +51,7 @@ async def test_invalid_county(hass: HomeAssistant) -> None:
with patch( with patch(
"homeassistant.components.peco.async_setup_entry", "homeassistant.components.peco.async_setup_entry",
return_value=True, return_value=True,
), pytest.raises(MultipleInvalid): ), pytest.raises(Invalid):
await hass.config_entries.flow.async_configure( await hass.config_entries.flow.async_configure(
result["flow_id"], result["flow_id"],
{ {

View file

@ -383,14 +383,14 @@ async def test_ha_to_risco_schema(hass: HomeAssistant) -> None:
) )
# Test an HA state that isn't used # Test an HA state that isn't used
with pytest.raises(vol.error.MultipleInvalid): with pytest.raises(vol.error.Invalid):
await hass.config_entries.options.async_configure( await hass.config_entries.options.async_configure(
result["flow_id"], result["flow_id"],
user_input={**TEST_HA_TO_RISCO, "armed_custom_bypass": "D"}, user_input={**TEST_HA_TO_RISCO, "armed_custom_bypass": "D"},
) )
# Test a combo that can't be selected # Test a combo that can't be selected
with pytest.raises(vol.error.MultipleInvalid): with pytest.raises(vol.error.Invalid):
await hass.config_entries.options.async_configure( await hass.config_entries.options.async_configure(
result["flow_id"], result["flow_id"],
user_input={**TEST_HA_TO_RISCO, "armed_night": "A"}, user_input={**TEST_HA_TO_RISCO, "armed_night": "A"},