Merge pull request #319 from Duke-GCB/better-exception-handling

johnbradley · web-flow · commit 87edfbd78dbe · 2020-11-25T09:32:35.000-05:00
Show stack traces for general exceptions
diff --git a/ddsc/__main__.py b/ddsc/__main__.py
@@ -1,6 +1,7 @@
 """Duke data service command line project management utility."""
 import sys
 from ddsc.ddsclient import DDSClient
+from ddsc.exceptions import DDSUserException
 
 
 def main(args=None):
@@ -9,7 +10,7 @@ def main(args=None):
     client = DDSClient()
     try:
         client.run_command(args)
-    except Exception as ex:
+    except DDSUserException as ex:
         if client.show_error_stack_trace:
             raise
         else:
diff --git a/ddsc/config.py b/ddsc/config.py
@@ -5,6 +5,7 @@
 import yaml
 import multiprocessing
 from ddsc.core.util import verify_file_private
+from ddsc.exceptions import DDSUserException
 
 try:
     from urllib.parse import urlparse
@@ -92,7 +93,7 @@ def add_properties(self, filename):
                 if config_data:
                     self.update_properties(config_data)
                 else:
-                    raise ValueError("Error: Empty config file {}".format(filename))
+                    raise DDSUserException("Error: Empty config file {}".format(filename))
 
     def update_properties(self, new_values):
         """
diff --git a/ddsc/core/d4s2.py b/ddsc/core/d4s2.py
@@ -14,6 +14,7 @@
 from ddsc.versioncheck import get_internal_version_str
 from ddsc.core.remotestore import ProjectNameOrId, RemotePath
 from ddsc.sdk.client import Client
+from ddsc.exceptions import DDSUserException
 
 UNAUTHORIZED_MESSAGE = """
 ERROR: Your account does not have authorization for D4S2 (the deliver/share service).
@@ -33,7 +34,7 @@
 """
 
 
-class D4S2Error(Exception):
+class D4S2Error(DDSUserException):
     def __init__(self, message, warning=False):
         """
         Setup error.
@@ -45,7 +46,7 @@ def __init__(self, message, warning=False):
         self.warning = warning
 
 
-class ShareWithSelfError(Exception):
+class ShareWithSelfError(DDSUserException):
     """
     Error raised whe user attempts to share/deliver a project just themselves
     """
@@ -56,7 +57,7 @@ def __init__(self, message):
         Exception.__init__(self, message)
 
 
-class UserMissingEmailError(Exception):
+class UserMissingEmailError(DDSUserException):
     """
     Raised when attempting to deliver or share with a DukeDS user that has a null email
     """
@@ -345,7 +346,7 @@ def _copy_project(self, project, new_project_name, path_filter):
         new_project_name_or_id = ProjectNameOrId.create_from_name(new_project_name)
         remote_project = self.remote_store.fetch_remote_project(new_project_name_or_id)
         if remote_project:
-            raise ValueError("A project with name '{}' already exists.".format(new_project_name))
+            raise DDSUserException("A project with name '{}' already exists.".format(new_project_name))
         activity = CopyActivity(self.remote_store.data_service, project, new_project_name)
         self._download_project(activity, project.id, temp_directory, path_filter)
         self._upload_project(activity, new_project_name, temp_directory)
diff --git a/ddsc/core/ddsapi.py b/ddsc/core/ddsapi.py
@@ -7,6 +7,7 @@
 from ddsc.config import get_user_config_filename
 from ddsc.versioncheck import APP_NAME, get_internal_version_str
 from ddsc.core.retry import RetrySettings
+from ddsc.exceptions import DDSUserException
 
 AUTH_TOKEN_CLOCK_SKEW_MAX = 5 * 60  # 5 minutes
 SETUP_GUIDE_URL = "https://github.com/Duke-GCB/DukeDSClient/blob/master/docs/GettingAgentAndUserKeys.md"
