Fix issue #4484: (feat) Improve retry_decorator with blacklist exceptions

openhands/llm/llm.py

@@ -44,14 +44,17 @@
 # tuple of exceptions to retry on
 LLM_RETRY_EXCEPTIONS: tuple[type[Exception], ...] = (
     APIConnectionError,
-    # FIXME: APIError is useful on 502 from a proxy for example,
-    # but it also retries on other errors that are permanent
+    # APIError is useful on 502 from a proxy for example,
+    # but some specific APIError instances should not be retried
     APIError,
     InternalServerError,
     RateLimitError,
     ServiceUnavailableError,
 )
 
+# tuple of exceptions that should not be retried even if they inherit from retry_exceptions
+LLM_EXCLUDE_EXCEPTIONS: tuple[type[Exception], ...] = (CloudFlareBlockageError,)
+
 # cache prompt supporting models
 # remove this when we gemini and deepseek are supported
 CACHE_PROMPT_SUPPORTED_MODELS = [
@@ -148,6 +151,7 @@ def __init__(
         @self.retry_decorator(
             num_retries=self.config.num_retries,
             retry_exceptions=LLM_RETRY_EXCEPTIONS,
+            exclude_exceptions=LLM_EXCLUDE_EXCEPTIONS,
             retry_min_wait=self.config.retry_min_wait,
             retry_max_wait=self.config.retry_max_wait,
             retry_multiplier=self.config.retry_multiplier,

openhands/llm/retry_mixin.py

@@ -1,6 +1,9 @@
+from typing import Any
+
 from tenacity import (
     retry,
     retry_if_exception_type,
+    retry_if_not_exception_type,
     stop_after_attempt,
     wait_exponential,
 )
@@ -12,28 +15,35 @@
 class RetryMixin:
     """Mixin class for retry logic."""
 
-    def retry_decorator(self, **kwargs):
+    def retry_decorator(self, **kwargs: Any):
         """
         Create a LLM retry decorator with customizable parameters. This is used for 429 errors, and a few other exceptions in LLM classes.
 
         Args:
             **kwargs: Keyword arguments to override default retry behavior.
-                      Keys: num_retries, retry_exceptions, retry_min_wait, retry_max_wait, retry_multiplier
+                      Keys: num_retries, retry_exceptions, exclude_exceptions, retry_min_wait, retry_max_wait, retry_multiplier
 
         Returns:
             A retry decorator with the parameters customizable in configuration.
         """
         num_retries = kwargs.get('num_retries')
-        retry_exceptions = kwargs.get('retry_exceptions')
+        retry_exceptions = kwargs.get('retry_exceptions', ())
+        exclude_exceptions = kwargs.get('exclude_exceptions', ())
         retry_min_wait = kwargs.get('retry_min_wait')
         retry_max_wait = kwargs.get('retry_max_wait')
         retry_multiplier = kwargs.get('retry_multiplier')
 
+        retry_condition = retry_if_exception_type(retry_exceptions)
+        if exclude_exceptions:
+            retry_condition = retry_condition & retry_if_not_exception_type(
+                exclude_exceptions
+            )
+
         return retry(
             before_sleep=self.log_retry_attempt,
             stop=stop_after_attempt(num_retries) | stop_if_should_exit(),
             reraise=True,
-            retry=(retry_if_exception_type(retry_exceptions)),
+            retry=retry_condition,
             wait=wait_exponential(
                 multiplier=retry_multiplier,
                 min=retry_min_wait,

tests/unit/test_retry_mixin.py

@@ -0,0 +1,57 @@
+import pytest
+
+from openhands.llm.retry_mixin import RetryMixin
+
+
+class TestException(Exception):
+    pass
+
+
+class TestExceptionChild(TestException):
+    pass
+
+
+class TestExceptionExcluded(TestException):
+    pass
+
+
+class TestRetryMixin:
+    def test_retry_decorator_with_exclude_exceptions(self):
+        mixin = RetryMixin()
+
+        # Create a function that raises different exceptions
+        attempt_count = 0
+
+        @mixin.retry_decorator(
+            num_retries=3,
+            retry_exceptions=(TestException,),
+            exclude_exceptions=(TestExceptionExcluded,),
+            retry_min_wait=0.1,
+            retry_max_wait=0.2,
+            retry_multiplier=0.1,
+        )
+        def test_func(exception_type):
+            nonlocal attempt_count
+            attempt_count += 1
+            raise exception_type()
+
+        # Test that retryable exception is retried
+        with pytest.raises(TestException):
+            test_func(TestException)
+        assert attempt_count == 3  # Should retry 2 times after initial attempt
+
+        # Reset counter
+        attempt_count = 0
+
+        # Test that child of retryable exception is retried
+        with pytest.raises(TestExceptionChild):
+            test_func(TestExceptionChild)
+        assert attempt_count == 3  # Should retry 2 times after initial attempt
+
+        # Reset counter
+        attempt_count = 0
+
+        # Test that excluded exception is not retried
+        with pytest.raises(TestExceptionExcluded):
+            test_func(TestExceptionExcluded)
+        assert attempt_count == 1  # Should not retry