Merge pull request #14 from gonicus/feat/improve_logging

Improve logging

src/action.py

@@ -65,8 +65,8 @@ def __send_action(self) -> bool:
                 f"Action '{self.__action.name}': Did not receive response after {self.__action.response_timeout}s")
             return False
         if future.response.status != "Success":
-            logging.error(
-                f"Unable to fetch {self.__action.name}: action response: {str(future.response)}")
+            msg = str(future.response.keys.get("Message", future.response))
+            logging.error(f"Unable to fetch {self.__action.name}: action response: {msg}")
             return False
 
         return True

src/client_wrapper.py

@@ -1,4 +1,5 @@
 import logging
+import socket
 from time import sleep, time
 from typing import List, Any
 from asterisk.ami import AMIClient, SimpleAction, Event, FutureResponse
@@ -27,7 +28,7 @@ def __raise_critical(self, msg: str) -> None:
 
         :param str message: Message that is logged and used within the exception."""
         logging.critical(msg)
-        raise Exception(msg)
+        exit(1)
 
     def __validate_login(self, event: Event, **kwargs) -> None:
         """Event callback used during login to wait for the SuccessfulAuth event."""
@@ -49,8 +50,7 @@ def __validate_ami_connection(self) -> bool:
 
         future = self.__client.send_action(action)
         if future.response is None:
-            logging.error(
-                f"Did not receive response after {str(self.__client._timeout)}s")
+            logging.error(f"Did not receive response after {str(self.__client._timeout)}s")
             return False
 
         self.__event_listener.update_time_of_last_event()
@@ -64,15 +64,19 @@ def login(self, username: str, secret: str, timeout: int) -> None:
 
         logging.debug(f"Connecting to AMI: {self.__client._address}:{self.__client._port}")
 
-        self.__client.connect()
+        try:
+            self.__client.connect()
+        except socket.error as e:
+            self.__raise_critical(f"Unable to connect to {self.__client._address}:{self.__client._port}, error: {e}")
 
         # Prevent socket from closing if no events are sent by the AMI. We are validating the
         # AMI connection using the custom EventListener.
         self.__client._socket.settimeout(None)
 
         future = self.__client.login(username, secret)
         if future.response.is_error():
-            self.__raise_critical(str(future.response))
+            msg = str(future.response.keys.get('Message', future.response))
+            self.__raise_critical(f"Unable to login AMI client: {msg}")
 
         # Validate successful login by waiting for the __validate_login
         # function to collect the SuccessfulAuth event.

src/main.py

@@ -29,8 +29,7 @@ def __reconnect(ami_client: ClientWrapper) -> None:
 
 def __restart_event_thread(ami_client: ClientWrapper) -> None:
     """Logs an error and restarts the connection to the AMI."""
-    logging.error(
-        "Event thread ended unexpectedly. Trying to restart connection")
+    logging.error("Event thread ended unexpectedly. Trying to restart connection")
     __reconnect(ami_client)
 
 

test/test_action.py

@@ -13,6 +13,7 @@ class EventMock():
 @dataclass
 class ResponseMock():
     status: str
+    keys: Dict[str, str]
 
 
 @dataclass
@@ -134,7 +135,7 @@ def test__attach_event_filter(self) -> None:
     def test__send_action(self) -> None:
         # Test general AMI error
         self.__client_mock.send_action_result = FutureResponseMock(
-            ResponseMock("error"))
+            ResponseMock("error", {"Message": "Some error"}))
         result = self.__ae._ActionExecuter__send_action()
         self.assertFalse(result, "Expected result to be false")
 
@@ -145,7 +146,7 @@ def test__send_action(self) -> None:
 
         # Test successful action
         self.__client_mock.send_action_result = FutureResponseMock(
-            ResponseMock("Success"))
+            ResponseMock("Success", {"Message": "Success"}))
         result = self.__ae._ActionExecuter__send_action()
         self.assertTrue(result, "Expected result to be true")
 

test/test_client_wrapper.py

@@ -127,9 +127,8 @@ def test_login(self):
 
         self.__ami_client.login_response = FutureResponseMock(
             ResponseMock("Error", False))
-        self.assertRaisesRegex(
-            Exception,
-            "Unable to login AMI client",
+        self.assertRaises(
+            SystemExit,
             self.__client.login,
             "",
             "",