From 891106daa3f6279f280fdfa4be274a85c281e5bf Mon Sep 17 00:00:00 2001 From: jsightler Date: Thu, 30 Jul 2026 20:35:15 -0400 Subject: [PATCH] Classify wrong-region login failures as invalid_auth, not cannot_connect Each Pecron region signs login requests with its own secret, so picking the wrong region rejects the login exactly like a wrong password would -- but the config flow only mapped errors to invalid_auth via fragile string matching ("authentication"/"401" in the message), so a region-mismatch rejection fell through to the generic cannot_connect error instead, telling users to check their internet connection when the real fix was to try another region. Two users hit this in GH#6: one couldn't log in at all, another found switching from EU to US fixed it. Use the API library's typed AuthenticationError instead of string matching, and mention region as a possible cause in the invalid_auth message. Also fixes an existing bug where the "no devices found" check was inside the try block and got silently re-wrapped as PecronConnectionError by the blanket except clause. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 5 +++ custom_components/pecron/config_flow.py | 44 +++++++++--------- custom_components/pecron/strings.json | 4 +- tests/test_config_flow.py | 59 ++++++++++++++++++++++--- 4 files changed, 85 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 942ae99..f5f5b1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- **Misleading `cannot_connect` error on login (#6)**: wrong-region login failures were being classified as a generic connection error instead of an authentication error. Each Pecron region signs login requests with a different secret, so selecting the wrong region rejects the login identically to a wrong password — but the message told users to check their internet connection instead of suggesting they try another region. The config flow now uses the API library's typed `AuthenticationError` instead of fragile string matching, and the `invalid_auth` message now mentions region as a possible cause. + ## [0.6.0] - 2026-07-30 ### Added diff --git a/custom_components/pecron/config_flow.py b/custom_components/pecron/config_flow.py index 225c809..7720d03 100644 --- a/custom_components/pecron/config_flow.py +++ b/custom_components/pecron/config_flow.py @@ -3,12 +3,13 @@ import logging from typing import Any +import requests import voluptuous as vol from homeassistant import config_entries -from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResult from homeassistant.exceptions import HomeAssistantError from unofficial_pecron_api import PecronAPI +from unofficial_pecron_api.exceptions import AuthenticationError from .const import ( CONF_EMAIL, @@ -34,9 +35,7 @@ def async_get_options_flow(config_entry): """Get the options flow for this handler.""" return PecronOptionsFlow() - async def async_step_user( - self, user_input: dict[str, Any] | None = None - ) -> FlowResult: + async def async_step_user(self, user_input: dict[str, Any] | None = None) -> FlowResult: """Handle the initial step.""" if self._async_current_entries(): return self.async_abort(reason="already_configured") @@ -71,15 +70,13 @@ async def async_step_user( vol.Required(CONF_EMAIL): str, vol.Required(CONF_PASSWORD): str, vol.Optional(CONF_REGION, default=DEFAULT_REGION): vol.In(REGIONS), - vol.Optional( - CONF_REFRESH_INTERVAL, default=DEFAULT_REFRESH_INTERVAL - ): vol.All(vol.Coerce(int), vol.Range(min=60, max=3600)), + vol.Optional(CONF_REFRESH_INTERVAL, default=DEFAULT_REFRESH_INTERVAL): vol.All( + vol.Coerce(int), vol.Range(min=60, max=3600) + ), } ) - return self.async_show_form( - step_id="user", data_schema=data_schema, errors=errors - ) + return self.async_show_form(step_id="user", data_schema=data_schema, errors=errors) async def async_step_import(self, import_data: dict[str, Any]) -> FlowResult: """Import configuration from YAML.""" @@ -87,27 +84,34 @@ async def async_step_import(self, import_data: dict[str, Any]) -> FlowResult: @staticmethod def _validate_pecron_credentials(email: str, password: str, region: str) -> None: - """Validate Pecron credentials.""" + """Validate Pecron credentials. + + The login endpoint rejects a request with an ``AuthenticationError`` + both for a wrong password and for a wrong region: each region signs + requests with a different secret, so picking the wrong region looks + identical to bad credentials from the API's point of view. Surface + both as ``invalid_auth`` (with a region hint in the error string) + rather than the generic ``cannot_connect``, which previously misled + users into thinking it was a network problem. See GH#6. + """ try: api = PecronAPI(region=region) api.login(email, password) devices = api.get_devices() api.close() - - if not devices: - raise PecronAuthError("No devices found on account") - except Exception as err: - if "authentication" in str(err).lower() or "401" in str(err): - raise PecronAuthError(str(err)) from err + except AuthenticationError as err: + raise PecronAuthError(str(err)) from err + except requests.exceptions.RequestException as err: raise PecronConnectionError(str(err)) from err + if not devices: + raise PecronAuthError("No devices found on account") + class PecronOptionsFlow(config_entries.OptionsFlow): """Handle options flow for Pecron integration.""" - async def async_step_init( - self, user_input: dict[str, Any] | None = None - ) -> FlowResult: + async def async_step_init(self, user_input: dict[str, Any] | None = None) -> FlowResult: """Manage the options.""" if user_input is not None: return self.async_create_entry(title="", data=user_input) diff --git a/custom_components/pecron/strings.json b/custom_components/pecron/strings.json index 576242f..f70c60f 100644 --- a/custom_components/pecron/strings.json +++ b/custom_components/pecron/strings.json @@ -19,8 +19,8 @@ } }, "error": { - "invalid_auth": "Invalid email or password", - "cannot_connect": "Failed to connect to Pecron API", + "invalid_auth": "Invalid email or password, or wrong region selected. Each region uses a different login server, so a correct email/password can still be rejected if your account is registered in a different region. If you're sure your credentials are correct, try a different region.", + "cannot_connect": "Failed to reach the Pecron API server. Check your internet connection and try again.", "unknown": "An unknown error occurred" }, "abort": { diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 4705847..51a0861 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -1,14 +1,17 @@ """Tests for config flow.""" -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest -from homeassistant.data_entry_flow import FlowResult +import requests +from unofficial_pecron_api.exceptions import AuthenticationError +from custom_components.pecron.config_flow import ( + PecronAuthError, + PecronConfigFlow, + PecronConnectionError, +) from custom_components.pecron.const import ( - CONF_EMAIL, - CONF_PASSWORD, - CONF_REGION, DEFAULT_REGION, DOMAIN, ) @@ -33,6 +36,52 @@ def test_config_flow_imports() -> None: """Test that config flow module imports successfully.""" try: from custom_components.pecron import config_flow + assert config_flow is not None except ImportError: assert False, "config_flow module could not be imported" + + +class TestValidatePecronCredentials: + """Tests for the region-vs-credentials error classification (GH#6).""" + + def test_authentication_error_raises_auth_error(self) -> None: + """A rejected login (bad password OR wrong region) maps to invalid_auth.""" + with patch("custom_components.pecron.config_flow.PecronAPI") as mock_api_class: + api = MagicMock() + api.login.side_effect = AuthenticationError("signature invalid", code=5001) + mock_api_class.return_value = api + + with pytest.raises(PecronAuthError): + PecronConfigFlow._validate_pecron_credentials("test@example.com", "password", "EU") + + def test_network_error_raises_connection_error(self) -> None: + """A genuine network failure maps to cannot_connect.""" + with patch("custom_components.pecron.config_flow.PecronAPI") as mock_api_class: + api = MagicMock() + api.login.side_effect = requests.exceptions.ConnectionError("DNS failure") + mock_api_class.return_value = api + + with pytest.raises(PecronConnectionError): + PecronConfigFlow._validate_pecron_credentials("test@example.com", "password", "US") + + def test_no_devices_raises_auth_error(self) -> None: + """An account with no bound devices is treated as an auth-level problem.""" + with patch("custom_components.pecron.config_flow.PecronAPI") as mock_api_class: + api = MagicMock() + api.login.return_value = None + api.get_devices.return_value = [] + mock_api_class.return_value = api + + with pytest.raises(PecronAuthError): + PecronConfigFlow._validate_pecron_credentials("test@example.com", "password", "US") + + def test_successful_login_does_not_raise(self) -> None: + """A successful login with devices present raises nothing.""" + with patch("custom_components.pecron.config_flow.PecronAPI") as mock_api_class: + api = MagicMock() + api.login.return_value = None + api.get_devices.return_value = [MagicMock()] + mock_api_class.return_value = api + + PecronConfigFlow._validate_pecron_credentials("test@example.com", "password", "US")