From 58641b31c7e6332ee7519f7b9feeac57886fd0c8 Mon Sep 17 00:00:00 2001
From: Stephen Finucane <stephen@that.guru>
Date: Sun, 26 Jan 2025 18:46:04 +0000
Subject: [PATCH] Don't decode error response bodies

It doesn't bring any advantages since we don't do anything special with
the result.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Closes: #75
---
 git_pw/api.py     | 10 ++++------
 tests/test_api.py | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/git_pw/api.py b/git_pw/api.py
index fac6854..c62a352 100644
--- a/git_pw/api.py
+++ b/git_pw/api.py
@@ -116,13 +116,11 @@ def _handle_error(
                 'Server error. Please report this issue to '
                 'https://github.com/getpatchwork/patchwork'
             )
-            raise
-
-        # we make the assumption that all responses will be JSON encoded
-        if exc.response.status_code == 404:
+        elif exc.response.status_code == 404:
             LOG.error('Resource not found')
         else:
-            LOG.error(exc.response.json())
+            LOG.error(exc.response.text)
+
     else:
         LOG.error(
             'Failed to %s resource. Is your configuration '
@@ -131,7 +129,7 @@ def _handle_error(
         LOG.error("Use the '--debug' flag for more information")
 
     if CONF.debug:
-        raise
+        raise exc
     else:
         sys.exit(1)
 
diff --git a/tests/test_api.py b/tests/test_api.py
index ffee31a..5eff792 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -2,6 +2,7 @@
 
 from unittest import mock
 
+import requests
 import pytest
 
 from git_pw import api
@@ -74,6 +75,52 @@ def test_version(mock_server):
     assert api.version() == (1, 1)
 
 
+def test_handle_error__server_error(caplog):
+    fake_response = mock.MagicMock(autospec=requests.Response)
+    fake_response.content = b'InternalServerError'
+    fake_response.status_code = 500
+    exc = requests.exceptions.RequestException(response=fake_response)
+
+    with pytest.raises(SystemExit):
+        api._handle_error('fetch', exc)
+
+    assert 'Server error.' in caplog.text
+
+
+def test_handle_error__not_found(caplog):
+    fake_response = mock.MagicMock(autospec=requests.Response)
+    fake_response.content = b'NotFound'
+    fake_response.status_code = 404
+    exc = requests.exceptions.RequestException(response=fake_response)
+
+    with pytest.raises(SystemExit):
+        api._handle_error('fetch', exc)
+
+    assert 'Resource not found' in caplog.text
+
+
+def test_handle_error__other(caplog):
+    fake_response = mock.MagicMock(autospec=requests.Response)
+    fake_response.content = b'{"key": "value"}'
+    fake_response.status_code = 403
+    fake_response.text = '{"key": "value"}'
+    exc = requests.exceptions.RequestException(response=fake_response)
+
+    with pytest.raises(SystemExit):
+        api._handle_error('fetch', exc)
+
+    assert '{"key": "value"}' in caplog.text
+
+
+def test_handle_error__no_response(caplog):
+    exc = requests.exceptions.RequestException()
+
+    with pytest.raises(SystemExit):
+        api._handle_error('fetch', exc)
+
+    assert 'Failed to fetch resource.' in caplog.text
+
+
 @mock.patch.object(api, 'index')
 def test_retrieve_filter_ids_too_short(mock_index):
     with pytest.raises(SystemExit):