From 2d04e370c9fcbd84f3a72fd772035a6c75366b42 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Fri, 5 Dec 2025 07:28:00 -0800 Subject: [PATCH 1/2] fix: Throw MQTT authentication errors as authentication related exceptions The intent is to allow callers to catch this to know when they need to re-authenticate a device and obtain new mqtt params. --- roborock/mqtt/roborock_session.py | 17 +++++++++++- roborock/mqtt/session.py | 7 ++++- tests/mqtt/test_roborock_session.py | 40 +++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/roborock/mqtt/roborock_session.py b/roborock/mqtt/roborock_session.py index b35e3910..82e52966 100644 --- a/roborock/mqtt/roborock_session.py +++ b/roborock/mqtt/roborock_session.py @@ -15,9 +15,10 @@ from contextlib import asynccontextmanager import aiomqtt -from aiomqtt import MqttError, TLSParameters +from aiomqtt import MqttCodeError, MqttError, TLSParameters from roborock.callbacks import CallbackMap +from roborock.exceptions import RoborockInvalidCredentials from .session import MqttParams, MqttSession, MqttSessionException @@ -33,6 +34,16 @@ BACKOFF_MULTIPLIER = 1.5 +class MqttReasonCode: + """MQTT Reason Codes used by Roborock devices. + + This is a subset of paho.mqtt.reasoncodes.ReasonCode where we would like + different error handling behavior. + """ + + RC_ERROR_UNAUTHORIZED = 135 + + class RoborockMqttSession(MqttSession): """An MQTT session for sending and receiving messages. @@ -83,6 +94,10 @@ async def start(self) -> None: self._reconnect_task = loop.create_task(self._run_reconnect_loop(start_future)) try: await start_future + except MqttCodeError as err: + if err.rc == MqttReasonCode.RC_ERROR_UNAUTHORIZED: + raise RoborockInvalidCredentials(f"Authorization error starting MQTT session: {err}") from err + raise MqttSessionException(f"Error starting MQTT session: {err}") from err except MqttError as err: raise MqttSessionException(f"Error starting MQTT session: {err}") from err except Exception as err: diff --git a/roborock/mqtt/session.py b/roborock/mqtt/session.py index f5922d23..bf97b63f 100644 --- a/roborock/mqtt/session.py +++ b/roborock/mqtt/session.py @@ -64,4 +64,9 @@ async def close(self) -> None: class MqttSessionException(RoborockException): - """ "Raised when there is an error communicating with MQTT.""" + """Raised when there is an error communicating with MQTT. + + Note that not all exceptions raised by the MQTT session are of this type + as other `RoborockException`s may be raised for specific error conditions + such as authentication errors. + """ diff --git a/tests/mqtt/test_roborock_session.py b/tests/mqtt/test_roborock_session.py index f3b10139..1e5dd0ce 100644 --- a/tests/mqtt/test_roborock_session.py +++ b/tests/mqtt/test_roborock_session.py @@ -11,6 +11,7 @@ import paho.mqtt.client as mqtt import pytest +from roborock.exceptions import RoborockInvalidCredentials from roborock.mqtt.roborock_session import RoborockMqttSession, create_mqtt_session from roborock.mqtt.session import MqttParams, MqttSessionException from tests import mqtt_packet @@ -366,3 +367,42 @@ async def test_idle_timeout_multiple_callbacks(mock_mqtt_client: AsyncMock) -> N mock_mqtt_client.unsubscribe.assert_called_once_with(topic) await session.close() + + +@pytest.mark.parametrize( + ("side_effect", "expected_exception", "match"), + [ + ( + aiomqtt.MqttError("Connection failed"), + MqttSessionException, + "Error starting MQTT session", + ), + ( + aiomqtt.MqttCodeError(rc=135), + RoborockInvalidCredentials, + "Authorization error starting MQTT session", + ), + ( + aiomqtt.MqttCodeError(rc=128), + MqttSessionException, + "Error starting MQTT session", + ), + ( + ValueError("Unexpected"), + MqttSessionException, + "Unexpected error starting session", + ), + ], +) +async def test_connect_failure( + side_effect: Exception, + expected_exception: type[Exception], + match: str, +) -> None: + """Test connection failure with different exceptions.""" + mock_aenter = AsyncMock() + mock_aenter.side_effect = side_effect + + with patch("roborock.mqtt.roborock_session.aiomqtt.Client.__aenter__", mock_aenter): + with pytest.raises(expected_exception, match=match): + await create_mqtt_session(FAKE_PARAMS) From 28237ec9cba037f84940c3e8b6bd115180e07ebd Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Tue, 9 Dec 2025 07:31:48 -0800 Subject: [PATCH 2/2] fix: Update the exception handling behavior to account for ambiguity --- roborock/mqtt/roborock_session.py | 5 ++--- roborock/mqtt/session.py | 16 ++++++++++++---- tests/mqtt/test_roborock_session.py | 5 ++--- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/roborock/mqtt/roborock_session.py b/roborock/mqtt/roborock_session.py index 82e52966..7f02f216 100644 --- a/roborock/mqtt/roborock_session.py +++ b/roborock/mqtt/roborock_session.py @@ -18,9 +18,8 @@ from aiomqtt import MqttCodeError, MqttError, TLSParameters from roborock.callbacks import CallbackMap -from roborock.exceptions import RoborockInvalidCredentials -from .session import MqttParams, MqttSession, MqttSessionException +from .session import MqttParams, MqttSession, MqttSessionException, MqttSessionUnauthorized _LOGGER = logging.getLogger(__name__) _MQTT_LOGGER = logging.getLogger(f"{__name__}.aiomqtt") @@ -96,7 +95,7 @@ async def start(self) -> None: await start_future except MqttCodeError as err: if err.rc == MqttReasonCode.RC_ERROR_UNAUTHORIZED: - raise RoborockInvalidCredentials(f"Authorization error starting MQTT session: {err}") from err + raise MqttSessionUnauthorized(f"Authorization error starting MQTT session: {err}") from err raise MqttSessionException(f"Error starting MQTT session: {err}") from err except MqttError as err: raise MqttSessionException(f"Error starting MQTT session: {err}") from err diff --git a/roborock/mqtt/session.py b/roborock/mqtt/session.py index bf97b63f..02295f1e 100644 --- a/roborock/mqtt/session.py +++ b/roborock/mqtt/session.py @@ -64,9 +64,17 @@ async def close(self) -> None: class MqttSessionException(RoborockException): - """Raised when there is an error communicating with MQTT. + """Raised when there is an error communicating with MQTT.""" - Note that not all exceptions raised by the MQTT session are of this type - as other `RoborockException`s may be raised for specific error conditions - such as authentication errors. + +class MqttSessionUnauthorized(RoborockException): + """Raised when there is an authorization error communicating with MQTT. + + This error may be raised in multiple scenarios so there is not a well + defined behavior for how the caller should behave. The two cases are: + - Rate limiting is in effect and the caller should retry after some time. + - The credentials are invalid and the caller needs to obtain new credentials + + However, it is observed that obtaining new credentials may resolve the + issue in both cases. """ diff --git a/tests/mqtt/test_roborock_session.py b/tests/mqtt/test_roborock_session.py index 1e5dd0ce..505ba539 100644 --- a/tests/mqtt/test_roborock_session.py +++ b/tests/mqtt/test_roborock_session.py @@ -11,9 +11,8 @@ import paho.mqtt.client as mqtt import pytest -from roborock.exceptions import RoborockInvalidCredentials from roborock.mqtt.roborock_session import RoborockMqttSession, create_mqtt_session -from roborock.mqtt.session import MqttParams, MqttSessionException +from roborock.mqtt.session import MqttParams, MqttSessionException, MqttSessionUnauthorized from tests import mqtt_packet from tests.conftest import FakeSocketHandler @@ -379,7 +378,7 @@ async def test_idle_timeout_multiple_callbacks(mock_mqtt_client: AsyncMock) -> N ), ( aiomqtt.MqttCodeError(rc=135), - RoborockInvalidCredentials, + MqttSessionUnauthorized, "Authorization error starting MQTT session", ), (