Repository: watson-developer-cloud/python-sdk

Issue:
Exceptions don't have enough information for users
As a Developer _I want_ to have `code`, `message`, `description` whenever a Watson service raises an exception _so that_ I can write code that reacts different depending on the exception.

# Context
It seems like the Python exceptions this SDK is throwing are not very useful for developers:
> **Bruce Adams:**
> I do not have a strong opinion about exception classes. I do find it irritating that `WatsonException` takes a structured error response from as service call and flattens it into a string as the exception args. When I want to make decisions based on the error code (my code needs to react differently to 429 compared to a 500), I have to pick the code out of the string.


More info:
 * https://stackoverflow.com/questions/256222/which-exception-should-i-raise-on-bad-illegal-argument-combinations-in-python
 * https://www.youtube.com/watch?v=o9pEzgHorH0
## Acceptance Criteria
- [ ] Switch to use `ValueError` when there is a missing or illegal argument
- [ ] Parse the error parameters and make them available to users (`code`, `description`)

Discussion:

Patch:
diff --git a/examples/conversation_tone_analyzer_integration/__init__.py b/examples/conversation_tone_analyzer_integration/__init__.py
index c4d6c44a..dc36e221 100644
--- a/examples/conversation_tone_analyzer_integration/__init__.py
+++ b/examples/conversation_tone_analyzer_integration/__init__.py
@@ -14,7 +14,6 @@
 
 from .watson_service import WatsonService
 from .watson_service import WatsonException
-from .watson_service import WatsonInvalidArgument
 from .conversation_v1 import ConversationV1
 from .tone_analyzer_v3 import ToneAnalyzerV3
 
diff --git a/examples/natural_language_classifier_v1.py b/examples/natural_language_classifier_v1.py
index 745b6bec..97554fa3 100644
--- a/examples/natural_language_classifier_v1.py
+++ b/examples/natural_language_classifier_v1.py
@@ -33,7 +33,7 @@ if status['status'] == 'Available':
 # delete = natural_language_classifier.remove('2374f9x68-nlc-2697')
 # print(json.dumps(delete, indent=2))
 
-# example of raising a WatsonException
+# example of raising a ValueError
 # print(json.dumps(
 #     natural_language_classifier.create(training_data='', name='weather3'),
 #     indent=2))
diff --git a/watson_developer_cloud/__init__.py b/watson_developer_cloud/__init__.py
index 925fcb8d..b25873e5 100755
--- a/watson_developer_cloud/__init__.py
+++ b/watson_developer_cloud/__init__.py
@@ -14,6 +14,7 @@
 
 from .watson_service import WatsonService
 from .watson_service import WatsonException
+from .watson_service import WatsonApiException
 from .watson_service import WatsonInvalidArgument
 from .alchemy_data_news_v1 import AlchemyDataNewsV1
 from .alchemy_language_v1 import AlchemyLanguageV1
diff --git a/watson_developer_cloud/language_translation_v2.py b/watson_developer_cloud/language_translation_v2.py
index 7829d96c..751b965e 100644
--- a/watson_developer_cloud/language_translation_v2.py
+++ b/watson_developer_cloud/language_translation_v2.py
@@ -19,7 +19,6 @@ The v2 Language Translation service
 
 from __future__ import print_function
 from .watson_service import WatsonService
-from .watson_service import WatsonInvalidArgument
 
 
 class LanguageTranslationV2(WatsonService):
@@ -56,8 +55,7 @@ class LanguageTranslationV2(WatsonService):
                      monolingual_corpus=None):
         if forced_glossary is None and parallel_corpus is None and \
                         monolingual_corpus is None:
