issues/181: Added retry logic for Harmony jobs that fail with 5xx errors

Author: jackiryan

bignbit/get_harmony_job_status.py

@@ -2,6 +2,7 @@
 import logging
 import os
 
+import requests
 from cumulus_logger import CumulusLogger
 from cumulus_process import Process
 from harmony import LinkType
@@ -12,6 +13,13 @@
 CUMULUS_LOGGER = CumulusLogger('get_harmony_job_status')
 
 
+class HarmonyTransientError(Exception):
+    """Exception raised when the Harmony API returns a transient 5xx error"""
+
+    def __init__(self, message):
+        super().__init__(message)
+
+
 class HarmonyJobIncompleteError(Exception):
     """Exception raised when a harmony job is not complete"""
 
@@ -98,12 +106,26 @@ def check_harmony_job(
     """
 
     harmony_client = utils.get_harmony_client(cmr_env)
-    job_status = harmony_client.status(harmony_job_id)
+    try:
+        job_status = harmony_client.status(harmony_job_id)
+    except requests.exceptions.HTTPError as exc:
+        if exc.response is not None and exc.response.status_code >= 500:
+            raise HarmonyTransientError(
+                f'Harmony API returned a transient error checking status of job {harmony_job_id}: {exc}'
+            ) from exc
+        raise
 
     # For a successful job, return the status; for all other states, raise an exception.
     if job_status.get('status') == 'successful':
         # Check that the harmony job returned data to confirm that the job was successful
-        result_urls = list(harmony_client.result_urls(harmony_job_id, link_type=LinkType.s3))
+        try:
+            result_urls = list(harmony_client.result_urls(harmony_job_id, link_type=LinkType.s3))
+        except requests.exceptions.HTTPError as exc:
+            if exc.response is not None and exc.response.status_code >= 500:
+                raise HarmonyTransientError(
+                    f'Harmony API returned a transient error fetching result URLs for job {harmony_job_id}: {exc}'
+                ) from exc
+            raise
         if not result_urls:
             error_msg = (
                 f'Harmony job {harmony_job_id} completed successfully but returned no data for {variable} and {crs}'

terraform/state_machine_definition.tpl

@@ -338,6 +338,14 @@
                         "BackoffRate":${HarmonyJobStatusBackoffRate},
                         "MaxDelaySeconds":${HarmonyJobStatusMaxDelaySeconds}
                       },
+                      {
+                        "ErrorEquals":[
+                          "HarmonyTransientError"
+                        ],
+                        "IntervalSeconds":2,
+                        "MaxAttempts":6,
+                        "BackoffRate":2
+                      },
                       {
                         "ErrorEquals":[
                           "Lambda.ServiceException",

tests/test_get_harmony_job_status.py

@@ -1,9 +1,16 @@
 """Unit tests for get_harmony_job_status module"""
+from unittest.mock import MagicMock, patch
+
 import pytest
+import requests
 from moto import mock_s3
 
 import bignbit.utils
-from bignbit.get_harmony_job_status import check_harmony_job, HarmonyJobNoDataError
+from bignbit.get_harmony_job_status import (
+    check_harmony_job,
+    HarmonyJobNoDataError,
+    HarmonyTransientError,
+)
 
 
 @pytest.mark.vcr
@@ -21,4 +28,75 @@ def test_process_results_no_data():
 
     assert 'no data' in str(exc_info.value).lower()
     assert 'test_variable' in str(exc_info.value)
-    assert 'EPSG:4326' in str(exc_info.value)
+    assert 'EPSG:4326' in str(exc_info.value)
+
+
+def _make_http_error(status_code):
+    """Build a requests.exceptions.HTTPError with a mocked response."""
+    response = MagicMock()
+    response.status_code = status_code
+    error = requests.exceptions.HTTPError(response=response)
+    return error
+
+
+@patch('bignbit.get_harmony_job_status.utils.get_harmony_client')
+def test_check_harmony_job_raises_transient_error_on_500(mock_get_client):
+    """Test that HarmonyTransientError is raised when Harmony status returns a 500."""
+    bignbit.utils.ED_USER = 'test'
+    bignbit.utils.ED_PASS = 'test'
+
+    mock_client = MagicMock()
+    mock_client.status.side_effect = _make_http_error(500)
+    mock_get_client.return_value = mock_client
+
+    with pytest.raises(HarmonyTransientError) as exc_info:
+        check_harmony_job('test-job-id', 'uat', 'test_variable', 'EPSG:4326')
+
+    assert 'transient error' in str(exc_info.value).lower()
+    assert 'test-job-id' in str(exc_info.value)
+
+
+@patch('bignbit.get_harmony_job_status.utils.get_harmony_client')
+def test_check_harmony_job_raises_transient_error_on_503(mock_get_client):
+    """Test that HarmonyTransientError is raised when Harmony status returns a 503."""
+    bignbit.utils.ED_USER = 'test'
+    bignbit.utils.ED_PASS = 'test'
+
+    mock_client = MagicMock()
+    mock_client.status.side_effect = _make_http_error(503)
+    mock_get_client.return_value = mock_client
+
+    with pytest.raises(HarmonyTransientError):
+        check_harmony_job('test-job-id', 'uat', 'test_variable', 'EPSG:4326')
+
+
+@patch('bignbit.get_harmony_job_status.utils.get_harmony_client')
+def test_check_harmony_job_reraises_non_5xx_http_error(mock_get_client):
+    """Test that non-5xx HTTPErrors (e.g. 404) are re-raised without wrapping."""
+    bignbit.utils.ED_USER = 'test'
+    bignbit.utils.ED_PASS = 'test'
+
+    mock_client = MagicMock()
+    mock_client.status.side_effect = _make_http_error(404)
+    mock_get_client.return_value = mock_client
+
+    with pytest.raises(requests.exceptions.HTTPError):
+        check_harmony_job('test-job-id', 'uat', 'test_variable', 'EPSG:4326')
+
+
+@patch('bignbit.get_harmony_job_status.utils.get_harmony_client')
+def test_check_harmony_job_result_urls_raises_transient_error_on_500(mock_get_client):
+    """Test that HarmonyTransientError is raised when result_urls call returns a 500."""
+    bignbit.utils.ED_USER = 'test'
+    bignbit.utils.ED_PASS = 'test'
+
+    mock_client = MagicMock()
+    mock_client.status.return_value = {'status': 'successful'}
+    mock_client.result_urls.side_effect = _make_http_error(500)
+    mock_get_client.return_value = mock_client
+
+    with pytest.raises(HarmonyTransientError) as exc_info:
+        check_harmony_job('test-job-id', 'uat', 'test_variable', 'EPSG:4326')
+
+    assert 'result urls' in str(exc_info.value).lower()
+    assert 'test-job-id' in str(exc_info.value)