fix(pygitguardian): accept non-json content and load as detail

Author: jguerreiro

pygitguardian/__init__.py

@@ -2,7 +2,7 @@
 from .client import GGClient
 
 
-__version__ = "1.1.1"
+__version__ = "1.1.2"
 GGClient._version = __version__
 
 __all__ = [

pygitguardian/client.py

@@ -2,7 +2,6 @@
 import urllib.parse
 from typing import Dict, List, Optional, Union
 
-import requests
 from requests import Response, Session, codes
 
 from .config import (
@@ -14,9 +13,38 @@
 from .models import Detail, Document, MultiScanResult, ScanResult
 
 
+def load_detail(resp: Response) -> Detail:
+    """
+    load_detail loads a Detail from a response
+    be it JSON or html.
+
+    :param resp: API response
+    :type resp: Response
+    :return: detail object of response
+    :rtype: Detail
+    """
+    if resp.headers["content-type"] == "application/json":
+        data = resp.json()
+    else:
+        data = {"detail": resp.text}
+
+    return Detail.SCHEMA.load(data)
+
+
+def is_ok(resp: Response) -> bool:
+    """
+    is_ok returns True is the API responded with 200
+    and the content type is JSON.
+    """
+    return (
+        resp.headers["content-type"] == "application/json"
+        and resp.status_code == codes.ok
+    )
+
+
 class GGClient:
     _version = "undefined"
-    session: requests.Session
+    session: Session
     api_key: str
     base_uri: str
     timeout: Optional[float]
@@ -26,7 +54,7 @@ def __init__(
         self,
         api_key: str,
         base_uri: Optional[str] = None,
-        session: Optional[requests.Session] = None,
+        session: Optional[Session] = None,
         user_agent: Optional[str] = None,
         timeout: Optional[float] = DEFAULT_TIMEOUT,
     ):
@@ -51,7 +79,7 @@ def __init__(
 
         self.base_uri = base_uri
         self.api_key = api_key
-        self.session = session if isinstance(session, Session) else requests.Session()
+        self.session = session if isinstance(session, Session) else Session()
         self.timeout = timeout
         self.user_agent = "pygitguardian/{0} ({1};py{2})".format(
             self._version, platform.system(), platform.python_version()
@@ -79,15 +107,10 @@ def request(
 
         url = urllib.parse.urljoin(self.base_uri, endpoint)
 
-        resp = self.session.request(
+        return self.session.request(
             method=method, url=url, timeout=self.timeout, **kwargs
         )
 
-        if resp.headers["content-type"] != "application/json":
-            raise TypeError("Response is not JSON")
-
-        return resp
-
     def post(
         self,
         endpoint: str,
@@ -115,7 +138,7 @@ def health_check(self) -> Detail:
         """
         resp = self.get(endpoint="health")
 
-        obj = Detail.SCHEMA.load(resp.json())
+        obj = load_detail(resp)
         obj.status_code = resp.status_code
 
         return obj
@@ -138,10 +161,10 @@ def content_scan(
         request_obj = Document.SCHEMA.load(doc_dict)
 
         resp = self.post(endpoint="scan", data=request_obj)
-        if resp.status_code == codes.ok:
+        if is_ok(resp):
             obj = ScanResult.SCHEMA.load(resp.json())
         else:
-            obj = Detail.SCHEMA.load(resp.json())
+            obj = load_detail(resp)
 
         obj.status_code = resp.status_code
 
@@ -173,10 +196,10 @@ def multi_content_scan(
 
         resp = self.post(endpoint="multiscan", data=request_obj)
 
-        if resp.status_code == codes.ok:
+        if is_ok(resp):
             obj = MultiScanResult.SCHEMA.load(dict(scan_results=resp.json()))
         else:
-            obj = Detail.SCHEMA.load(resp.json())
+            obj = load_detail(resp)
 
         obj.status_code = resp.status_code
 

tests/test_client.py

@@ -1,11 +1,12 @@
 import json
-from typing import Dict, List
+from typing import Dict, List, Type
 from unittest.mock import patch
 
 import pytest
 from marshmallow import ValidationError
 
 from pygitguardian import GGClient
+from pygitguardian.client import is_ok, load_detail
 from pygitguardian.config import (
     DEFAULT_BASE_URI,
     DOCUMENT_SIZE_THRESHOLD_BYTES,
@@ -183,7 +184,7 @@
     ],
 )
 def test_client_creation(
-    api_key: str, uri: str, user_agent: str, timeout: float, exception: Exception
+    api_key: str, uri: str, user_agent: str, timeout: float, exception: Type[Exception]
 ):
     if exception is not None:
         with pytest.raises(exception):
@@ -299,7 +300,7 @@ def test_multi_content_scan(
     ],
 )
 def test_content_scan_exceptions(
-    client: GGClient, to_scan: str, exception: Exception, regex: str
+    client: GGClient, to_scan: str, exception: Type[Exception], regex: str
 ):
     with pytest.raises(exception, match=regex):
         client.content_scan(to_scan)
@@ -313,7 +314,7 @@ def test_content_scan_exceptions(
     ],
 )
 def test_multi_content_exceptions(
-    client: GGClient, to_scan: List, exception: Exception
+    client: GGClient, to_scan: List, exception: Type[Exception]
 ):
     with pytest.raises(exception):
         client.multi_content_scan(to_scan)
@@ -326,7 +327,7 @@ def test_multi_content_not_ok():
 
     obj = client.multi_content_scan(req)
 
-    assert obj.status_code, 401
+    assert obj.status_code == 401
     assert isinstance(obj, Detail)
     assert obj.detail == "Invalid API key."
 
@@ -338,7 +339,7 @@ def test_content_not_ok():
 
     obj = client.content_scan(**req)
 
-    assert obj.status_code, 401
+    assert obj.status_code == 401
     assert isinstance(obj, Detail)
     assert obj.detail == "Invalid API key."
 
@@ -401,5 +402,17 @@ def test_content_scan(
 
 @my_vcr.use_cassette
 def test_assert_content_type(client: GGClient):
-    with pytest.raises(TypeError):
-        client.get(endpoint="/docs/static/logo.png", version=None)
+    """
+    GIVEN a response that's 200 but the content is not JSON
+    WHEN is_ok is called
+    THEN is_ok should be false
+    WHEN load_detail is called
+    THEN is should return a Detail object
+    """
+    resp = client.get(endpoint="/docs/static/logo.png", version=None)
+    assert is_ok(resp) is False
+    obj = load_detail(resp)
+    obj.status_code = resp.status_code
+    assert obj.status_code == 200
+    assert isinstance(obj, Detail)
+    assert str(obj).startswith("200:"), str(obj)