Merge pull request #180 from kalikiana/browser_get_json_decode

Catch JSON decoding errors in browser.get

openqa_review/browser.py

@@ -148,15 +148,21 @@ def _get(self, url, as_json=False):  # pragma: no cover
             msg = "Request to {} was not successful: {}".format(url, str(e))
             log.warn(msg)
             raise DownloadError(msg)
-
         try:
             r.raise_for_status()
         except requests.exceptions.HTTPError as e:
             msg = "Request to {} failed: {}".format(url, str(e))
             log.warn(msg)
             raise DownloadError(msg)
+        return self._decode_content(r.content.decode("utf-8"), url, as_json)
 
-        content = r.json() if as_json else r.content.decode("utf8")
+    def _decode_content(self, url, raw, as_json=False):
+        try:
+            content = json.loads(raw) if as_json else raw
+        except json.decoder.JSONDecodeError as e:
+            msg = 'Unable to decode JSON for {}: {} (Content was: "{}")'.format(url, str(e), raw)
+            log.warning(msg)
+            raise DownloadError(msg)
         return content
 
     def json_rpc_get(self, url, method, params, cache=True):

tests/test_openqa_review.py

@@ -22,7 +22,7 @@
 
 sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..")))
 
-from openqa_review.browser import filename_to_url
+from openqa_review.browser import filename_to_url, DownloadError
 from openqa_review import openqa_review  # SUT
 
 
@@ -573,3 +573,20 @@ def test_arch_distinguish():
 
     report = str(openqa_review.generate_report(args))
     assert "ppc64le" in report
+
+
+def test_browser_decode_content():
+    args = cache_test_args_factory()
+    args.include_softfails = True
+    browser = browser_factory(args)
+    url = "http://example.com"
+    json = '{"spam": "eggs"}'
+
+    content = browser._decode_content(url, json)
+    assert content == json
+    content = browser._decode_content(url, json, as_json=True)
+    assert "spam" in content
+
+    with pytest.raises(DownloadError) as e:
+        browser._decode_content("http://example.com", "", as_json=True)
+        assert "Unable to decode JSON" in str(e)