Refactors cli to avoid multi-type returns (normal return + None). Adds exceptions for errors.

cli/exceptions.py

@@ -0,0 +1,22 @@
+class InvalidParameter(ValueError):
+    """Raised when a command line parameter is invalid (either missing or wrong)"""
+
+    def __init__(self, msg, **kwargs):
+        self.msg = msg
+        self.kwargs = kwargs
+
+
+class InvalidKey(Exception):
+    """Raised when there is an error loading the keys"""
+
+    def __init__(self, msg, **kwargs):
+        self.reason = msg
+        self.kwargs = kwargs
+
+
+class TowerResponseError(Exception):
+    """Raised when the tower responds with an error"""
+
+    def __init__(self, msg, **kwargs):
+        self.reason = msg
+        self.kwargs = kwargs

cli/teos_cli.py

@@ -10,8 +10,9 @@ from getopt import getopt, GetoptError
 from requests import ConnectTimeout, ConnectionError
 from requests.exceptions import MissingSchema, InvalidSchema, InvalidURL
 
-from cli.help import show_usage, help_add_appointment, help_get_appointment, help_register
 from cli import DEFAULT_CONF, DATA_DIR, CONF_FILE_NAME, LOG_PREFIX
+from cli.exceptions import InvalidKey, InvalidParameter, TowerResponseError
+from cli.help import show_usage, help_add_appointment, help_get_appointment, help_register
 
 import common.cryptographer
 from common.blob import Blob
@@ -36,26 +37,29 @@ def register(compressed_pk, teos_url):
         teos_url (:obj:`str`): the teos base url.
 
     Returns:
-        :obj:`dict` or :obj:`None`: a dictionary containing the tower response if the registration succeeded. ``None``
-        otherwise.
+        :obj:`dict`: a dictionary containing the tower response if the registration succeeded.
+
+    Raises:
+        :obj:`InvalidParameter <cli.exceptions.InvalidParameter>`: if `compressed_pk` is invalid.
+        :obj:`ConnectionError`: if the client cannot connect to the tower.
+        :obj:`TowerResponseError <cli.exceptions.TowerResponseError>`: if the tower responded with an error, or the
+        response was invalid.
     """
 
     if not is_compressed_pk(compressed_pk):
-        logger.error("The cli public key is not valid")
-        return None
+        raise InvalidParameter("The cli public key is not valid")
 
     # Send request to the server.
     register_endpoint = "{}/register".format(teos_url)
     data = {"public_key": compressed_pk}
 
     logger.info("Registering in the Eye of Satoshi")
-    server_response = post_request(data, register_endpoint)
-    if server_response:
-        response_json = process_post_response(server_response)
-        return response_json
+    response = process_post_response(post_request(data, register_endpoint))
+
+    return response
 
 
-def add_appointment(appointment_data, cli_sk, teos_pk, teos_url, appointments_folder_path):
+def add_appointment(appointment_data, cli_sk, teos_pk, teos_url):
     """
     Manages the add_appointment command.
 
