DAS-2493: Implement retry behavior for server errors. (#69)

Author: flamingbear

CHANGELOG.md

@@ -1,3 +1,10 @@
+## [v1.2.2] - 2026-03-23
+
+### Changed
+
+- Failures encountered during downloading of OPeNDAP files are now retried
+  if the error is not due to Authorization.
+
 ## [v1.2.2] - 2026-01-22
 
 ### Changed
@@ -237,6 +244,7 @@ Repository structure changes include:
 
 For more information on internal releases prior to NASA open-source approval,
 see legacy-CHANGELOG.md.
+[v1.2.3]: https://github.com/nasa/harmony-opendap-subsetter/releases/tag/1.2.3
 [v1.2.2]: https://github.com/nasa/harmony-opendap-subsetter/releases/tag/1.2.2
 [v1.2.1]: https://github.com/nasa/harmony-opendap-subsetter/releases/tag/1.2.1
 [v1.2.0]: https://github.com/nasa/harmony-opendap-subsetter/releases/tag/1.2.0

docker/service_version.txt

@@ -1 +1 @@
-1.2.2
+1.2.3

hoss/exceptions.py

@@ -293,16 +293,26 @@ def __init__(self, units_string):
         )
 
 
-class UrlAccessFailed(CustomNoRetryError):
-    """This exception is raised when an HTTP request for a given URL has a non
-    500 error, and is therefore not retried.
+class UrlAccessFailed(CustomError):
+    """This exception is raised when an HTTP request for a given URL fails due
+    to a server or transient network error that may succeed on retry.
 
     """
 
     def __init__(self, url, status_code):
         super().__init__('UrlAccessFailed', f'{status_code} error retrieving: {url}')
 
 
+class UrlAccessForbidden(CustomNoRetryError):
+    """This exception is raised when access to a URL is forbidden (e.g., HTTP
+    403) and should not be retried.
+
+    """
+
+    def __init__(self, url, status_code):
+        super().__init__('UrlAccessForbidden', f'{status_code} error retrieving: {url}')
+
+
 class InvalidVariableRequest(CustomNoRetryError):
     """This exception is raised when invalid variables are requested,
     e.g., excluded science variables listed in the varinfo configuration.

hoss/utilities.py

@@ -21,7 +21,7 @@
 from harmony_service_lib.util import Config
 from harmony_service_lib.util import download as util_download
 
-from hoss.exceptions import CustomNoRetryError, UrlAccessFailed
+from hoss.exceptions import CustomNoRetryError, UrlAccessFailed, UrlAccessForbidden
 from hoss.harmony_log_context import get_logger
 
 
@@ -166,7 +166,7 @@ def download_url(
             cfg=config,
         )
     except ForbiddenException as harmony_exception:
-        raise UrlAccessFailed(url, 400) from harmony_exception
+        raise UrlAccessForbidden(url, 403) from harmony_exception
     except ServerException as harmony_exception:
         raise UrlAccessFailed(url, 500) from harmony_exception
     except Exception as harmony_exception:

tests/test_adapter.py

@@ -13,6 +13,7 @@
 from unittest.mock import ANY, Mock, call, patch
 
 from harmony_service_lib.exceptions import NoDataException, NoRetryException
+from harmony_service_lib.http import ForbiddenException
 from harmony_service_lib.message import Message
 from harmony_service_lib.util import HarmonyException, config
 from netCDF4 import Dataset
@@ -21,7 +22,7 @@
 from varinfo import VarInfoFromDmr
 
 from hoss.adapter import HossAdapter
-from hoss.exceptions import InvalidVariableRequest
+from hoss.exceptions import InvalidVariableRequest, UrlAccessForbidden
 from tests.utilities import Granule, create_stac, write_dmr
 
 
