fix(log)!: Invalid* exceptions replaced with CommandError; errors are logged with the API response (#137)

Author: palazzem

src/elmo/api/client.py

@@ -11,9 +11,8 @@
 from .decorators import require_lock, require_session
 from .exceptions import (
     CodeError,
+    CommandError,
     CredentialError,
-    InvalidInput,
-    InvalidSector,
     InvalidToken,
     LockError,
     ParseError,
@@ -263,20 +262,14 @@ def arm(self, sectors=None):
         # Send the payload to arm sectors
         response = self._session.post(self._router.send_command, data=payload)
         response.raise_for_status()
-
-        errors = []
-        # A not existing sector returns 200 with a fail state
         body = response.json()
-        _LOGGER.debug(f"Client | Arming response: {body}")
-        if not body[0]["Successful"]:
-            errors.append(payload["ElementsIndexes"])
 
-        # Raise an exception if errors are detected
-        if errors:
-            invalid_sectors = ",".join(str(x) for x in errors)
-            raise InvalidSector("Selected sectors don't exist: {}".format(invalid_sectors))
+        # Errors returns 200 with "Successful == False" JSON key
+        if not body[0]["Successful"]:
+            _LOGGER.error(f"Client | Arming response: {body}")
+            raise CommandError
 
-        _LOGGER.debug("Client | Arming successful")
+        _LOGGER.debug(f"Client | Arming successful with response: {body}")
         return True
 
     @require_session
@@ -323,20 +316,14 @@ def disarm(self, sectors=None):
         # Send the payload to disarm sectors
         response = self._session.post(self._router.send_command, data=payload)
         response.raise_for_status()
-
-        errors = []
-        # A not existing sector returns 200 with a fail state
         body = response.json()
-        _LOGGER.debug(f"Client | Disarming response: {body}")
-        if not body[0]["Successful"]:
-            errors.append(payload["ElementsIndexes"])
 
-        # Raise an exception if errors are detected
-        if errors:
-            invalid_sectors = ",".join(str(x) for x in errors)
-            raise InvalidSector("Selected sectors don't exist: {}".format(invalid_sectors))
+        # Errors returns 200 with "Successful == False" JSON key
+        if not body[0]["Successful"]:
+            _LOGGER.error(f"Client | Disarming response: {body}")
+            raise CommandError
 
-        _LOGGER.debug("Client | Disarming successful")
+        _LOGGER.debug(f"Client | Disarming successful with response: {body}")
         return True
 
     @require_session
@@ -392,7 +379,7 @@ def exclude(self, inputs):
         # Raise an exception if errors are detected
         if errors:
             invalid_inputs = ",".join(str(x) for x in errors)
-            raise InvalidInput("Selected inputs don't exist: {}".format(invalid_inputs))
+            raise CommandError("Selected inputs don't exist: {}".format(invalid_inputs))
 
         _LOGGER.debug("Client | Excluding successful")
         return True
@@ -450,7 +437,7 @@ def include(self, inputs):
         # Raise an exception if errors are detected
         if errors:
             invalid_inputs = ",".join(str(x) for x in errors)
-            raise InvalidInput("Selected inputs don't exist: {}".format(invalid_inputs))
+            raise CommandError("Selected inputs don't exist: {}".format(invalid_inputs))
 
         _LOGGER.debug("Client | Including successful")
         return True
@@ -488,24 +475,17 @@ def turn_on(self, outputs):
             "sessionId": self._session_id,
         }
 
-        errors = []
-
         # Send turn on request
         response = self._session.post(self._router.send_command, data=payload)
         response.raise_for_status()
-
-        # A not existing output returns 200 with a fail state
         body = response.json()
-        _LOGGER.debug(f"Client | Turning on response: {body}")
-        if not body[0]["Successful"]:
-            errors.append(payload["ElementsIndexes"])
 
-        # Raise an exception if errors are detected
-        if errors:
-            invalid_outputs = ",".join(str(x) for x in errors)
-            raise InvalidInput("Selected outputs don't exist: {}".format(invalid_outputs))
+        # Errors returns 200 with "Successful == False" JSON key
+        if not body[0]["Successful"]:
+            _LOGGER.error(f"Client | Turning on response: {body}")
+            raise CommandError
 
-        _LOGGER.debug("Client | Turning on output successful")
+        _LOGGER.debug(f"Client | Turning on successful with response: {body}")
         return True
 
     @require_session
@@ -544,20 +524,14 @@ def turn_off(self, outputs):
         # Send turn off request
         response = self._session.post(self._router.send_command, data=payload)
         response.raise_for_status()
-
-        errors = []
-        # A not existing output returns 200 with a fail state
         body = response.json()
-        _LOGGER.debug(f"Client | Turning off response: {body}")
-        if not body[0]["Successful"]:
-            errors.append(payload["ElementsIndexes"])
 
-        # Raise an exception if errors are detected
-        if errors:
-            invalid_outputs = ",".join(str(x) for x in errors)
-            raise InvalidInput("Selected outputs don't exist: {}".format(invalid_outputs))
+        # Errors returns 200 with "Successful == False" JSON key
+        if not body[0]["Successful"]:
+            _LOGGER.error(f"Client | Turning on response: {body}")
+            raise CommandError
 
-        _LOGGER.debug("Client | Turning off output successful")
+        _LOGGER.debug(f"Client | Turning on successful with response: {body}")
         return True
 
     @lru_cache(maxsize=1)

src/elmo/api/exceptions.py

@@ -70,13 +70,7 @@ class CodeError(APIException):
     default_message = "Digited panel code is not correct"
 
 
-class InvalidSector(APIException):
-    """Exception raised when armed/disarmed sector doesn't exist."""
+class CommandError(APIException):
+    """Exception raised when the API returns an error response after issuing a command."""
 
-    default_message = "Selected sector doesn't exist."
-
-
-class InvalidInput(APIException):
-    """Exception raised when included/excluded input doesn't exist."""
-
-    default_message = "Selected input doesn't exist."
+    default_message = "An error occurred while executing the command."

tests/test_client.py

@@ -8,9 +8,8 @@
 from elmo.api.client import ElmoClient
 from elmo.api.exceptions import (
     CodeError,
+    CommandError,
     CredentialError,
-    InvalidInput,
-    InvalidSector,
     InvalidToken,
     LockError,
     LockNotAcquired,
@@ -889,7 +888,7 @@ def test_client_arm_fails_wrong_sector(server):
     client._session_id = "test"
     client._lock.acquire()
     # Test
-    with pytest.raises(InvalidSector):
+    with pytest.raises(CommandError):
         assert client.arm([200])
 
 
@@ -1041,7 +1040,7 @@ def test_client_disarm_fails_wrong_sector(server):
     client._session_id = "test"
     client._lock.acquire()
     # Test
-    with pytest.raises(InvalidSector):
+    with pytest.raises(CommandError):
         assert client.disarm([200])
 
 
@@ -1172,7 +1171,7 @@ def test_client_include_fails_wrong_input(server):
     client._session_id = "test"
     client._lock.acquire()
     # Test
-    with pytest.raises(InvalidInput):
+    with pytest.raises(CommandError):
         assert client.include([9000])
 
 
@@ -1303,7 +1302,7 @@ def test_client_exclude_fails_wrong_input(server):
     client._session_id = "test"
     client._lock.acquire()
     # Test
-    with pytest.raises(InvalidInput):
+    with pytest.raises(CommandError):
         assert client.exclude([9000])
 
 
@@ -1414,7 +1413,7 @@ def test_client_fails_wrong_input(self, server):
         client = ElmoClient(base_url="https://example.com", domain="domain")
         client._session_id = "test"
         # Test
-        with pytest.raises(InvalidInput):
+        with pytest.raises(CommandError):
             assert client.turn_on([9000])
 
     def test_client_fails_unknown_error(self, server):
@@ -1523,7 +1522,7 @@ def test_client_fails_wrong_input(self, server):
         client = ElmoClient(base_url="https://example.com", domain="domain")
         client._session_id = "test"
         # Test
-        with pytest.raises(InvalidInput):
+        with pytest.raises(CommandError):
             assert client.turn_off([9000])
 
     def test_client_fails_unknown_error(self, server):