@@ -1278,7 +1279,7 @@ class ActivityRelationTypes(object):
     WAS_INVALIDATED_BY = "was_invalidated_by"
 
 
-class AuthTokenException(Exception):
+class AuthTokenException(DDSUserException):
     def __init__(self, message):
         super(AuthTokenException, self).__init__(message)
 
diff --git a/ddsc/core/fileuploader.py b/ddsc/core/fileuploader.py
@@ -9,6 +9,7 @@
 from ddsc.core.util import ProgressQueue, wait_for_processes
 from ddsc.core.localstore import HashData
 from ddsc.core.retry import RetrySettings
+from ddsc.exceptions import DDSUserException
 import traceback
 import sys
 import time
@@ -260,8 +261,8 @@ def _verify_upload_response(upload_response):
         if file_chunks:
             total_chunk_size = sum([chunk['size'] for chunk in file_chunks])
             if file_size != total_chunk_size:
-                raise ValueError("Failure uploading {}. Size mismatch file: {} vs chunks:{}."
-                                 "\nPlease retry uploading.".format(file_name, file_size, total_chunk_size))
+                raise DDSUserException("Failure uploading {}. Size mismatch file: {} vs chunks:{}."
+                                       "\nPlease retry uploading.".format(file_name, file_size, total_chunk_size))
 
 
 class ParallelChunkProcessor(object):
diff --git a/ddsc/core/moveutil.py b/ddsc/core/moveutil.py
@@ -1,5 +1,6 @@
 import os
 from ddsc.core.util import KindType
+from ddsc.exceptions import DDSUserException
 
 
 class MoveUtil(object):
@@ -24,7 +25,7 @@ def get_new_parent(self):
             if self.is_folder_or_project(target):
                 return target
             else:
-                raise ValueError("Cannot move to existing file {}.".format(self.target_remote_path))
+                raise DDSUserException("Cannot move to existing file {}.".format(self.target_remote_path))
         else:
             source_parent_remote_path = os.path.dirname(self.source_remote_path)
             target_parent_remote_path = os.path.dirname(self.target_remote_path)
@@ -34,9 +35,9 @@ def get_new_parent(self):
                     if self.is_folder_or_project(target_parent):
                         return target_parent
                     else:
-                        raise ValueError("Target parent {} is a file.".format(target_parent_remote_path))
+                        raise DDSUserException("Target parent {} is a file.".format(target_parent_remote_path))
                 else:
-                    raise ValueError("Target parent directory {} does not exist.".format(target_parent_remote_path))
+                    raise DDSUserException("Target parent directory {} does not exist.".format(target_parent_remote_path))
         return None
 
     def get_new_name(self):
diff --git a/ddsc/core/remotestore.py b/ddsc/core/remotestore.py
@@ -3,6 +3,7 @@
 from ddsc.core.util import KindType, REMOTE_PATH_SEP, RemotePath
 from ddsc.core.localstore import HashUtil
 from ddsc.core.userutil import UserUtil, DUKE_EMAIL_SUFFIX
+from ddsc.exceptions import DDSUserException
 
 FETCH_ALL_USERS_PAGE_SIZE = 25
 DOWNLOAD_FILE_CHUNK_SIZE = 20 * 1024 * 1024
@@ -110,7 +111,7 @@ def lookup_user_by_name(self, full_name):
         if found_cnt == 0:
             raise NotFoundError("User not found:" + full_name)
         elif found_cnt > 1:
-            raise ValueError("Multiple users with name:" + full_name)
+            raise DDSUserException("Multiple users with name:" + full_name)
         user = RemoteUser(results[0])
         if user.full_name.lower() != full_name.lower():
             raise NotFoundError("User not found:" + full_name)
@@ -142,7 +143,7 @@ def get_or_register_user_by_email(self, email):
             if affiliate:
                 user_json = util.register_user_by_username(affiliate['uid'])
             else:
