TDL-24459: Retry on JSON decode error (#66)

* Add JSONDecodeError to retry list

* Add retry on bad response and test

* Change `IntercomBadResponseError` to an `IntercomError`

* make pylint happy

* Version bump to `v2.0.2`

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## 2.0.2
+ * Retry on responses with no JSON [#66](https://github.com/singer-io/tap-intercom/pull/66)
+
 ## 2.0.1
  * Fix schema for Conversations custom_attributes field [#63](https://github.com/singer-io/tap-intercom/pull/63)
 

setup.py

@@ -3,7 +3,7 @@
 from setuptools import setup, find_packages
 
 setup(name='tap-intercom',
-      version='2.0.1',
+      version='2.0.2',
       description='Singer.io tap for extracting data from the Intercom API',
       author='[email protected]',
       classifiers=['Programming Language :: Python :: 3 :: Only'],

tap_intercom/client.py

@@ -1,8 +1,9 @@
 import backoff
 import requests
+import singer
 from requests.exceptions import ConnectionError, Timeout
+from simplejson.scanner import JSONDecodeError
 from singer import metrics, utils
-import singer
 
 LOGGER = singer.get_logger()
 
@@ -26,6 +27,10 @@ class IntercomBadRequestError(IntercomError):
     pass
 
 
+class IntercomBadResponseError(IntercomError):
+    pass
+
+
 class IntercomScrollExistsError(IntercomError):
     pass
 
@@ -268,7 +273,7 @@ def check_access_token(self):
     #  https://developers.intercom.com/intercom-api-reference/reference#rate-limiting
     @backoff.on_exception(backoff.expo, Timeout, max_tries=5, factor=2) # Backoff for request timeout
     @backoff.on_exception(backoff.expo,
-                          (Server5xxError, ConnectionError, IntercomRateLimitError, IntercomScrollExistsError),
+                          (Server5xxError, ConnectionError, IntercomBadResponseError, IntercomRateLimitError, IntercomScrollExistsError),
                           max_tries=7,
                           factor=3)
     @utils.ratelimit(1000, 60)
@@ -306,7 +311,11 @@ def request(self, method, path=None, url=None, **kwargs):
         if response.status_code != 200:
             raise_for_error(response)
 
-        return response.json()
+        # Sometimes a 200 status code is returned with no content, which breaks JSON decoding.
+        try:
+            return response.json()
+        except JSONDecodeError as err:
+            raise IntercomBadResponseError from err
 
     def get(self, path, **kwargs):
         return self.request('GET', path=path, **kwargs)

tests/unittests/test_backoff.py

@@ -1,22 +1,31 @@
-from parameterized import parameterized
 import unittest
 from unittest import mock
+
 import requests
-from tap_intercom.client import IntercomClient, Server5xxError, ConnectionError, IntercomRateLimitError, IntercomScrollExistsError
+from parameterized import parameterized
+
+from tap_intercom.client import (ConnectionError, IntercomBadResponseError,
+                                 IntercomClient, IntercomRateLimitError,
+                                 IntercomScrollExistsError, Server5xxError)
+
 
 def get_mock_http_response(status_code, contents):
     """Returns mock rep"""
     response = requests.Response()
     response.status_code = status_code
-    response._content = contents.encode()
+    response._content = contents.encode() if contents else response._content
     return response
 
 
+@mock.patch("time.sleep")
+@mock.patch("requests.Session.request")
+@mock.patch("tap_intercom.client.IntercomClient.check_access_token")
 class TestBackoff(unittest.TestCase):
     """
-    Test cases to verify we backoff 7 times for ConnectionError, 5XX errors, 429 error, 423 error
+    Test cases to verify we backoff 7 times for IntercomBadResponseError, ConnectionError, 5XX errors, 429 error, 423 error
     """
-    client = IntercomClient(config_request_timeout= "", access_token="test_access_token")
+
+    client = IntercomClient(config_request_timeout="", access_token="test_access_token")
     method = 'GET'
     path = 'path'
     url = 'url'
@@ -27,17 +36,20 @@ class TestBackoff(unittest.TestCase):
             ['Server5xx_error_backoff', Server5xxError, None],
             ['423_error_backoff', IntercomScrollExistsError, None],
     ])
-
-    @mock.patch("time.sleep")
-    @mock.patch("requests.Session.request")
-    @mock.patch("tap_intercom.client.IntercomClient.check_access_token")
-    def test_backoff(self, name, test_exception, data, mocked_api_cred, mocked_request, mocked_sleep):
+    def test_backoff(self, mocked_api_cred, mocked_request, mocked_sleep, name, test_exception, data):
         """Test case to verify backoff works as expected"""
-        
+
         mocked_request.side_effect = test_exception('exception')
-        with self.assertRaises(test_exception) as e:
-            response_json = self.client.request(self.method, self.path, self.url)
+        with self.assertRaises(test_exception):
+            self.client.request(self.method, self.path, self.url)
 
         self.assertEqual(mocked_request.call_count, 7)
 
+    def test_backoff_bad_response(self, mocked_api_cred, mocked_request, mocked_sleep):
+        """Test case to verify backoff works on an empty response with a 200 status code"""
 
+        mocked_request.return_value = get_mock_http_response(200, None)
+        with self.assertRaises(IntercomBadResponseError):
+            self.client.request(self.method, self.path, self.url)
+
+        self.assertEqual(mocked_request.call_count, 7)