diff --git a/app/clients/document_download.py b/app/clients/document_download.py index cb99ac88e..b40184452 100644 --- a/app/clients/document_download.py +++ b/app/clients/document_download.py @@ -2,6 +2,25 @@ import base64 import requests +from flask import current_app + + +class DocumentDownloadError(Exception): + def __init__(self, message, status_code): + self.message = message + self.status_code = status_code + + @classmethod + def from_exception(cls, e): + try: + message = e.response.json()['error'] + status_code = e.response.status_code + except (TypeError, ValueError, AttributeError, KeyError): + message = 'connection error' + status_code = 503 + + return cls(message, status_code) + class DocumentDownloadClient: @@ -13,15 +32,24 @@ class DocumentDownloadClient: return "{}/services/{}/documents".format(self.api_host, service_id) def upload_document(self, service_id, file_contents): - response = requests.post( - self.get_upload_url(service_id), - headers={ - 'Authorization': "Bearer {}".format(self.auth_token), - }, - files={ - 'document': base64.b64decode(file_contents) - } - ) - response.raise_for_status() + try: + response = requests.post( + self.get_upload_url(service_id), + headers={ + 'Authorization': "Bearer {}".format(self.auth_token), + }, + files={ + 'document': base64.b64decode(file_contents) + } + ) + + response.raise_for_status() + except requests.RequestException as e: + error = DocumentDownloadError.from_exception(e) + current_app.logger.warning( + 'Document download request failed with error: {}'.format(error.message) + ) + + raise error return response.json()['document']['url'] diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 998636859..388a78168 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -8,6 +8,7 @@ from notifications_utils.pdf import pdf_page_count, PdfReadError from notifications_utils.recipients import try_validate_and_format_phone_number from app import api_user, authenticated_service, notify_celery, document_download_client +from app.clients.document_download import DocumentDownloadError from app.config import QueueNames, TaskNames from app.dao.notifications_dao import dao_update_notification, update_notification_status_by_reference from app.dao.templates_dao import dao_create_template @@ -231,9 +232,12 @@ def process_document_uploads(personalisation_data, service, simulated=False): if simulated: personalisation_data[key] = document_download_client.get_upload_url(service.id) + '/test-document' else: - personalisation_data[key] = document_download_client.upload_document( - service.id, personalisation_data[key]['file'] - ) + try: + personalisation_data[key] = document_download_client.upload_document( + service.id, personalisation_data[key]['file'] + ) + except DocumentDownloadError as e: + raise BadRequestError(message=e.message, status_code=e.status_code) return personalisation_data diff --git a/tests/app/clients/test_document_download.py b/tests/app/clients/test_document_download.py index e01094192..0d6cc6473 100644 --- a/tests/app/clients/test_document_download.py +++ b/tests/app/clients/test_document_download.py @@ -2,7 +2,7 @@ import requests import requests_mock import pytest -from app.clients.document_download import DocumentDownloadClient +from app.clients.document_download import DocumentDownloadClient, DocumentDownloadError @pytest.fixture(scope='function') @@ -32,6 +32,25 @@ def test_upload_document(document_download): def test_should_raise_for_status(document_download): - with pytest.raises(requests.HTTPError), requests_mock.Mocker() as request_mock: - request_mock.post('https://document-download/services/service-id/documents', json={}, status_code=403) + with pytest.raises(DocumentDownloadError) as excinfo, requests_mock.Mocker() as request_mock: + request_mock.post('https://document-download/services/service-id/documents', json={ + 'error': 'Invalid encoding' + }, status_code=403) + document_download.upload_document('service-id', 'abababab') + + assert excinfo.value.message == 'Invalid encoding' + assert excinfo.value.status_code == 403 + + +def test_should_raise_for_connection_errors(document_download): + with pytest.raises(DocumentDownloadError) 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 excinfo.value.message == 'connection error' + assert excinfo.value.status_code == 503