-                raise ValueError("Unable to find or register a user with email {}".format(email))
+                raise DDSUserException("Unable to find or register a user with email {}".format(email))
         return RemoteUser(user_json)
 
     def get_auth_providers(self):
@@ -272,7 +273,7 @@ def delete_project(self, project_name_or_id):
         if project:
             self.data_service.delete_project(project.id)
         else:
-            raise ValueError("No project with {} found.\n".format(project_name_or_id.description()))
+            raise DDSUserException("No project with {} found.\n".format(project_name_or_id.description()))
 
     def get_active_auth_roles(self, context):
         """
@@ -435,7 +436,7 @@ def get_upload_from_json(json_data):
             if 'upload' in json_data:
                 return json_data['upload']
             else:
-                raise ValueError("Invalid file json data, unable to find upload.")
+                raise DDSUserException("Invalid file json data, unable to find upload.")
 
     @staticmethod
     def get_hash_from_upload(upload, target_algorithm=HashUtil.HASH_NAME):
@@ -572,7 +573,7 @@ def __init__(self, json_data):
         self.login_initiation_url = json_data['login_initiation_url']
 
 
-class NotFoundError(Exception):
+class NotFoundError(DDSUserException):
     def __init__(self, message):
         Exception.__init__(self, message)
         self.message = message
diff --git a/ddsc/core/tests/test_fileuploader.py b/ddsc/core/tests/test_fileuploader.py
@@ -2,6 +2,7 @@
 from ddsc.core.fileuploader import ParallelChunkProcessor, upload_async, FileUploadOperations, \
     RetrySettings, ForbiddenSendExternalException, ChunkSender, FileUploader
 from ddsc.core.ddsapi import DSResourceNotConsistentError, DataServiceError
+from ddsc.exceptions import DDSUserException
 import requests
 from mock import MagicMock, Mock, patch, call, ANY
 
@@ -188,7 +189,7 @@ def test_finish_upload_with_bad_chunks(self):
             'chunks': [{'size': 50}, {'size': 50}]
         }
         fop = FileUploadOperations(data_service, MagicMock())
-        with self.assertRaises(ValueError) as raised_exception:
+        with self.assertRaises(DDSUserException) as raised_exception:
             fop.finish_upload(upload_id="123",
                               hash_data=MagicMock(),
                               parent_data=MagicMock(),
diff --git a/ddsc/core/tests/test_moveutil.py b/ddsc/core/tests/test_moveutil.py
@@ -1,6 +1,7 @@
 from __future__ import absolute_import
 from unittest import TestCase
 from ddsc.core.moveutil import MoveUtil, KindType
+from ddsc.exceptions import DDSUserException
 from mock import Mock
 
 
@@ -49,7 +50,7 @@ def test_get_new_parent__target_is_file(self):
         move_util.is_folder_or_project.return_value = False
         mock_folder = Mock()
         mock_project.try_get_item_for_path.return_value = mock_folder
-        with self.assertRaises(ValueError) as raised_exception:
+        with self.assertRaises(DDSUserException) as raised_exception:
             move_util.get_new_parent()
         self.assertEqual(str(raised_exception.exception), 'Cannot move to existing file data/file2.txt.')
 
@@ -79,7 +80,7 @@ def test_get_new_parent__target_parent_file(self):
             None,
             mock_parent_folder
         ]
-        with self.assertRaises(ValueError) as raised_exception:
+        with self.assertRaises(DDSUserException) as raised_exception:
             move_util.get_new_parent()
         self.assertEqual(str(raised_exception.exception), 'Target parent /data2/file1.txt is a file.')
 
@@ -90,7 +91,7 @@ def test_get_new_parent__target_parent_not_found(self):
             None,
             None
         ]
-        with self.assertRaises(ValueError) as raised_exception:
+        with self.assertRaises(DDSUserException) as raised_exception:
             move_util.get_new_parent()
         self.assertEqual(str(raised_exception.exception), 'Target parent directory /data2 does not exist.')
 
diff --git a/ddsc/core/tests/test_remotestore.py b/ddsc/core/tests/test_remotestore.py
@@ -10,6 +10,7 @@
 from ddsc.core.remotestore import RemoteAuthProvider
 from ddsc.core.remotestore import ProjectNameOrId
 from ddsc.core.remotestore import ProjectFile, RemoteFileUrl
+from ddsc.exceptions import DDSUserException
 
 
 class TestProjectFolderFile(TestCase):
@@ -664,7 +665,7 @@ def test_get_or_register_user_by_email__unable_to_find_user(self, mock_remote_us
         mock_user_util.return_value.find_affiliate_by_email.return_value = None
 
         remote_store = RemoteStore(config=MagicMock())
-        with self.assertRaises(ValueError) as raised_exception:
+        with self.assertRaises(DDSUserException) as raised_exception:
             remote_store.get_or_register_user_by_email("user@user.user")
         self.assertEqual(str(raised_exception.exception), 'Unable to find or register a user with email user@user.user')
         mock_util = mock_user_util.return_value
diff --git a/ddsc/core/tests/test_userutil.py b/ddsc/core/tests/test_userutil.py
@@ -1,6 +1,7 @@
 from unittest import TestCase
 from mock import Mock
 from ddsc.core.userutil import UserUtil
+from ddsc.exceptions import DDSUserException
 
 
 class UserUtilTestCase(TestCase):
@@ -126,6 +127,6 @@ def test__get_single_user_or_none(self):
 
         # When multiple found raise exception
         response.json.return_value = {"results": [{"id": "123"}, {"id": "456"}]}
-        with self.assertRaises(ValueError) as raised_exception:
+        with self.assertRaises(DDSUserException) as raised_exception:
             self.user_util._get_single_user_or_none(response, lookup_value="fakeuser@duke.edu")
         self.assertEqual(str(raised_exception.exception), 'Found multiple users for fakeuser@duke.edu.')
diff --git a/ddsc/core/tests/test_util.py b/ddsc/core/tests/test_util.py
@@ -2,6 +2,7 @@
 
 from ddsc.core.util import verify_terminal_encoding, ProgressBar, ProgressPrinter, KindType, RemotePath, humanize_bytes,\
     plural_fmt, join_with_commas_and_and
+from ddsc.exceptions import DDSUserException
 from mock import patch, Mock
 
 
@@ -14,7 +15,7 @@ def test_verify_terminal_encoding_lower(self):
         verify_terminal_encoding('utf')
 
     def test_verify_terminal_encoding_ascii_raises(self):
-        with self.assertRaises(ValueError):
+        with self.assertRaises(DDSUserException):
             verify_terminal_encoding('ascii')
 
     def test_verify_terminal_encoding_empty_is_ok(self):
diff --git a/ddsc/core/userutil.py b/ddsc/core/userutil.py
@@ -1,4 +1,5 @@
 import logging
+from ddsc.exceptions import DDSUserException
 
 DUKE_EMAIL_SUFFIX = "@duke.edu"
 
@@ -56,4 +57,4 @@ def _get_single_user_or_none(response, lookup_value):
             return None
         if len(results) == 1:
             return results[0]
-        raise ValueError("Found multiple users for {}.".format(lookup_value))
+        raise DDSUserException("Found multiple users for {}.".format(lookup_value))
diff --git a/ddsc/core/util.py b/ddsc/core/util.py
@@ -3,6 +3,7 @@
 import platform
 import stat
 import time
+from ddsc.exceptions import DDSUserException
 
 TERMINAL_ENCODING_NOT_UTF_ERROR = """
 ERROR: DukeDSClient requires UTF terminal encoding.
