[PRMP-1607] Increase timeout and handle exceptions in bulk upload service (#1184)

Author: NogaNHS

lambdas/services/base/s3_service.py

@@ -29,8 +29,8 @@ def __new__(cls, *args, **kwargs):
     def __init__(self, custom_aws_role=None):
         if not self.initialised:
             self.config = BotoConfig(
-                connect_timeout=3,
-                read_timeout=5,
+                connect_timeout=10,
+                read_timeout=60,
                 retries={"max_attempts": 3, "mode": "standard"},
                 s3={"addressing_style": "virtual"},
                 signature_version="s3v4",

lambdas/services/bulk_upload_service.py

@@ -3,7 +3,7 @@
 import uuid
 
 import pydantic
-from botocore.exceptions import ClientError
+from botocore.exceptions import ClientError, ConnectTimeoutError, ReadTimeoutError
 
 from enums.document_review_reason import DocumentReviewReason
 from enums.patient_ods_inactive_status import PatientOdsInactiveStatus
@@ -335,7 +335,7 @@ def handle_sqs_message(self, message: dict):
                 f"Successfully uploaded the Lloyd George records for patient: {staging_metadata.nhs_number}",
                 {"Result": "Successful upload"},
             )
-        except ClientError as e:
+        except (ClientError, ReadTimeoutError, ConnectTimeoutError) as e:
             logger.info(
                 f"Got unexpected error during file transfer: {str(e)}",
                 {"Result": "Unsuccessful upload"},

lambdas/tests/unit/services/base/test_s3_service.py

@@ -5,6 +5,7 @@
 import pytest
 from botocore.exceptions import ClientError
 from freezegun import freeze_time
+
 from services.base.s3_service import S3Service
 from tests.unit.conftest import (
     MOCK_BUCKET,
@@ -72,8 +73,8 @@ def test_s3_service_constructs_boto_client_with_timeouts(mocker):
 
     mocked_boto_client.assert_called_once()
     _, kwargs = mocked_boto_client.call_args
-    assert kwargs["config"].connect_timeout == 3
-    assert kwargs["config"].read_timeout == 5
+    assert kwargs["config"].connect_timeout == 10
+    assert kwargs["config"].read_timeout == 60
 
     S3Service._instance = None
 

lambdas/tests/unit/services/test_bulk_upload_service.py

@@ -2,7 +2,7 @@
 from copy import copy
 
 import pytest
-from botocore.exceptions import ClientError
+from botocore.exceptions import ClientError, ConnectTimeoutError, ReadTimeoutError
 from freezegun import freeze_time
 
 from enums.document_review_reason import DocumentReviewReason
@@ -736,7 +736,20 @@ def test_handle_sqs_message_put_staging_metadata_back_to_queue_when_virus_scan_r
     repo_under_test.sqs_repository.send_message_to_pdf_stitching_queue.assert_not_called()
 
 
+@pytest.mark.parametrize(
+    "transfer_error",
+    [
+        ClientError(
+            {"Error": {"Code": "AccessDenied", "Message": "Access Denied"}},
+            "GetObject",
+        ),
+        ReadTimeoutError(endpoint_url="https://s3.amazonaws.com"),
+        ConnectTimeoutError(endpoint_url="https://s3.amazonaws.com"),
+    ],
+    ids=["ClientError", "ReadTimeoutError", "ConnectTimeoutError"],
+)
 def test_handle_sqs_message_rollback_transaction_when_validation_pass_but_file_transfer_failed_halfway(
+    transfer_error,
     repo_under_test,
     set_env,
     mocker,
@@ -766,16 +779,11 @@ def test_handle_sqs_message_rollback_transaction_when_validation_pass_but_file_t
         repo_under_test.bulk_upload_s3_repository,
         "remove_ingested_file_from_source_bucket",
     )
-    mock_client_error = ClientError(
-        {"Error": {"Code": "AccessDenied", "Message": "Access Denied"}},
-        "GetObject",
-    )
 
-    # simulate a client error occur when copying the 3rd file
     repo_under_test.bulk_upload_s3_repository.copy_to_lg_bucket.side_effect = [
         MOCK_COPY_OBJECT_RESPONSE,
         MOCK_COPY_OBJECT_RESPONSE,
-        mock_client_error,
+        transfer_error,
     ]
 
     repo_under_test.handle_sqs_message(message=TEST_SQS_MESSAGE)