@@ -2742,18 +2743,18 @@ def test_retriable_exception_handling(
 
     @patch('hoss.adapter.mkdtemp')
     @patch('shutil.rmtree')
-    @patch('hoss.subset.check_invalid_variable_request')
+    @patch('hoss.utilities.util_download')
     @patch('hoss.adapter.stage')
     def test_not_retriable_exception_handling(
-        self, mock_stage, mock_check_variable, mock_rmtree, mock_mkdtemp
+        self, mock_stage, mock_util_download, mock_rmtree, mock_mkdtemp
     ):
         """Ensure that if a not retriable exception is raised during
         processing, this causes a NoRetryException to be raised, to prevent
         extra Harmony CPU cycles.
 
         """
         mock_mkdtemp.return_value = self.tmp_dir
-        mock_check_variable.side_effect = InvalidVariableRequest('Random error')
+        mock_util_download.side_effect = ForbiddenException('403 no permission error')
 
         message = Message(
             {

tests/unit/test_utilities.py

@@ -2,10 +2,20 @@
 from unittest import TestCase
 from unittest.mock import Mock, patch
 
-from harmony_service_lib.exceptions import ForbiddenException, ServerException
+from harmony_service_lib.exceptions import (
+    ForbiddenException,
+    HarmonyException,
+    NoRetryException,
+    ServerException,
+)
 from harmony_service_lib.util import config
 
-from hoss.exceptions import UrlAccessFailed
+from hoss.exceptions import (
+    CustomError,
+    CustomNoRetryError,
+    UrlAccessFailed,
+    UrlAccessForbidden,
+)
 from hoss.harmony_log_context import set_logger
 from hoss.utilities import (
     download_url,
@@ -16,6 +26,7 @@
     get_opendap_nc4,
     get_value_or_default,
     move_downloaded_nc4,
+    raise_from_hoss_exception,
     unexecuted_url_requested,
 )
 
@@ -99,12 +110,15 @@ def test_download_url(self, mock_util_download):
             )
             mock_util_download.reset_mock()
 
-        with self.subTest('500 error is caught and handled.'):
+        with self.subTest('500 error is caught and handled, and is retryable.'):
             mock_util_download.side_effect = [self.harmony_500_error, http_response]
 
-            with self.assertRaises(UrlAccessFailed):
+            with self.assertRaises(UrlAccessFailed) as context:
                 download_url(test_url, output_directory, access_token, self.config)
 
+            self.assertIsInstance(context.exception, CustomError)
+            self.assertNotIsInstance(context.exception, CustomNoRetryError)
+
             mock_util_download.assert_called_once_with(
                 test_url,
                 output_directory,
@@ -115,12 +129,36 @@ def test_download_url(self, mock_util_download):
             )
             mock_util_download.reset_mock()
 
-        with self.subTest('Non-500 error does not retry, and is re-raised.'):
+        with self.subTest('A 403 error (forbidden) is not retried.'):
             mock_util_download.side_effect = [self.harmony_auth_error, http_response]
 
-            with self.assertRaises(UrlAccessFailed):
+            with self.assertRaises(UrlAccessForbidden) as context:
+                download_url(test_url, output_directory, access_token, self.config)
+
+            self.assertIsInstance(context.exception, CustomNoRetryError)
+
+            mock_util_download.assert_called_once_with(
+                test_url,
+                output_directory,
+                self.logger,
+                access_token=access_token,
+                data=None,
+                cfg=self.config,
+            )
+            mock_util_download.reset_mock()
+
+        with self.subTest('Unknown/transient error is caught and is retryable.'):
+            mock_util_download.side_effect = [
+                Exception('something broke'),
+                http_response,
+            ]
+
+            with self.assertRaises(UrlAccessFailed) as context:
                 download_url(test_url, output_directory, access_token, self.config)
 
+            self.assertIsInstance(context.exception, CustomError)
+            self.assertNotIsInstance(context.exception, CustomNoRetryError)
+
             mock_util_download.assert_called_once_with(
                 test_url,
                 output_directory,
@@ -335,3 +373,23 @@ def test_unexecuted_url_requested(self):
 
         with self.subTest('Format type is None'):
             self.assertFalse(unexecuted_url_requested(None))
+
+    def test_raise_from_hoss_exception(self):
+        """Ensure that HOSS exceptions are correctly converted to Harmony
+        exceptions. CustomNoRetryError subclasses should become
+        NoRetryException, while CustomError subclasses should become a
+        retryable HarmonyException.
+
+        """
+        test_url = 'fake_website.com'
+
+        with self.subTest('UrlAccessForbidden (no-retry) raises NoRetryException.'):
+            forbidden_exception = UrlAccessForbidden(test_url, 403)
+            with self.assertRaises(NoRetryException):
+                raise_from_hoss_exception(forbidden_exception)
+
+        with self.subTest('UrlAccessFailed (retryable) raises HarmonyException.'):
+            failed_exception = UrlAccessFailed(test_url, 500)
+            with self.assertRaises(HarmonyException) as context:
+                raise_from_hoss_exception(failed_exception)
+            self.assertNotIsInstance(context.exception, NoRetryException)