-            raise WatsonInvalidArgument(
-                'A glossary or corpus must be provided')
+            raise ValueError('A glossary or corpus must be provided')
         params = {'name': name,
                   'base_model_id': base_model_id}
         files = {'forced_glossary': forced_glossary,
@@ -80,7 +78,7 @@ class LanguageTranslationV2(WatsonService):
         Translates text from a source language to a target language
         """
         if model_id is None and (source is None or target is None):
-            raise WatsonInvalidArgument(
+            raise ValueError(
                 'Either model_id or source and target must be specified')
 
         data = {'text': text, 'source': source, 'target': target,
diff --git a/watson_developer_cloud/watson_service.py b/watson_developer_cloud/watson_service.py
index 5c938b23..83eeaaf6 100755
--- a/watson_developer_cloud/watson_service.py
+++ b/watson_developer_cloud/watson_service.py
@@ -45,9 +45,31 @@ def load_from_vcap_services(service_name):
 
 
 class WatsonException(Exception):
+    """
+    Custom exception class for Watson Services.
+    """
     pass
 
 
+class WatsonApiException(WatsonException):
+    """
+    Custom exception class for errors returned from Watson APIs.
+
+    :param int code: The HTTP status code returned.
+    :param str message: A message describing the error.
+    :param dict info: A dictionary of additional information about the error.
+    """
+    def __init__(self, code, message, info=None):
+        # Call the base class constructor with the parameters it needs
+        super(WatsonApiException, self).__init__(message)
+        self.message = message
+        self.code = code
+        self.info = info
+
+    def __str__(self):
+        return 'Error: ' + self.message + ', Code: ' + str(self.code)
+
+
 class WatsonInvalidArgument(WatsonException):
     pass
 
@@ -115,7 +137,7 @@ class WatsonService(object):
 
         if api_key is not None:
             if username is not None or password is not None:
-                raise WatsonInvalidArgument(
+                raise ValueError(
                     'Cannot set api_key and username and password together')
             self.set_api_key(api_key)
         else:
@@ -138,10 +160,9 @@ class WatsonService(object):
 
         if (self.username is None or self.password is None)\
                 and self.api_key is None:
-            raise WatsonException(
+            raise ValueError(
                 'You must specify your username and password service '
-                'credentials ' +
-                '(Note: these are different from your Bluemix id)')
+                'credentials (Note: these are different from your Bluemix id)')
 
     def set_username_and_password(self, username=None, password=None):
         if username == 'YOUR SERVICE USERNAME':
@@ -171,7 +192,7 @@ class WatsonService(object):
         if isinstance(headers, dict):
             self.default_headers = headers
         else:
-            raise WatsonException("headers parameter must be a dictionary")
+            raise TypeError("headers parameter must be a dictionary")
 
     # Could make this compute the label_id based on the variable name of the
     # dictionary passed in (using **kwargs), but
@@ -192,10 +213,8 @@ class WatsonService(object):
     def _get_error_message(response):
         """
         Gets the error message from a JSON response.
-        {
-            code: 400
-            error: 'Bad request'
-        }
+        :return: the error message
+        :rtype: string
         """
         error_message = 'Unknown error'
         try:
@@ -203,22 +222,36 @@ class WatsonService(object):
             if 'error' in error_json:
                 if isinstance(error_json['error'], dict) and 'description' in \
                         error_json['error']:
-                    error_message = 'Error: ' + error_json['error'][
-                        'description']
+                    error_message = error_json['error']['description']
                 else:
-                    error_message = 'Error: ' + error_json['error']
+                    error_message = error_json['error']
             elif 'error_message' in error_json:
-                error_message = 'Error: ' + error_json['error_message']
+                error_message = error_json['error_message']
             elif 'msg' in error_json:
-                error_message = 'Error: ' + error_json['msg']
+                error_message = error_json['msg']
             elif 'statusInfo' in error_json:
-                error_message = 'Error: ' + error_json['statusInfo']
-            if 'description' in error_json:
-                error_message += ', Description: ' + error_json['description']
-            error_message += ', Code: ' + str(response.status_code)
+                error_message = error_json['statusInfo']
             return error_message
         except:
-            return {'error': response.text or error_message, 'code': str(response.status_code)}
+            return response.text or error_message
+
+
+    @staticmethod
+    def _get_error_info(response):
+        """
+        Gets the error info (if any) from a JSON response.
+        :return: A `dict` containing additional information about the error.
+        :rtype: dict
+        """
+        info_keys = ['code_description', 'description', 'errors', 'help',
+                     'sub_code', 'warnings']
+        error_info = {}
+        try:
+            error_json = response.json()
+            error_info = {k:v for k, v in error_json.items() if k in info_keys}
+        except:
+            pass
+        return error_info if any(error_info) else None
 
 
     def _alchemy_html_request(self, method_name=None, url=None, html=None,
@@ -333,13 +366,14 @@ class WatsonService(object):
                 response_json = response.json()
                 if 'status' in response_json and response_json['status'] \
                         == 'ERROR':
-                    response.status_code = 400
+                    status_code = 400
                     error_message = 'Unknown error'
+
                     if 'statusInfo' in response_json:
                         error_message = response_json['statusInfo']
                     if error_message == 'invalid-api-key':
-                        response.status_code = 401
-                    raise WatsonException('Error: ' + error_message)
+                        status_code = 401
+                    raise WatsonApiException(status_code, error_message)
                 return response_json
             return response
         else:
@@ -348,4 +382,6 @@ class WatsonService(object):
                                 'invalid credentials '
             else:
                 error_message = self._get_error_message(response)
-            raise WatsonException(error_message)
+            error_info = self._get_error_info(response)
+            raise WatsonApiException(response.status_code, error_message,
+                                     error_info)


Test patch:
diff --git a/test/test_conversation_v1.py b/test/test_conversation_v1.py
index 03dc4eb7..b4f339f0 100644
--- a/test/test_conversation_v1.py
+++ b/test/test_conversation_v1.py
@@ -16,6 +16,7 @@ import json
 import responses
 import watson_developer_cloud
 from watson_developer_cloud import WatsonException
+from watson_developer_cloud import WatsonApiException
 from watson_developer_cloud.conversation_v1 import *
 
 platform_url = 'https://gateway.watsonplatform.net'
@@ -56,13 +57,13 @@ def test_create_counterexample():
 def test_rate_limit_exceeded():
     endpoint = '/v1/workspaces/{0}/counterexamples'.format('boguswid')
     url = '{0}{1}'.format(base_url, endpoint)
-    error_code = "'code': '407'"
+    error_code = 429
     error_msg = 'Rate limit exceeded'
     responses.add(
         responses.POST,
         url,
         body='Rate limit exceeded',
-        status=407,
+        status=429,
         content_type='application/json')
     service = watson_developer_cloud.ConversationV1(
         username='username', password='password', version='2017-02-03')
@@ -71,7 +72,8 @@ def test_rate_limit_exceeded():
             workspace_id='boguswid', text='I want financial advice today.')
     except WatsonException as ex:
         assert len(responses.calls) == 1
-        assert error_code in str(ex)
+        assert isinstance(ex, WatsonApiException)
+        assert error_code == ex.code
         assert error_msg in str(ex)
 
 @responses.activate
diff --git a/test/test_natural_language_understanding.py b/test/test_natural_language_understanding.py
index 97775a64..c59d4eb0 100644
--- a/test/test_natural_language_understanding.py
+++ b/test/test_natural_language_understanding.py
@@ -68,9 +68,9 @@ class TestNaturalLanguageUnderstanding(TestCase):
     @pytest.mark.skipif(os.getenv('VCAP_SERVICES') is not None,
                     reason='credentials may come from VCAP_SERVICES')
     def test_missing_credentials(self):
-        with pytest.raises(WatsonException):
+        with pytest.raises(ValueError):
             NaturalLanguageUnderstandingV1(version='2016-01-23')
-        with pytest.raises(WatsonException):
+        with pytest.raises(ValueError):
             NaturalLanguageUnderstandingV1(version='2016-01-23',
                                           url='https://bogus.com')
 
diff --git a/test/test_tone_analyzer_v3.py b/test/test_tone_analyzer_v3.py
index b925c2f2..e8f978a0 100755
--- a/test/test_tone_analyzer_v3.py
+++ b/test/test_tone_analyzer_v3.py
@@ -1,6 +1,9 @@
 import responses
 import watson_developer_cloud
+from watson_developer_cloud import WatsonException
+from watson_developer_cloud import WatsonApiException
 import os
+import json
 
 
 @responses.activate
@@ -104,3 +107,40 @@ def test_tone_chat():
     assert responses.calls[0].request.url == tone_url + tone_args
     assert responses.calls[0].response.text == tone_response
     assert len(responses.calls) == 1
+
+
+#########################
+# error response
+#########################
+
+
+@responses.activate
+def test_error():
+    tone_url = 'https://gateway.watsonplatform.net/tone-analyzer/api/v3/tone'
+    tone_args = '?version=2016-05-19'
+    error_code = 400
+    error_message = "Invalid JSON input at line 2, column 12"
+    tone_response = {
+            "code": error_code,
+            "sub_code": "C00012",
+            "error": error_message
+        }
+    responses.add(responses.POST,
+                  tone_url,
+                  body=json.dumps(tone_response),
+                  status=error_code,
+                  content_type='application/json')
+
+    tone_analyzer = watson_developer_cloud.ToneAnalyzerV3("2016-05-19",
+            username="username", password="password")
+    text = "Team, I know that times are tough!"
+    try:
+        tone_analyzer.tone(text)
+    except WatsonException as ex:
+        assert len(responses.calls) == 1
+        assert isinstance(ex, WatsonApiException)
+        assert ex.code == error_code
+        assert ex.message == error_message
+        assert len(ex.info) == 1
+        assert ex.info['sub_code'] == 'C00012'
+