From f7120e562bafc57c2c908429f023e815feec98b0 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 30 Nov 2024 00:29:49 +0000 Subject: [PATCH 01/14] Fix issue #4484: (feat) Improve retry_decorator with blacklist exceptions --- openhands/llm/llm.py | 8 ++- openhands/llm/retry_mixin.py | 25 ++++++-- .../runtime/impl/remote/remote_runtime.py | 1 - openhands/server/session/README.md | 12 ++-- openhands/server/session/session.py | 2 - poetry.lock | 2 +- pyproject.toml | 1 + tests/unit/test_retry_mixin.py | 58 +++++++++++++++++++ 8 files changed, 92 insertions(+), 17 deletions(-) create mode 100644 tests/unit/test_retry_mixin.py diff --git a/openhands/llm/llm.py b/openhands/llm/llm.py index 700c3827fda0..78c8363d3d92 100644 --- a/openhands/llm/llm.py +++ b/openhands/llm/llm.py @@ -43,14 +43,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 = [ @@ -141,6 +144,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..a39bde8b6caa 100644 --- a/openhands/llm/retry_mixin.py +++ b/openhands/llm/retry_mixin.py @@ -1,6 +1,8 @@ +from typing import Any + from tenacity import ( retry, - retry_if_exception_type, + retry_if_exception, stop_after_attempt, wait_exponential, ) @@ -12,28 +14,41 @@ 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') + def _should_retry(exception: Exception) -> bool: + """Check if an exception should be retried based on its type and exclusion list.""" + # First check if it's an excluded exception + for exc_type in exclude_exceptions: + if isinstance(exception, exc_type): + return False + # Then check if it's a retryable exception + for exc_type in retry_exceptions: + if isinstance(exception, exc_type): + return True + return False + 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_if_exception(_should_retry), wait=wait_exponential( multiplier=retry_multiplier, min=retry_min_wait, diff --git a/openhands/runtime/impl/remote/remote_runtime.py b/openhands/runtime/impl/remote/remote_runtime.py index 548e8a0ee23e..4226bfef8f15 100644 --- a/openhands/runtime/impl/remote/remote_runtime.py +++ b/openhands/runtime/impl/remote/remote_runtime.py @@ -10,7 +10,6 @@ import tenacity from openhands.core.config import AppConfig -from openhands.core.logger import openhands_logger as logger from openhands.events import EventStream from openhands.events.action import ( BrowseInteractiveAction, diff --git a/openhands/server/session/README.md b/openhands/server/session/README.md index b367ba586495..12f7e270d622 100644 --- a/openhands/server/session/README.md +++ b/openhands/server/session/README.md @@ -8,19 +8,19 @@ interruptions are recoverable. There are 3 main server side event handlers: * `connect` - Invoked when a new connection to the server is established. (This may be via http or WebSocket) -* `oh_action` - Invoked when a connected client sends an event (Such as `INIT` or a prompt for the Agent) - +* `oh_action` - Invoked when a connected client sends an event (Such as `INIT` or a prompt for the Agent) - this is distinct from the `oh_event` sent from the server to the client. * `disconnect` - Invoked when a connected client disconnects from the server. ## Init Each connection has a unique id, and when initially established, is not associated with any session. An -`INIT` event must be sent to the server in order to attach a connection to a session. The `INIT` event -may optionally include a GitHub token and a token to connect to an existing session. (Which may be running -locally or may need to be hydrated). If no token is received as part of the init event, it is assumed a +`INIT` event must be sent to the server in order to attach a connection to a session. The `INIT` event +may optionally include a GitHub token and a token to connect to an existing session. (Which may be running +locally or may need to be hydrated). If no token is received as part of the init event, it is assumed a new session should be started. ## Disconnect -The (manager)[manager.py] manages connections and sessions. Each session may have zero or more connections +The (manager)[manager.py] manages connections and sessions. Each session may have zero or more connections associated with it, managed by invocations of `INIT` and disconnect. When a session no longer has any connections associated with it, after a set amount of time (determined by `config.sandbox.close_delay`), -the session and runtime are passivated (So will need to be rehydrated to continue.) +the session and runtime are passivated (So will need to be rehydrated to continue.) diff --git a/openhands/server/session/session.py b/openhands/server/session/session.py index e26d237a8764..2b0ec8742cff 100644 --- a/openhands/server/session/session.py +++ b/openhands/server/session/session.py @@ -8,7 +8,6 @@ from openhands.core.const.guide_url import TROUBLESHOOTING_URL from openhands.core.logger import openhands_logger as logger from openhands.core.schema import AgentState -from openhands.core.schema.action import ActionType from openhands.core.schema.config import ConfigType from openhands.events.action import MessageAction, NullAction from openhands.events.event import Event, EventSource @@ -23,7 +22,6 @@ from openhands.llm.llm import LLM from openhands.server.session.agent_session import AgentSession from openhands.storage.files import FileStore -from openhands.utils.async_utils import call_coro_in_bg_thread ROOM_KEY = 'room:{sid}' diff --git a/poetry.lock b/poetry.lock index d97ef683fe6b..11b19f1e827d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -10350,4 +10350,4 @@ testing = ["coverage[toml]", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "^3.12" -content-hash = "ff370b7b5077720b73fe3b90cc1b7fb9c7a262bfbd35885bb717369061e8a466" +content-hash = "3a53d7fd7faa61d4e014deb4603f8f25575cc8fd27f46505b6c82e7fbc50c782" diff --git a/pyproject.toml b/pyproject.toml index ec148baadc5c..30c31039d77d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,6 +66,7 @@ pygithub = "^2.5.0" openhands-aci = "^0.1.1" python-socketio = "^5.11.4" redis = "^5.2.0" +pytest = "^8.3.3" [tool.poetry.group.llama-index.dependencies] llama-index = "*" diff --git a/tests/unit/test_retry_mixin.py b/tests/unit/test_retry_mixin.py new file mode 100644 index 000000000000..c5e1319c8f3e --- /dev/null +++ b/tests/unit/test_retry_mixin.py @@ -0,0 +1,58 @@ +import pytest +from tenacity import RetryError + +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 From cfebaf1786fce4d7892dccab4d7b36376c9cced6 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 30 Nov 2024 01:09:43 +0000 Subject: [PATCH 02/14] Fix pr #5333: Fix issue #4484: (feat) Improve retry_decorator with blacklist exceptions --- openhands/llm/retry_mixin.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/openhands/llm/retry_mixin.py b/openhands/llm/retry_mixin.py index a39bde8b6caa..181c0ff80e2e 100644 --- a/openhands/llm/retry_mixin.py +++ b/openhands/llm/retry_mixin.py @@ -2,7 +2,8 @@ from tenacity import ( retry, - retry_if_exception, + retry_if_exception_type, + retry_if_not_exception_type, stop_after_attempt, wait_exponential, ) @@ -32,23 +33,15 @@ def retry_decorator(self, **kwargs: Any): retry_max_wait = kwargs.get('retry_max_wait') retry_multiplier = kwargs.get('retry_multiplier') - def _should_retry(exception: Exception) -> bool: - """Check if an exception should be retried based on its type and exclusion list.""" - # First check if it's an excluded exception - for exc_type in exclude_exceptions: - if isinstance(exception, exc_type): - return False - # Then check if it's a retryable exception - for exc_type in retry_exceptions: - if isinstance(exception, exc_type): - return True - return False + 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(_should_retry), + retry=retry_condition, wait=wait_exponential( multiplier=retry_multiplier, min=retry_min_wait, From 04903ab188ebe0430f647e8fba71ca4040d2038a Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 02:27:41 +0100 Subject: [PATCH 03/14] Update tests/unit/test_retry_mixin.py --- tests/unit/test_retry_mixin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_retry_mixin.py b/tests/unit/test_retry_mixin.py index c5e1319c8f3e..d7d25bf750a8 100644 --- a/tests/unit/test_retry_mixin.py +++ b/tests/unit/test_retry_mixin.py @@ -1,5 +1,4 @@ import pytest -from tenacity import RetryError from openhands.llm.retry_mixin import RetryMixin From d9ebcc357cea281654d8f96cee631870ec285f96 Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:11:05 +0100 Subject: [PATCH 04/14] Update lint.yml --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b6a9d327d860..97de04535bc4 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -33,7 +33,7 @@ jobs: - name: Lint run: | cd frontend - npm run lint + npm run lint || true # Run lint on the python code lint-python: From 8cea37cf9782543882ab2067b4378eb9331eaebe Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:16:26 +0100 Subject: [PATCH 05/14] temp --- .github/workflows/lint.yml | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 97de04535bc4..caa705d6e866 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,24 +16,6 @@ concurrency: cancel-in-progress: true jobs: - # Run lint on the frontend code - lint-frontend: - name: Lint frontend - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: Install Node.js 20 - uses: actions/setup-node@v4 - with: - node-version: 20 - - name: Install dependencies - run: | - cd frontend - npm install --frozen-lockfile - - name: Lint - run: | - cd frontend - npm run lint || true # Run lint on the python code lint-python: From 0a5df7362c2aea50aca76cbc202e16ae9140f3f7 Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:21:49 +0100 Subject: [PATCH 06/14] Update lint.yml --- .github/workflows/lint.yml | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index caa705d6e866..1d305b65876a 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,7 +16,25 @@ concurrency: cancel-in-progress: true jobs: - + # Run lint on the frontend code + lint-frontend: + name: Lint frontend + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install Node.js 20 + uses: actions/setup-node@v4 + with: + node-version: 20 + - name: Install dependencies + run: | + cd frontend + npm install --frozen-lockfile + - name: Lint + run: | + cd frontend + npm run lint + # Run lint on the python code lint-python: name: Lint python From c57f2679aa753ed57c45a6613d8d2f703b08d1e3 Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:23:31 +0100 Subject: [PATCH 07/14] fix lint-fix.yml --- .github/workflows/lint-fix.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint-fix.yml b/.github/workflows/lint-fix.yml index 9fa97eaaf2f1..53f409355c60 100644 --- a/.github/workflows/lint-fix.yml +++ b/.github/workflows/lint-fix.yml @@ -32,7 +32,7 @@ jobs: - name: Fix frontend lint issues run: | cd frontend - npm run lint:fix + npm run lint:fix || true # Python lint fixes - name: Set up python From 7a81bd0c129c74cc0652c876cdc735786b37f905 Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:32:13 +0100 Subject: [PATCH 08/14] temp --- .github/workflows/lint-fix.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint-fix.yml b/.github/workflows/lint-fix.yml index 53f409355c60..a2dbbac83ec3 100644 --- a/.github/workflows/lint-fix.yml +++ b/.github/workflows/lint-fix.yml @@ -30,9 +30,9 @@ jobs: cd frontend npm install --frozen-lockfile - name: Fix frontend lint issues - run: | - cd frontend - npm run lint:fix || true + #run: | + # cd frontend + # npm run lint:fix || true # Python lint fixes - name: Set up python From c2c9c2260c3005686bb9878211d7dd29623e77b5 Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:42:27 +0100 Subject: [PATCH 09/14] temp --- .github/workflows/lint-fix.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/lint-fix.yml b/.github/workflows/lint-fix.yml index a2dbbac83ec3..6636baedc6bd 100644 --- a/.github/workflows/lint-fix.yml +++ b/.github/workflows/lint-fix.yml @@ -21,15 +21,15 @@ jobs: token: ${{ secrets.GITHUB_TOKEN }} # Frontend lint fixes - - name: Install Node.js 20 - uses: actions/setup-node@v4 - with: - node-version: 20 - - name: Install frontend dependencies - run: | - cd frontend - npm install --frozen-lockfile - - name: Fix frontend lint issues + #- name: Install Node.js 20 + # uses: actions/setup-node@v4 + # with: + # node-version: 20 + #- name: Install frontend dependencies + # run: | + # cd frontend + # npm install --frozen-lockfile + #- name: Fix frontend lint issues #run: | # cd frontend # npm run lint:fix || true From df64ac90d8c9233abf119691c6bb26da2f7b2876 Mon Sep 17 00:00:00 2001 From: OpenHands Bot Date: Sat, 30 Nov 2024 03:43:35 +0000 Subject: [PATCH 10/14] =?UTF-8?q?=F0=9F=A4=96=20Auto-fix=20linting=20issue?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- openhands/llm/retry_mixin.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openhands/llm/retry_mixin.py b/openhands/llm/retry_mixin.py index 181c0ff80e2e..cfcf237c0d4a 100644 --- a/openhands/llm/retry_mixin.py +++ b/openhands/llm/retry_mixin.py @@ -35,7 +35,9 @@ def retry_decorator(self, **kwargs: Any): retry_condition = retry_if_exception_type(retry_exceptions) if exclude_exceptions: - retry_condition = retry_condition & retry_if_not_exception_type(exclude_exceptions) + retry_condition = retry_condition & retry_if_not_exception_type( + exclude_exceptions + ) return retry( before_sleep=self.log_retry_attempt, From f2c50e3cc40d2cc13dd971401b1fa5a1ba9a9b7c Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:46:00 +0100 Subject: [PATCH 11/14] Update lint-fix.yml --- .github/workflows/lint-fix.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/lint-fix.yml b/.github/workflows/lint-fix.yml index 6636baedc6bd..53f409355c60 100644 --- a/.github/workflows/lint-fix.yml +++ b/.github/workflows/lint-fix.yml @@ -21,18 +21,18 @@ jobs: token: ${{ secrets.GITHUB_TOKEN }} # Frontend lint fixes - #- name: Install Node.js 20 - # uses: actions/setup-node@v4 - # with: - # node-version: 20 - #- name: Install frontend dependencies - # run: | - # cd frontend - # npm install --frozen-lockfile - #- name: Fix frontend lint issues - #run: | - # cd frontend - # npm run lint:fix || true + - name: Install Node.js 20 + uses: actions/setup-node@v4 + with: + node-version: 20 + - name: Install frontend dependencies + run: | + cd frontend + npm install --frozen-lockfile + - name: Fix frontend lint issues + run: | + cd frontend + npm run lint:fix || true # Python lint fixes - name: Set up python From db5abf8ab68cd6b2ff059e921a2f644548a2aa6e Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:46:42 +0100 Subject: [PATCH 12/14] Update .github/workflows/lint-fix.yml --- .github/workflows/lint-fix.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint-fix.yml b/.github/workflows/lint-fix.yml index 53f409355c60..9fa97eaaf2f1 100644 --- a/.github/workflows/lint-fix.yml +++ b/.github/workflows/lint-fix.yml @@ -32,7 +32,7 @@ jobs: - name: Fix frontend lint issues run: | cd frontend - npm run lint:fix || true + npm run lint:fix # Python lint fixes - name: Set up python From 3ee072ef476ee48917dc242bc65a21f06c94d06b Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:47:03 +0100 Subject: [PATCH 13/14] Update .github/workflows/lint.yml --- .github/workflows/lint.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 1d305b65876a..f81815da0d87 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -34,7 +34,6 @@ jobs: run: | cd frontend npm run lint - # Run lint on the python code lint-python: name: Lint python From 69b6e4cb883391314b7b2ef75fc0937dac7705f2 Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sat, 30 Nov 2024 04:47:31 +0100 Subject: [PATCH 14/14] Update .github/workflows/lint.yml --- .github/workflows/lint.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index f81815da0d87..b6a9d327d860 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -34,6 +34,7 @@ jobs: run: | cd frontend npm run lint + # Run lint on the python code lint-python: name: Lint python