Repository: phac-nml/irida-uploader

Issue:
Update response text to include 400 bad request
        if response.status_code == HTTPStatus.CREATED:
            json_res = json.loads(response.text)
        else:
            e = exceptions.IridaConnectionError("Error {status_code}: {err_msg}\n".format(
                                                status_code=str(response.status_code), err_msg=response.reason))
            logging.error("Error while uploading [{}]: [{}]".format(sample_name, response.reason))
            logging.debug("response.text: " + response.text)
            raise e

**Note:** Code at line 642. Currently we just check for a 201 if successful or we return a 500 if there is any type of error

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8b5d251..a351c66 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,9 @@ Bug Fixes:
 * Fixed hard crash that sometimes occurred when valid url's were given as the base_url
 * Fixed hard crash when running the miniseq parser on a miseq run directory
 
+API Changes:
+* Added 400, 401, 403, and 500 to http errors we explicitly have exceptions for. (Other errors still generate generic exceptions that are handled the same way.)
+
 Beta 0.2.1
 ----------
 Bug Fixes:
diff --git a/api/api_calls.py b/api/api_calls.py
index dea1c27..5f410d4 100644
--- a/api/api_calls.py
+++ b/api/api_calls.py
@@ -253,6 +253,39 @@ class ApiCalls(object):
 
         return ret_val
 