@@ -67,76 +71,67 @@ def add_appointment(appointment_data, cli_sk, teos_pk, teos_url, appointments_fo
         - Send the appointment to the tower
         - Wait for the response
         - Check the tower's response and signature
-        - Store the receipt (appointment + signature) on disk
 
     Args:
         appointment_data (:obj:`dict`): a dictionary containing the appointment data.
         cli_sk (:obj:`PrivateKey`): the client's private key.
         teos_pk (:obj:`PublicKey`): the tower's public key.
         teos_url (:obj:`str`): the teos base url.
-        appointments_folder_path (:obj:`str`): the path to the appointments folder.
-
 
     Returns:
-        :obj:`bool`: True if the appointment is accepted by the tower and the receipt is properly stored. False if any
-        error occurs during the process.
+        :obj:`tuple`: A tuple (`:obj:Appointment <common.appointment.Appointment>`, :obj:`str`) containing the
+        appointment and the tower's signature.
+
+    Raises:
+        :obj:`InvalidParameter <cli.exceptions.InvalidParameter>`: if `appointment_data` or any of its fields is
+        invalid.
+        :obj:`ValueError`: if the appointment cannot be signed.
+        :obj:`ConnectionError`: if the client cannot connect to the tower.
+        :obj:`TowerResponseError <cli.exceptions.TowerResponseError>`: if the tower responded with an error, or the
+        response was invalid.
     """
 
-    if appointment_data is None:
-        logger.error("The provided appointment JSON is empty")
-        return False
-
-    if not is_256b_hex_str(appointment_data.get("tx_id")):
-        logger.error("The provided txid is not valid")
-        return False
+    if not appointment_data:
+        raise InvalidParameter("The provided appointment JSON is empty")
 
     tx_id = appointment_data.get("tx_id")
     tx = appointment_data.get("tx")
 
-    if None not in [tx_id, tx]:
-        appointment_data["locator"] = compute_locator(tx_id)
-        appointment_data["encrypted_blob"] = Cryptographer.encrypt(Blob(tx), tx_id)
+    if not is_256b_hex_str(tx_id):
+        raise InvalidParameter("The provided locator is wrong or missing")
 
-    else:
-        logger.error("Appointment data is missing some fields")
-        return False
+    if not tx:
+        raise InvalidParameter("The provided data is missing the transaction")
 
+    appointment_data["locator"] = compute_locator(tx_id)
+    appointment_data["encrypted_blob"] = Cryptographer.encrypt(Blob(tx), tx_id)
     appointment = Appointment.from_dict(appointment_data)
     signature = Cryptographer.sign(appointment.serialize(), cli_sk)
 
-    if not (appointment and signature):
-        return False
+    # FIXME: the cryptographer should return exception we can capture
+    if not signature:
+        raise ValueError("The provided appointment cannot be signed")
 
     data = {"appointment": appointment.to_dict(), "signature": signature}
 
     # Send appointment to the server.
-    add_appointment_endpoint = "{}/add_appointment".format(teos_url)
     logger.info("Sending appointment to the Eye of Satoshi")
-    server_response = post_request(data, add_appointment_endpoint)
-    if server_response is None:
-        return False
+    add_appointment_endpoint = "{}/add_appointment".format(teos_url)
+    response = process_post_response(post_request(data, add_appointment_endpoint))
 
-    response_json = process_post_response(server_response)
-
-    if response_json is None:
-        return False
-
-    signature = response_json.get("signature")
+    signature = response.get("signature")
     # Check that the server signed the appointment as it should.
-    if signature is None:
-        logger.error("The response does not contain the signature of the appointment")
-        return False
+    if not signature:
+        raise TowerResponseError("The response does not contain the signature of the appointment")
 
     rpk = Cryptographer.recover_pk(appointment.serialize(), signature)
     if not Cryptographer.verify_rpk(teos_pk, rpk):
-        logger.error("The returned appointment's signature is invalid")
-        return False
+        raise TowerResponseError("The returned appointment's signature is invalid")
 
     logger.info("Appointment accepted and signed by the Eye of Satoshi")
-    logger.info("Remaining slots: {}".format(response_json.get("available_slots")))
+    logger.info("Remaining slots: {}".format(response.get("available_slots")))
 
-    # All good, store appointment and signature
-    return save_appointment_receipt(appointment.to_dict(), signature, appointments_folder_path)
+    return appointment, signature
 
 
 def get_appointment(locator, cli_sk, teos_pk, teos_url):
@@ -150,17 +145,20 @@ def get_appointment(locator, cli_sk, teos_pk, teos_url):
         teos_url (:obj:`str`): the teos base url.
 
     Returns:
-        :obj:`dict` or :obj:`None`: a dictionary containing the appointment data if the locator is valid and the tower
-        responds. ``None`` otherwise.
+        :obj:`dict`: a dictionary containing the appointment data.
+
+    Raises:
+        :obj:`InvalidParameter <cli.exceptions.InvalidParameter>`: if `appointment_data` or any of its fields is
+        invalid.
+        :obj:`ConnectionError`: if the client cannot connect to the tower.
+        :obj:`TowerResponseError <cli.exceptions.TowerResponseError>`: if the tower responded with an error, or the
+        response was invalid.
     """
 
     # FIXME: All responses from the tower should be signed. Not using teos_pk atm.
 
-    valid_locator = is_locator(locator)
-
-    if not valid_locator:
-        logger.error("The provided locator is not valid", locator=locator)
-        return None
+    if not is_locator(locator):
+        raise InvalidParameter("The provided locator is not valid", locator=locator)
 
     message = "get appointment {}".format(locator)
     signature = Cryptographer.sign(message.encode(), cli_sk)
@@ -169,10 +167,9 @@ def get_appointment(locator, cli_sk, teos_pk, teos_url):
     # Send request to the server.
     get_appointment_endpoint = "{}/get_appointment".format(teos_url)
     logger.info("Sending appointment to the Eye of Satoshi")
-    server_response = post_request(data, get_appointment_endpoint)
-    response_json = process_post_response(server_response)
+    response = process_post_response(post_request(data, get_appointment_endpoint))
 
-    return response_json
+    return response
 
 
 def load_keys(teos_pk_path, cli_sk_path, cli_pk_path):
@@ -185,45 +182,41 @@ def load_keys(teos_pk_path, cli_sk_path, cli_pk_path):
         cli_pk_path (:obj:`str`): path to the client public key file.
 
     Returns:
-        :obj:`tuple` or ``None``: a three-item tuple containing a ``PrivateKey``, a ``PublicKey`` and a ``str``
-        representing the tower pk, user sk and user compressed pk respectively if all keys can be loaded.
-        ``None`` otherwise.
+        :obj:`tuple`: a three-item tuple containing a ``PrivateKey``, a ``PublicKey`` and a ``str``
+        representing the tower pk, user sk and user compressed pk respectively.
+
+    Raises:
+        :obj:`InvalidKey <cli.exceptions.InvalidKey>`: if any of the keys is invalid or cannot be loaded.
     """
 
-    if teos_pk_path is None:
-        logger.error("TEOS's public key file not found. Please check your settings")
-        return None
+    if not teos_pk_path:
+        raise InvalidKey("TEOS's public key file not found. Please check your settings")
 
-    if cli_sk_path is None:
-        logger.error("Client's private key file not found. Please check your settings")
-        return None
+    if not cli_sk_path:
+        raise InvalidKey("Client's private key file not found. Please check your settings")
 
-    if cli_pk_path is None:
-        logger.error("Client's public key file not found. Please check your settings")
-        return None
+    if not cli_pk_path:
+        raise InvalidKey("Client's public key file not found. Please check your settings")
 
     try:
         teos_pk_der = Cryptographer.load_key_file(teos_pk_path)
         teos_pk = PublicKey(teos_pk_der)
 
     except ValueError:
-        logger.error("TEOS public key is invalid or cannot be parsed")
-        return None
+        raise InvalidKey("TEOS public key is invalid or cannot be parsed")
 
     cli_sk_der = Cryptographer.load_key_file(cli_sk_path)
     cli_sk = Cryptographer.load_private_key_der(cli_sk_der)
 
     if cli_sk is None:
-        logger.error("Client private key is invalid or cannot be parsed")
-        return None
+        raise InvalidKey("Client private key is invalid or cannot be parsed")
 
     try:
         cli_pk_der = Cryptographer.load_key_file(cli_pk_path)
         compressed_cli_pk = Cryptographer.get_compressed_pk(PublicKey(cli_pk_der))
 
     except ValueError:
-        logger.error("Client public key is invalid or cannot be parsed")
-        return None
+        raise InvalidKey("Client public key is invalid or cannot be parsed")
 
     return teos_pk, cli_sk, compressed_cli_pk
 
@@ -237,26 +230,25 @@ def post_request(data, endpoint):
         endpoint (:obj:`str`): the endpoint to send the post request.
 
     Returns:
-        :obj:`dict` or ``None``: a json-encoded dictionary with the server response if the data can be posted.
-        ``None`` otherwise.
+        :obj:`dict`: a json-encoded dictionary with the server response if the data can be posted.
+
+    Raises:
+        :obj:`ConnectionError`: if the client cannot connect to the tower.
     """
 
     try:
         return requests.post(url=endpoint, json=data, timeout=5)
 
     except ConnectTimeout:
-        logger.error("Can't connect to the Eye of Satoshi's API. Connection timeout")
+        message = "Can't connect to the Eye of Satoshi's API. Connection timeout"
 
     except ConnectionError:
-        logger.error("Can't connect to the Eye of Satoshi's API. Server cannot be reached")
+        message = "Can't connect to the Eye of Satoshi's API. Server cannot be reached"
 
     except (InvalidSchema, MissingSchema, InvalidURL):
-        logger.error("Invalid URL. No schema, or invalid schema, found ({})".format(endpoint))
+        message = "Invalid URL. No schema, or invalid schema, found ({})".format(endpoint)
 
-    except requests.exceptions.Timeout:
-        logger.error("The request timed out")
-
-    return None
+    raise ConnectionError(message)
 
 
 def process_post_response(response):
@@ -267,27 +259,26 @@ def process_post_response(response):
         response (:obj:`requests.models.Response`): a ``Response`` object obtained from the request.
 
     Returns:
-        :obj:`dict` or :obj:`None`: a dictionary containing the tower's response data if the response type is
-        ``HTTP_OK`` and the response can be properly parsed. ``None`` otherwise.
-    """
+        :obj:`dict`: a dictionary containing the tower's response data if the response type is
+        ``HTTP_OK``.
 
-    if not response:
-        return None
+    Raises:
+        :obj:`TowerResponseError <cli.exceptions.TowerResponseError>`: if the tower responded with an error, or the
+        response was invalid.
+    """
 
     try:
         response_json = response.json()
 
     except (json.JSONDecodeError, AttributeError):
-        logger.error(
+        raise TowerResponseError(
             "The server returned a non-JSON response", status_code=response.status_code, reason=response.reason
         )
-        return None
 
     if response.status_code != constants.HTTP_OK:
-        logger.error(
+        raise TowerResponseError(
             "The server returned an error", status_code=response.status_code, reason=response.reason, data=response_json
         )
-        return None
 
     return response_json
 
@@ -301,15 +292,18 @@ def parse_add_appointment_args(args):
         option and the path to a file containing a json encoded appointment.
 
     Returns:
-        :obj:`dict` or :obj:`None`: A dictionary containing the appointment data if it can be loaded. ``None``
-        otherwise.
+        :obj:`dict`: A dictionary containing the appointment data.
+
+    Raises:
+        :obj:`InvalidParameter <cli.exceptions.InvalidParameter>`: if the appointment data is not JSON encoded.
+        :obj:`FileNotFoundError`: if -f is passed and the appointment file is not found.
+        :obj:`IOError`: if -f was passed and the file cannot be read.
     """
 
     use_help = "Use 'help add_appointment' for help of how to use the command"
 
     if not args:
-        logger.error("No appointment data provided. " + use_help)
-        return None
+        raise InvalidParameter("No appointment data provided. " + use_help)
 
     arg_opt = args.pop(0)
 
@@ -320,22 +314,20 @@ def parse_add_appointment_args(args):
         if arg_opt in ["-f", "--file"]:
             fin = args.pop(0)
             if not os.path.isfile(fin):
-                logger.error("Can't find file", filename=fin)
-                return None
+                raise FileNotFoundError("Cannot find {}".format(fin))
 
             try:
                 with open(fin) as f:
                     appointment_data = json.load(f)
 
             except IOError as e:
-                logger.error("I/O error", errno=e.errno, error=e.strerror)
-                return None
+                raise IOError("Cannot read appointment file. {}".format(str(e)))
+
         else:
             appointment_data = json.loads(arg_opt)
 
     except json.JSONDecodeError:
-        logger.error("Non-JSON encoded data provided as appointment. " + use_help)
-        return None
+        raise InvalidParameter("Non-JSON encoded data provided as appointment. " + use_help)
 
     return appointment_data
 
@@ -349,9 +341,6 @@ def save_appointment_receipt(appointment, signature, appointments_folder_path):
         signature (:obj:`str`): the signature of the appointment performed by the tower.
         appointments_folder_path (:obj:`str`): the path to the appointments folder.
 
-    Returns:
-        :obj:`bool`: True if the appointment if properly saved. False otherwise.
-
     Raises:
         IOError: if an error occurs whilst writing the file on disk.
     """
@@ -370,14 +359,12 @@ def save_appointment_receipt(appointment, signature, appointments_folder_path):
         with open(filename, "w") as f:
             json.dump(data, f)
             logger.info("Appointment saved at {}".format(filename))
-            return True
 
     except IOError as e:
-        logger.error("There was an error while saving the appointment", error=e)
-        return False
+        raise IOError("There was an error while saving the appointment. {}".format(e))
 
 
-def main(args, command_line_conf):
+def main(command, args, command_line_conf):
     # Loads config and sets up the data folder and log file
     config_loader = ConfigLoader(DATA_DIR, CONF_FILE_NAME, DEFAULT_CONF, command_line_conf)
     config = config_loader.build_config()
@@ -391,68 +378,59 @@ def main(args, command_line_conf):
     if not teos_url.startswith("http"):
         teos_url = "http://" + teos_url
 
-    keys = load_keys(config.get("TEOS_PUBLIC_KEY"), config.get("CLI_PRIVATE_KEY"), config.get("CLI_PUBLIC_KEY"))
-    if keys is not None:
-        teos_pk, cli_sk, compressed_cli_pk = keys
+    try:
+        teos_pk, cli_sk, compressed_cli_pk = load_keys(
+            config.get("TEOS_PUBLIC_KEY"), config.get("CLI_PRIVATE_KEY"), config.get("CLI_PUBLIC_KEY")
+        )
 
-        try:
+        if command == "register":
+            register_data = register(compressed_cli_pk, teos_url)
+            logger.info("Registration succeeded. Available slots: {}".format(register_data.get("available_slots")))
+
+        if command == "add_appointment":
+            appointment_data = parse_add_appointment_args(args)
+            appointment, signature = add_appointment(appointment_data, cli_sk, teos_pk, teos_url)
+            save_appointment_receipt(appointment.to_dict(), signature, config.get("APPOINTMENTS_FOLDER_NAME"))
+
+        elif command == "get_appointment":
+            if not args:
+                logger.error("No arguments were given")
+
+            else:
+                arg_opt = args.pop(0)
+
+                if arg_opt in ["-h", "--help"]:
+                    sys.exit(help_get_appointment())
+
+                appointment_data = get_appointment(arg_opt, cli_sk, teos_pk, teos_url)
+                if appointment_data:
+                    print(appointment_data)
+
+        elif command == "help":
             if args:
                 command = args.pop(0)
 
-                if command in commands:
-                    if command == "register":
-                        register_data = register(compressed_cli_pk, teos_url)
-                        if register_data:
-                            print(register_data)
+                if command == "register":
+                    sys.exit(help_register())
 
-                    if command == "add_appointment":
-                        # Get appointment data from user.
-                        appointment_data = parse_add_appointment_args(args)
-                        add_appointment(
-                            appointment_data, cli_sk, teos_pk, teos_url, config.get("APPOINTMENTS_FOLDER_NAME")
-                        )
+                if command == "add_appointment":
+                    sys.exit(help_add_appointment())
 
-                    elif command == "get_appointment":
-                        if not args:
-                            logger.error("No arguments were given")
-
-                        else:
-                            arg_opt = args.pop(0)
-
-                            if arg_opt in ["-h", "--help"]:
-                                sys.exit(help_get_appointment())
-
-                            appointment_data = get_appointment(arg_opt, cli_sk, teos_pk, teos_url)
-                            if appointment_data:
-                                print(appointment_data)
-
-                    elif command == "help":
-                        if args:
-                            command = args.pop(0)
-
-                            if command == "register":
-                                sys.exit(help_register())
-
-                            if command == "add_appointment":
-                                sys.exit(help_add_appointment())
-
-                            elif command == "get_appointment":
-                                sys.exit(help_get_appointment())
-
-                            else:
-                                logger.error("Unknown command. Use help to check the list of available commands")
-
-                        else:
-                            sys.exit(show_usage())
+                elif command == "get_appointment":
+                    sys.exit(help_get_appointment())
 
                 else:
                     logger.error("Unknown command. Use help to check the list of available commands")
 
             else:
-                logger.error("No command provided. Use help to check the list of available commands")
+                sys.exit(show_usage())
 
-        except json.JSONDecodeError:
-            logger.error("Non-JSON encoded appointment passed as parameter")
+    except (FileNotFoundError, IOError, ConnectionError, ValueError) as e:
+        logger.error(str(e))
+    except (InvalidKey, InvalidParameter, TowerResponseError) as e:
+        logger.error(e.reason, **e.params)
+    except Exception as e:
+        logger.error("Unknown error occurred", error=str(e))
 
 
 if __name__ == "__main__":
@@ -477,7 +455,13 @@ if __name__ == "__main__":
             if opt in ["-h", "--help"]:
                 sys.exit(show_usage())
 
-        main(args, command_line_conf)
+        command = args.pop(0)
+        if command in commands:
+            main(command, args, command_line_conf)
+        elif not command:
+            logger.error("No command provided. Use help to check the list of available commands")
+        else:
+            logger.error("Unknown command. Use help to check the list of available commands")
 
     except GetoptError as e:
         logger.error("{}".format(e))