Handle and log document-download-api request errors

Catches the requests exception for document-download-api calls, logs
a warning and returns a matching response code and message.

Connection errors to document download result in 503 response to the
user.

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(
+        try:
-            self.get_upload_url(service_id),
+            response = requests.post(
-            headers={
+                self.get_upload_url(service_id),
-                'Authorization': "Bearer {}".format(self.auth_token),
+                headers={
-            },
+                    'Authorization': "Bearer {}".format(self.auth_token),
-            files={
+                },
-                'document': base64.b64decode(file_contents)
+                files={
-            }
+                    'document': base64.b64decode(file_contents)
-        )
+                }
-        response.raise_for_status()
+            )
+
+            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']

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(
+            try:
-                service.id, personalisation_data[key]['file']
+                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
 

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:
+    with pytest.raises(DocumentDownloadError) as excinfo, requests_mock.Mocker() as request_mock:
-        request_mock.post('https://document-download/services/service-id/documents', json={}, status_code=403)
+        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