diff --git a/openhands/llm/llm.py b/openhands/llm/llm.py index 743d6535ba3b..40f152884d6c 100644 --- a/openhands/llm/llm.py +++ b/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, diff --git a/openhands/llm/retry_mixin.py b/openhands/llm/retry_mixin.py index 1005677320e1..cfcf237c0d4a 100644 --- a/openhands/llm/retry_mixin.py +++ b/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, diff --git a/tests/unit/test_retry_mixin.py b/tests/unit/test_retry_mixin.py new file mode 100644 index 000000000000..d7d25bf750a8 --- /dev/null +++ b/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