Skip to content

Commit 58641b3

Browse files
committed
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
1 parent 9527a4f commit 58641b3

File tree

2 files changed

+51
-6
lines changed

2 files changed

+51
-6
lines changed

git_pw/api.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,11 @@ def _handle_error(
116116
'Server error. Please report this issue to '
117117
'https://github.com/getpatchwork/patchwork'
118118
)
119-
raise
120-
121-
# we make the assumption that all responses will be JSON encoded
122-
if exc.response.status_code == 404:
119+
elif exc.response.status_code == 404:
123120
LOG.error('Resource not found')
124121
else:
125-
LOG.error(exc.response.json())
122+
LOG.error(exc.response.text)
123+
126124
else:
127125
LOG.error(
128126
'Failed to %s resource. Is your configuration '
@@ -131,7 +129,7 @@ def _handle_error(
131129
LOG.error("Use the '--debug' flag for more information")
132130

133131
if CONF.debug:
134-
raise
132+
raise exc
135133
else:
136134
sys.exit(1)
137135

tests/test_api.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from unittest import mock
44

5+
import requests
56
import pytest
67

78
from git_pw import api
@@ -74,6 +75,52 @@ def test_version(mock_server):
7475
assert api.version() == (1, 1)
7576

7677

78+
def test_handle_error__server_error(caplog):
79+
fake_response = mock.MagicMock(autospec=requests.Response)
80+
fake_response.content = b'InternalServerError'
81+
fake_response.status_code = 500
82+
exc = requests.exceptions.RequestException(response=fake_response)
83+
84+
with pytest.raises(SystemExit):
85+
api._handle_error('fetch', exc)
86+
87+
assert 'Server error.' in caplog.text
88+
89+
90+
def test_handle_error__not_found(caplog):
91+
fake_response = mock.MagicMock(autospec=requests.Response)
92+
fake_response.content = b'NotFound'
93+
fake_response.status_code = 404
94+
exc = requests.exceptions.RequestException(response=fake_response)
95+
96+
with pytest.raises(SystemExit):
97+
api._handle_error('fetch', exc)
98+
99+
assert 'Resource not found' in caplog.text
100+
101+
102+
def test_handle_error__other(caplog):
103+
fake_response = mock.MagicMock(autospec=requests.Response)
104+
fake_response.content = b'{"key": "value"}'
105+
fake_response.status_code = 403
106+
fake_response.text = '{"key": "value"}'
107+
exc = requests.exceptions.RequestException(response=fake_response)
108+
109+
with pytest.raises(SystemExit):
110+
api._handle_error('fetch', exc)
111+
112+
assert '{"key": "value"}' in caplog.text
113+
114+
115+
def test_handle_error__no_response(caplog):
116+
exc = requests.exceptions.RequestException()
117+
118+
with pytest.raises(SystemExit):
119+
api._handle_error('fetch', exc)
120+
121+
assert 'Failed to fetch resource.' in caplog.text
122+
123+
77124
@mock.patch.object(api, 'index')
78125
def test_retrieve_filter_ids_too_short(mock_index):
79126
with pytest.raises(SystemExit):

0 commit comments

Comments
 (0)