@@ -364,7 +365,7 @@ def wait_for_processes(processes, size, progress_queue, watcher, item):
             error_message = value
             for process in processes:
                 process.terminate()
-            raise ValueError(error_message)
+            raise DDSUserException(error_message)
     for process in processes:
         process.join()
 
@@ -375,7 +376,7 @@ def verify_terminal_encoding(encoding):
     :param encoding: str: encoding we want to check
     """
     if encoding and not ("UTF" in encoding.upper()):
-        raise ValueError(TERMINAL_ENCODING_NOT_UTF_ERROR)
+        raise DDSUserException(TERMINAL_ENCODING_NOT_UTF_ERROR)
 
 
 def verify_file_private(filename):
@@ -388,7 +389,7 @@ def verify_file_private(filename):
         if os.path.exists(filename):
             file_stat = os.stat(filename)
             if mode_allows_group_or_other(file_stat.st_mode):
-                raise ValueError(CONFIG_FILE_PERMISSIONS_ERROR)
+                raise DDSUserException(CONFIG_FILE_PERMISSIONS_ERROR)
 
 
 def mode_allows_group_or_other(st_mode):
diff --git a/ddsc/ddsclient.py b/ddsc/ddsclient.py
@@ -16,6 +16,7 @@
 from ddsc.config import create_config
 from ddsc.sdk.client import Client
 from ddsc.core.download import ProjectFileDownloader
+from ddsc.exceptions import DDSUserException
 
 
 NO_PROJECTS_FOUND_MESSAGE = 'No projects found.'
@@ -365,7 +366,7 @@ def run(self, args):
             new_project_name = self.get_new_project_name(project.name)
         to_user = self.remote_store.lookup_or_register_user_by_email_or_username(email, username)
         if to_user.id in [share_user.id for share_user in share_users]:
-            raise ValueError(INVALID_DELIVERY_RECIPIENT_MSG)
+            raise DDSUserException(INVALID_DELIVERY_RECIPIENT_MSG)
         try:
             path_filter = PathFilter(args.include_paths, args.exclude_paths)
             dest_email = self.service.deliver(project, new_project_name, to_user, share_users,
diff --git a/ddsc/exceptions.py b/ddsc/exceptions.py
@@ -0,0 +1,5 @@
+class DDSUserException(Exception):
+    """
+    Exception with an error message to be displayed to the user on the command line.
+    """
+    pass
diff --git a/ddsc/sdk/client.py b/ddsc/sdk/client.py
@@ -8,6 +8,7 @@
 from ddsc.core.download import FileDownloadState, download_file
 from ddsc.core.util import KindType, REMOTE_PATH_SEP, humanize_bytes, plural_fmt
 from ddsc.core.moveutil import MoveUtil
+from ddsc.exceptions import DDSUserException
 from future.utils import python_2_unicode_compatible
 
 
@@ -50,7 +51,7 @@ def get_project_by_name(self, project_name):
         if not projects:
             raise ItemNotFound("No project named {} found.".format(project_name))
         if len(projects) != 1:
-            raise ValueError("Multiple projects found with name {}.".format(project_name))
+            raise DDSUserException("Multiple projects found with name {}.".format(project_name))
         return projects[0]
 
     def create_project(self, name, description):
@@ -751,9 +752,9 @@ def __init__(self, path_data):
         self.kind = KindType.file_str
 
 
-class ItemNotFound(Exception):
+class ItemNotFound(DDSUserException):
     pass
 
 
-class DuplicateNameError(Exception):
+class DuplicateNameError(DDSUserException):
     pass
diff --git a/ddsc/sdk/tests/test_client.py b/ddsc/sdk/tests/test_client.py
diff --git a/ddsc/tests/test_config.py b/ddsc/tests/test_config.py
diff --git a/ddsc/tests/test_ddsclient.py b/ddsc/tests/test_ddsclient.py
diff --git a/ddsc/tests/test_util.py b/ddsc/tests/test_util.py
diff --git a/ddsc/versioncheck.py b/ddsc/versioncheck.py
