TDL-20359 Add custom exception handling.

README.md

@@ -36,11 +36,13 @@ This tap:
     Create a JSON file called `config.json` containing the api token you just found and
     the subdomain to your Freshdesk account. The subdomain will take the format
     `subdomain.freshdesk.com`.
+    The `request_timeout` is an optional parameter to set the timeout for requests. Default: 300 seconds
 
     ```json
     {
       "api_key": "your-api-token",
       "domain": "subdomain",
+      "request_timeout": 300,
       "start_date": "2017-01-17T20:32:05Z"
     }
     ```

tap_freshdesk/client.py

@@ -3,11 +3,115 @@
 import backoff
 import requests
 import singer
+from simplejson import JSONDecodeError
 from tap_freshdesk import utils
 
 LOGGER = singer.get_logger()
 BASE_URL = "https://{}.freshdesk.com"
+REQUEST_TIMEOUT = 300
 
+class FreshdeskException(Exception):
+    pass
+
+class FreshdeskValidationError(FreshdeskException):
+    pass
+
+class FreshdeskAuthenticationError(FreshdeskException):
+    pass
+
+class FreshdeskAccessDeniedError(FreshdeskException):
+    pass
+
+class FreshdeskNotFoundError(FreshdeskException):
+    pass
+
+class FreshdeskMethodNotAllowedError(FreshdeskException):
+    pass
+
+class FreshdeskUnsupportedAcceptHeaderError(FreshdeskException):
+    pass
+
+class FreshdeskConflictingStateError(FreshdeskException):
+    pass
+
+class FreshdeskUnsupportedContentError(FreshdeskException):
+    pass
+
+class FreshdeskRateLimitError(FreshdeskException):
+    pass
+
+class Server5xxError(FreshdeskException):
+    pass
+
+class FreshdeskServerError(Server5xxError):
+    pass
+
+
+ERROR_CODE_EXCEPTION_MAPPING = {
+    400: {
+        "raise_exception": FreshdeskValidationError,
+        "message": "The request body/query string is not in the correct format."
+    },
+    401: {
+        "raise_exception": FreshdeskAuthenticationError,
+        "message": "The Authorization header is either missing or incorrect."
+    },
+    403: {
+        "raise_exception": FreshdeskAccessDeniedError,
+        "message": "The agent whose credentials were used to make this request was not authorized to perform this API call."
+    },
+    404: {
+        "raise_exception": FreshdeskNotFoundError,
+        "message": "The request contains invalid ID/Freshdesk domain in the URL or an invalid URL itself."
+    },
+    405: {
+        "raise_exception": FreshdeskMethodNotAllowedError,
+        "message": "This API request used the wrong HTTP verb/method."
+    },
+    406: {
+        "raise_exception": FreshdeskUnsupportedAcceptHeaderError,
+        "message": "Only application/json and */* are supported in 'Accepted' header."
+    },
+    409: {
+        "raise_exception": FreshdeskConflictingStateError,
+        "message": "The resource that is being created/updated is in an inconsistent or conflicting state."
+    },
+    415: {
+        "raise_exception": FreshdeskUnsupportedContentError,
+        "message": "Content type application/xml is not supported. Only application/json is supported."
+    },
+    429: {
+        "raise_exception": FreshdeskRateLimitError,
+        "message": "The API rate limit allotted for your Freshdesk domain has been exhausted."
+    },
+    500: {
+        "raise_exception": FreshdeskServerError,
+        "message": "Unexpected Server Error."
+    }
+}
+
+def raise_for_error(response):
+    """
+    Retrieve the error code and the error message from the response and return custom exceptions accordingly.
+    """
+    error_code = response.status_code
+    # Forming a response message for raising a custom exception
+    try:
+        response_json = response.json()
+    except JSONDecodeError:
+        response_json = {}
+
+    if error_code not in ERROR_CODE_EXCEPTION_MAPPING and error_code > 500:
+        # Raise `Server5xxError` for all 5xx unknown error
+        exc = Server5xxError
+    else:
+        exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("raise_exception", FreshdeskException)
+    message = response_json.get("description", ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("message", "Unknown Error"))
+    code = response_json.get("code", "")
+    if code:
+        error_code = f"{str(error_code)} {code}"
+    formatted_message = "HTTP-error-code: {}, Error: {}".format(error_code, message)
+    raise exc(formatted_message) from None
 
 class FreshdeskClient:
     """
@@ -18,6 +122,8 @@ def __init__(self, config):
         self.config = config
         self.session = requests.Session()
         self.base_url = BASE_URL.format(config.get("domain"))
+        self.timeout = REQUEST_TIMEOUT
+        self.set_timeout()
 
     def __enter__(self):
         self.check_access_token()
@@ -27,16 +133,27 @@ def __exit__(self, exception_type, exception_value, traceback):
         # Kill the session instance.
         self.session.close()
 
+    def set_timeout(self):
+        """
+        Set timeout value from config, if the value is passed.
+        Else raise an exception.
+        """
+        timeout = self.config.get("request_timeout", REQUEST_TIMEOUT)
+        if ((type(timeout) in [int, float]) or
+            (isinstance(timeout, str) and timeout.replace('.', '', 1).isdigit())) and float(timeout):
+            self.timeout = float(timeout)
+        else:
+            raise Exception("The entered timeout is invalid, it should be a valid none-zero integer.")
+
     def check_access_token(self):
         """
         Check if the access token is valid.
         """
         self.request(self.base_url+"/api/v2/roles", {"per_page": 1, "page": 1})
 
     @backoff.on_exception(backoff.expo,
-                          (requests.exceptions.RequestException),
+                          (requests.Timeout, requests.ConnectionError, Server5xxError),
                           max_tries=5,
-                          giveup=lambda e: e.response is not None and 400 <= e.response.status_code < 500,
                           factor=2)
     @utils.ratelimit(1, 2)
     def request(self, url, params=None):
@@ -49,7 +166,7 @@ def request(self, url, params=None):
 
         req = requests.Request('GET', url, params=params, auth=(self.config['api_key'], ""), headers=headers).prepare()
         LOGGER.info("GET %s", req.url)
-        response = self.session.send(req)
+        response = self.session.send(req, timeout=self.timeout)
 
         # Call the function again if the rate limit is exceeded
         if 'Retry-After' in response.headers:
@@ -58,6 +175,7 @@ def request(self, url, params=None):
             time.sleep(retry_after)
             return self.request(url, params)
 
-        response.raise_for_status()
+        if response.status_code != 200:
+            raise_for_error(response)
 
         return response.json()

tap_freshdesk/streams.py

@@ -2,6 +2,7 @@
 from datetime import datetime as dt
 import singer
 from singer.bookmarks import get_bookmark
+from tap_freshdesk.client import FreshdeskNotFoundError
 
 
 LOGGER = singer.get_logger()
@@ -109,7 +110,7 @@ def write_records(self, catalog, state, selected_streams, start_date, data, max_
                 stream_metadata = singer.metadata.to_map(stream_catalog['metadata'])
                 for row in data:
                     if self.tap_stream_id in selected_streams and row[self.replication_keys[0]] >= bookmark:
-                        # Custom fields are expected to be strings, but sometimes the API sends 
+                        # Custom fields are expected to be strings, but sometimes the API sends
                         # booleans. We cast those to strings to match the schema.
                         if 'custom_fields' in row:
                             row['custom_fields'] = self.transform_dict(row['custom_fields'], force_str=self.force_str)
@@ -126,7 +127,7 @@ def write_records(self, catalog, state, selected_streams, start_date, data, max_
                             child_obj.parent_id = row['id']
                             child_max_bookmark = get_bookmark(state, child_obj.tap_stream_id, child_obj.replication_keys[0], start_date)
                             # Update the child's max_bookmark as the max of the already present max value and the return value
-                            child_max_bookmark = max(child_max_bookmarks.get(child, child_max_bookmark), child_obj.sync_obj(state, start_date, client, catalog, selected_streams, streams_to_sync))
+                            child_max_bookmark = max(child_max_bookmarks.get(child, child_max_bookmark), child_obj.sync_obj(state, start_date, client, catalog, selected_streams, streams_to_sync) or "")
                             child_max_bookmarks[child] = child_max_bookmark
         return max_bookmark, child_max_bookmarks
 
@@ -301,6 +302,15 @@ class TimeEntries(ChildStream):
     path = 'tickets/{}/time_entries'
     parent = 'tickets'
 
+    def sync_obj(self, state, start_date, client, catalog, selected_streams, streams_to_sync, predefined_filter=None):
+        try:
+            return super().sync_obj(state, start_date, client, catalog, selected_streams, streams_to_sync, predefined_filter)
+        except FreshdeskNotFoundError:
+            # Skipping 404 error as it is returned for deleted tickets and spam
+            LOGGER.warning("Could not retrieve time entries for ticket id %s. This may be caused by tickets "
+                           "marked as spam or deleted.", self.parent_id)
+            return None
+
 
 STREAMS = {
     "agents": Agents,