handle doc dl connection errors correctly

previously we'd see an error message in the logs:
`AttributeError: 'NoneType' object has no attribute 'status_code'`
because we were assuming the requests exception would always have a
response - it won't have a response if it wasn't able to create a
connection at all.
This commit is contained in:
Leo Hemsted
2020-12-23 12:21:24 +00:00
parent 7152ac7cba
commit 325f271e25
2 changed files with 17 additions and 1 deletions

View File

@@ -42,7 +42,9 @@ class DocumentDownloadClient:
# if doc dl responds with a non-400, (eg 403) it's referring to credentials that the API and Doc DL use.
# we don't want to tell users about that, so anything that isn't a 400 (virus scan failed or file type
# unrecognised) should be raised as a 500 internal server error here.
if e.response.status_code == 400:
if e.response is None:
raise Exception(f'Unhandled document download error: {repr(e)}')
elif e.response.status_code == 400:
error = DocumentDownloadError.from_exception(e)
current_app.logger.info(
'Document download request failed with error: {}'.format(error.message)

View File

@@ -1,3 +1,4 @@
import requests
import requests_mock
import pytest
@@ -56,3 +57,16 @@ def test_should_raise_non_400_statuses_as_exceptions(document_download):
assert type(excinfo.value) == Exception # make sure it's a base exception, so will be handled as a 500 by v2 api
assert str(excinfo.value) == 'Unhandled document download error: {"error": "Auth Error Of Some Kind"}'
def test_should_raise_exceptions_without_http_response_bodies_as_exceptions(document_download):
with pytest.raises(Exception) as excinfo, requests_mock.Mocker() as request_mock:
request_mock.post(
'https://document-download/services/service-id/documents',
exc=requests.exceptions.ConnectTimeout
)
document_download.upload_document('service-id', 'abababab')
assert type(excinfo.value) == Exception # make sure it's a base exception, so will be handled as a 500 by v2 api
assert str(excinfo.value) == 'Unhandled document download error: ConnectTimeout()'