Raise an exception instead of swallowing an error (#126)

* Craft the perfect storm to get an AttributeError from raise_for_error * removed the code for checking content length Co-authored-by: harshpatel4_crest <[email protected]> Co-authored-by: Harsh <[email protected]>
singer-io · Jun 28, 2021 · 778682b · 778682b
1 parent a513fdd
commit 778682b
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 16 deletions.
diff --git a/tap_github/__init__.py b/tap_github/__init__.py
@@ -165,13 +165,6 @@ def get_bookmark(state, repo, stream_name, bookmark_key, start_date):
     return None
 
 def raise_for_error(resp, source):
-
-    content_length = len(resp.content)
-    if content_length == 0:
-        # There is nothing we can do here since Github has neither sent
-        # us a 2xx response nor a response content.
-        return
-
     error_code = resp.status_code
     try:
         response_json = resp.json()
@@ -212,11 +205,9 @@ def authed_get(source, url, headers={}):
         resp = session.request(method='get', url=url)
         if resp.status_code != 200:
             raise_for_error(resp, source)
-            return None
-        else:
-            timer.tags[metrics.Tag.http_status_code] = resp.status_code
-            rate_throttling(resp)
-            return resp
+        timer.tags[metrics.Tag.http_status_code] = resp.status_code
+        rate_throttling(resp)
+        return resp
 
 def authed_get_all_pages(source, url, headers={}):
     while True:

diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py
@@ -4,12 +4,12 @@
 import requests
 
 class Mockresponse:
-    def __init__(self, status_code, json, raise_error, headers={'X-RateLimit-Remaining': 1}, text=None):
+    def __init__(self, status_code, json, raise_error, headers={'X-RateLimit-Remaining': 1}, text=None, content=None):
         self.status_code = status_code
         self.raise_error = raise_error
         self.text = json
         self.headers = headers
-        self.content = "github"
+        self.content = content if content is not None else 'github'
 
     def raise_for_status(self):
         if not self.raise_error:
@@ -20,11 +20,19 @@ def raise_for_status(self):
     def json(self):
         return self.text
 
-def get_response(status_code, json={}, raise_error=False):
-    return Mockresponse(status_code, json, raise_error)
+def get_response(status_code, json={}, raise_error=False, content=None):
+    return Mockresponse(status_code, json, raise_error, content=content)
 
 @mock.patch("requests.Session.request")
 class TestExceptionHandling(unittest.TestCase):
+    def test_zero_content_length(self, mocked_request):
+        mocked_request.return_value = get_response(400, raise_error = True, content='')
+
+        try:
+            tap_github.authed_get("", "")
+        except tap_github.BadRequestException as e:
+            self.assertEquals(str(e), "HTTP-error-code: 400, Error: The request is missing or has a bad parameter.")
+
     def test_400_error(self, mocked_request):
         mocked_request.return_value = get_response(400, raise_error = True)