Skip to content

Commit

Permalink
Don't decode error response bodies
Browse files Browse the repository at this point in the history
It doesn't bring any advantages since we don't do anything special with
the result.

Signed-off-by: Stephen Finucane <[email protected]>
Closes: #75
  • Loading branch information
stephenfin committed Jan 26, 2025
1 parent 9527a4f commit 58641b3
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 6 deletions.
10 changes: 4 additions & 6 deletions git_pw/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 '
Expand All @@ -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)

Expand Down
47 changes: 47 additions & 0 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from unittest import mock

import requests
import pytest

from git_pw import api
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 58641b3

Please sign in to comment.