+    @staticmethod
+    def _get_irida_exception(response):
+        """
+        Generates and returns an appropriate exception based on the status code in the response returned by irida
+
+        :param response: response from irida (Non 201)
+        :return: Exception Sub Class
+        """
+        logging.debug("response.status_code: " + str(response.status_code))
+        logging.debug("response.text: " + response.text)
+        if response.status_code == HTTPStatus.BAD_REQUEST:
+            e = exceptions.IridaResourceError("Request to IRIDA was invalid: "
+                                              "{status_code}: {err_msg}\n".format(
+                                                status_code=str(response.status_code),
+                                                err_msg=response.reason))
+        elif response.status_code in [HTTPStatus.UNAUTHORIZED,
+                                      HTTPStatus.FORBIDDEN]:
+            e = exceptions.IridaResourceError("Request to IRIDA is Forbidden. Do you have access to this resource?: "
+                                              "{status_code}: {err_msg}\n".format(
+                                                status_code=str(response.status_code),
+                                                err_msg=response.reason))
+        elif response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR:
+            e = exceptions.IridaConnectionError("IRIDA encountered an error while handling your request: "
+                                                "{status_code}: {err_msg}\n".format(
+                                                  status_code=str(response.status_code),
+                                                  err_msg=response.reason))
+        else:
+            e = exceptions.IridaConnectionError("Error encountered while communicating with IRIDA: "
+                                                "{status_code}: {err_msg}\n".format(
+                                                  status_code=str(response.status_code),
+                                                  err_msg=response.reason))
+        return e
+
     def get_projects(self):
         """
         API call to api/projects to get list of projects
@@ -423,8 +456,7 @@ class ApiCalls(object):
             json_res = json.loads(response.text)
         else:
             logging.error("Error sending project: {} {}".format(response.status_code, response.text))
-            raise exceptions.IridaResourceError("Error sending project: {} {}"
-                                                "".format(response.status_code, response.text))
+            raise self._get_irida_exception(response)
 
         return json_res
 
@@ -468,13 +500,7 @@ class ApiCalls(object):
         else:
             logging.error("Did not create sample on server. Response code is '{}' and error message is '{}'"
                           "".format(response.status_code, response.text))
-            e = exceptions.IridaResourceError("Error {status_code}: {err_msg}.\nSample data: {sample_data}"
-                                              "".format(
-                                                    status_code=str(response.status_code),
-                                                    err_msg=response.text,
-                                                    sample_data=str(sample)
-                                              ), sample.sample_name)
-            raise e
+            raise self._get_irida_exception(response)
 
         return json_res
 
@@ -648,11 +674,8 @@ class ApiCalls(object):
         if response.status_code == HTTPStatus.CREATED:
             json_res = json.loads(response.text)
         else:
-            e = exceptions.IridaConnectionError("Error {status_code}: {err_msg}\n".format(
-                                                status_code=str(response.status_code), err_msg=response.reason))
             logging.error("Error while uploading [{}]: [{}]".format(sample_name, response.reason))
-            logging.debug("response.text: " + response.text)
-            raise e
+            raise self._get_irida_exception(response)
 
         return json_res
 
@@ -714,7 +737,7 @@ class ApiCalls(object):
         else:
             logging.error("Encountered error while creating sequence run: {} {}"
                           "".format(response.status_code, response.reason))
-            raise exceptions.IridaConnectionError("Error: {} {}".format(response.status_code, response.reason))
+            raise self._get_irida_exception(response)
 
         # Grab the run identifier from the returned json
         sequencing_run_id = json_res['resource']['identifier']
diff --git a/core/api_handler.py b/core/api_handler.py
index e3719e7..ce0b83c 100644
--- a/core/api_handler.py
+++ b/core/api_handler.py
@@ -107,6 +107,10 @@ def prepare_and_validate_for_upload(sequencing_run):
                     logging.debug("Sample could not be created")
                     validation_result.add_error(e)
                     continue
+                except api.exceptions.IridaConnectionError as e:
+                    logging.debug("Sample could not be created")
+                    validation_result.add_error(e)
+                    continue
                 logging.debug("Verifying sample was created")
                 if not api_instance.sample_exists(sample.sample_name, project.id):
                     logging.debug("Sample was not created")
@@ -192,3 +196,6 @@ def send_project(project):
     except api.exceptions.IridaResourceError as e:
         logging.error("Failed to send project to IRIDA")
         raise e
+    except api.exceptions.IridaConnectionError as e:
+        logging.error("Failed to send project to IRIDA")
+        raise e
diff --git a/core/cli_entry.py b/core/cli_entry.py
index 6d773dd..0e0f242 100644
--- a/core/cli_entry.py
+++ b/core/cli_entry.py
@@ -183,6 +183,18 @@ def _validate_and_upload(directory_status):
         full_error = "Lost connection to Irida. Errors: " + pformat(e.args)
         _set_and_write_directory_status(directory_status, DirectoryStatus.ERROR, full_error)
         return exit_error()
+    except api.exceptions.IridaResourceError as e:
+        logging.error("Could not access IRIDA resource")
+        logging.error("Errors: " + pformat(e.args))
+        full_error = "Could not access IRIDA resource Errors: " + pformat(e.args)
+        _set_and_write_directory_status(directory_status, DirectoryStatus.ERROR, full_error)
+        return exit_error()
+    except api.exceptions.FileError as e:
+        logging.error("Could not upload file to IRIDA")
+        logging.error("Errors: " + pformat(e.args))
+        full_error = "Could not upload file to IRIDA. Errors: " + pformat(e.args)
+        _set_and_write_directory_status(directory_status, DirectoryStatus.ERROR, full_error)
+        return exit_error()
     logging.info("*** Upload Complete ***")
 
     # Set progress file to complete


Test patch:
diff --git a/tests/core/test_cli_entry.py b/tests/core/test_cli_entry.py
index f5120bc..dc0af66 100644
--- a/tests/core/test_cli_entry.py
+++ b/tests/core/test_cli_entry.py
@@ -6,6 +6,7 @@ import os
 from core import cli_entry, logger
 from model import DirectoryStatus
 from parsers.exceptions import DirectoryError
+from api.exceptions import FileError, IridaResourceError, IridaConnectionError
 
 path_to_module = path.abspath(path.dirname(__file__))
 if len(path_to_module) == 0:
@@ -360,6 +361,150 @@ class TestUploadRunSingleEntry(unittest.TestCase):
         # make sure the upload is NOT done, as validation is invalid
         mock_api_handler.upload_sequencing_run.assert_not_called()
 
+    @patch("core.cli_entry.progress")
+    @patch("core.cli_entry.api_handler")
+    @patch("core.cli_entry.parsing_handler")
+    def test_valid_connection_error_during_upload(self, mock_parsing_handler, mock_api_handler, mock_progress):
+        """
+        Makes sure no crash occurs and program exits with error when IridaConnectionError occurs during upload
+        :return:
+        """
+        my_directory = path.join(path_to_module, "fake_ngs_data")
+
+        class StubValidationResult:
+            @staticmethod
+            def is_valid():
+                return True
+
+        class StubDirectoryStatus:
+            directory = my_directory
+            status = DirectoryStatus.NEW
+
+            @staticmethod
+            def status_equals(status):
+                return status == DirectoryStatus.NEW
+
+        # add mock data to the function calls that are essential
+        stub_directory_status = StubDirectoryStatus()
+        mock_parsing_handler.get_run_status.side_effect = [stub_directory_status]
+        mock_parsing_handler.parse_and_validate.side_effect = ["Fake Sequencing Run"]
+        mock_api_handler.initialize_api_from_config.side_effect = [None]
+        mock_api_handler.prepare_and_validate_for_upload.side_effect = [StubValidationResult]
+        mock_api_handler.upload_sequencing_run.side_effect = [None]
+        mock_api_handler.upload_sequencing_run.side_effect = [IridaConnectionError("")]
+        mock_progress.write_directory_status.side_effect = [None, None]
+
+        result = cli_entry.upload_run_single_entry(my_directory, force_upload=False)
+
+        # Check that the run failed to upload
+        self.assertEqual(result, cli_entry.EXIT_CODE_ERROR)
+        # Make sure directory status is init
+        mock_progress.write_directory_status.assert_called_with(stub_directory_status)
+        # Make sure parsing and validation is done
+        mock_parsing_handler.parse_and_validate.assert_called_with(my_directory)
+        # api must be initialized
+        mock_api_handler.initialize_api_from_config.assert_called_with()
+        # api must prep for upload
+        mock_api_handler.prepare_and_validate_for_upload.assert_called_with("Fake Sequencing Run")
+        # api should try to upload
+        mock_api_handler.upload_sequencing_run.assert_called_with("Fake Sequencing Run")
+
+    @patch("core.cli_entry.progress")
+    @patch("core.cli_entry.api_handler")
+    @patch("core.cli_entry.parsing_handler")
+    def test_valid_resourse_error_during_upload(self, mock_parsing_handler, mock_api_handler, mock_progress):
+        """
+        Makes sure no crash occurs and program exits with error when IridaResourceError occurs during upload
+        :return:
+        """
+        my_directory = path.join(path_to_module, "fake_ngs_data")
+
+        class StubValidationResult:
+            @staticmethod
+            def is_valid():
+                return True
+
+        class StubDirectoryStatus:
+            directory = my_directory
+            status = DirectoryStatus.NEW
+
+            @staticmethod
+            def status_equals(status):
+                return status == DirectoryStatus.NEW
+
+        # add mock data to the function calls that are essential
+        stub_directory_status = StubDirectoryStatus()
+        mock_parsing_handler.get_run_status.side_effect = [stub_directory_status]
+        mock_parsing_handler.parse_and_validate.side_effect = ["Fake Sequencing Run"]
+        mock_api_handler.initialize_api_from_config.side_effect = [None]
+        mock_api_handler.prepare_and_validate_for_upload.side_effect = [StubValidationResult]
+        mock_api_handler.upload_sequencing_run.side_effect = [None]
+        mock_api_handler.upload_sequencing_run.side_effect = [IridaResourceError("")]
+        mock_progress.write_directory_status.side_effect = [None, None]
+
+        result = cli_entry.upload_run_single_entry(my_directory, force_upload=False)
+
+        # Check that the run failed to upload
+        self.assertEqual(result, cli_entry.EXIT_CODE_ERROR)
+        # Make sure directory status is init
+        mock_progress.write_directory_status.assert_called_with(stub_directory_status)
+        # Make sure parsing and validation is done
+        mock_parsing_handler.parse_and_validate.assert_called_with(my_directory)
+        # api must be initialized
+        mock_api_handler.initialize_api_from_config.assert_called_with()
+        # api must prep for upload
+        mock_api_handler.prepare_and_validate_for_upload.assert_called_with("Fake Sequencing Run")
+        # api should try to upload
+        mock_api_handler.upload_sequencing_run.assert_called_with("Fake Sequencing Run")
+
+    @patch("core.cli_entry.progress")
+    @patch("core.cli_entry.api_handler")
+    @patch("core.cli_entry.parsing_handler")
+    def test_valid_file_error_during_upload(self, mock_parsing_handler, mock_api_handler, mock_progress):
+        """
+        Makes sure no crash occurs and program exits with error when FileError occurs during upload
+        :return:
+        """
+        my_directory = path.join(path_to_module, "fake_ngs_data")
+
+        class StubValidationResult:
+            @staticmethod
+            def is_valid():
+                return True
+
+        class StubDirectoryStatus:
+            directory = my_directory
+            status = DirectoryStatus.NEW
+
+            @staticmethod
+            def status_equals(status):
+                return status == DirectoryStatus.NEW
+
+        # add mock data to the function calls that are essential
+        stub_directory_status = StubDirectoryStatus()
+        mock_parsing_handler.get_run_status.side_effect = [stub_directory_status]
+        mock_parsing_handler.parse_and_validate.side_effect = ["Fake Sequencing Run"]
+        mock_api_handler.initialize_api_from_config.side_effect = [None]
+        mock_api_handler.prepare_and_validate_for_upload.side_effect = [StubValidationResult]
+        mock_api_handler.upload_sequencing_run.side_effect = [None]
+        mock_api_handler.upload_sequencing_run.side_effect = [FileError("")]
+        mock_progress.write_directory_status.side_effect = [None, None]
+
+        result = cli_entry.upload_run_single_entry(my_directory, force_upload=False)
+
+        # Check that the run failed to upload
+        self.assertEqual(result, cli_entry.EXIT_CODE_ERROR)
+        # Make sure directory status is init
+        mock_progress.write_directory_status.assert_called_with(stub_directory_status)
+        # Make sure parsing and validation is done
+        mock_parsing_handler.parse_and_validate.assert_called_with(my_directory)
+        # api must be initialized
+        mock_api_handler.initialize_api_from_config.assert_called_with()
+        # api must prep for upload
+        mock_api_handler.prepare_and_validate_for_upload.assert_called_with("Fake Sequencing Run")
+        # api should try to upload
+        mock_api_handler.upload_sequencing_run.assert_called_with("Fake Sequencing Run")
+
 
 class TestBatchUploadSingleEntry(unittest.TestCase):
     """