Revamp Exception handling (All-Hands-AI#1080)

* Revamp exception handling

* Agent controller: sleep 3 seconds if APIConnection error

* Fix AuthenticationError capture

* Revert unrelated style fixes

* Add type enforcement for action_from_dict call

agenthub/monologue_agent/agent.py

@@ -3,6 +3,7 @@
 from opendevin.state import State
 from opendevin.llm.llm import LLM
 from opendevin.schema import ActionType, ObservationType
+from opendevin.exceptions import AgentNoInstructionError
 
 from opendevin.action import (
     Action,
@@ -131,14 +132,14 @@ def _initialize(self, task: str):
         - task (str): The initial goal statement provided by the user
 
         Raises:
-        - ValueError: If task is not provided
+        - AgentNoInstructionError: If task is not provided
         """
 
         if self._initialized:
             return
 
         if task is None or task == '':
-            raise ValueError('Instruction must be provided')
+            raise AgentNoInstructionError()
         self.monologue = Monologue()
         self.memory = LongTermMemory()
 

agenthub/monologue_agent/utils/monologue.py

@@ -1,5 +1,7 @@
 import traceback
+
 from opendevin.llm.llm import LLM
+from opendevin.exceptions import AgentEventTypeError
 import agenthub.monologue_agent.utils.json as json
 import agenthub.monologue_agent.utils.prompts as prompts
 
@@ -24,10 +26,10 @@ def add_event(self, t: dict):
         - t (dict): The thought that we want to add to memory
 
         Raises:
-        - ValueError: If t is not a dict
+        - AgentEventTypeError: If t is not a dict
         """
         if not isinstance(t, dict):
-            raise ValueError('Event must be a dictionary')
+            raise AgentEventTypeError()
         self.thoughts.append(t)
 
     def get_thoughts(self):

agenthub/monologue_agent/utils/prompts.py

@@ -12,6 +12,7 @@
 from opendevin.observation import (
     CmdOutputObservation,
 )
+from opendevin.exceptions import LLMOutputError
 
 ACTION_PROMPT = """
 You're a thoughtful robot. Your main task is this:
@@ -170,7 +171,7 @@ def rank(match):
         try:
             action_dict = json.loads(max(response_json_matches, key=rank)[0])  # Use the highest ranked response
         except ValueError as e:
-            raise ValueError(
+            raise LLMOutputError(
                 "Output from the LLM isn't properly formatted. The model may be misconfigured."
             ) from e
     if 'content' in action_dict:

opendevin/action/__init__.py

@@ -28,6 +28,8 @@
 
 
 def action_from_dict(action: dict) -> Action:
+    if not isinstance(action, dict):
+        raise TypeError('action must be a dictionary')
     action = action.copy()
     if 'action' not in action:
         raise KeyError(f"'action' key is not found in {action=}")

opendevin/agent.py

@@ -5,6 +5,7 @@
     from opendevin.action import Action
     from opendevin.state import State
 from opendevin.llm.llm import LLM
+from opendevin.exceptions import AgentAlreadyRegisteredError, AgentNotRegisteredError
 
 
 class Agent(ABC):
@@ -71,9 +72,12 @@ def register(cls, name: str, agent_cls: Type['Agent']):
         Parameters:
         - name (str): The name to register the class under.
         - agent_cls (Type['Agent']): The class to register.
+
+        Raises:
+        - AgentAlreadyRegisteredError: If name already registered
         """
         if name in cls._registry:
-            raise ValueError(f"Agent class already registered under '{name}'.")
+            raise AgentAlreadyRegisteredError(name)
         cls._registry[name] = agent_cls
 
     @classmethod
@@ -86,16 +90,22 @@ def get_cls(cls, name: str) -> Type['Agent']:
 
         Returns:
         - agent_cls (Type['Agent']): The class registered under the specified name.
+
+        Raises:
+        - AgentNotRegisteredError: If name not registered
         """
         if name not in cls._registry:
-            raise ValueError(f"No agent class registered under '{name}'.")
+            raise AgentNotRegisteredError(name)
         return cls._registry[name]
 
     @classmethod
     def list_agents(cls) -> list[str]:
         """
         Retrieves the list of all agent names from the registry.
+
+        Raises:
+        - AgentNotRegisteredError: If no agent is registered
         """
         if not bool(cls._registry):
-            raise ValueError('No agent class registered.')
+            raise AgentNotRegisteredError()
         return list(cls._registry.keys())

opendevin/controller/agent_controller.py

@@ -1,9 +1,12 @@
 import asyncio
 import inspect
 import traceback
+import time
 from typing import List, Callable, Literal, Mapping, Awaitable, Any, cast
 
 from termcolor import colored
+from litellm.exceptions import APIConnectionError
+from openai import AuthenticationError
 
 from opendevin import config
 from opendevin.action import (
@@ -15,7 +18,7 @@
 )
 from opendevin.agent import Agent
 from opendevin.logger import opendevin_logger as logger
-from opendevin.exceptions import MaxCharsExceedError
+from opendevin.exceptions import MaxCharsExceedError, AgentNoActionError
 from opendevin.observation import Observation, AgentErrorObservation, NullObservation
 from opendevin.plan import Plan
 from opendevin.state import State
@@ -102,9 +105,11 @@ def update_state_after_step(self):
 
     def add_history(self, action: Action, observation: Observation):
         if not isinstance(action, Action):
-            raise ValueError('action must be an instance of Action')
+            raise TypeError(
+                f'action must be an instance of Action, got {type(action).__name__} instead')
         if not isinstance(observation, Observation):
-            raise ValueError('observation must be an instance of Observation')
+            raise TypeError(
+                f'observation must be an instance of Observation, got {type(observation).__name__} instead')
         self.state.history.append((action, observation))
         self.state.updated_info.append((action, observation))
 
@@ -144,17 +149,23 @@ async def step(self, i: int):
         try:
             action = self.agent.step(self.state)
             if action is None:
-                raise ValueError('Agent must return an action')
+                raise AgentNoActionError()
             print_with_color(action, 'ACTION')
         except Exception as e:
             observation = AgentErrorObservation(str(e))
             print_with_color(observation, 'ERROR')
             traceback.print_exc()
-            # TODO Change to more robust error handling
-            if (
-                'The api_key client option must be set' in observation.content
-                or 'Incorrect API key provided:' in observation.content
-            ):
+            if isinstance(e, APIConnectionError):
+                time.sleep(3)
+
+            # raise specific exceptions that need to be handled outside
+            # note: we are using AuthenticationError class from openai rather than
+            # litellm because:
+            # 1) litellm.exceptions.AuthenticationError is a subclass of openai.AuthenticationError
+            # 2) embeddings call, initiated by llama-index, has no wrapper for authentication
+            #    errors. This means we have to catch individual authentication errors
+            #    from different providers, and OpenAI is one of these.
+            if isinstance(e, (AuthenticationError, AgentNoActionError)):
                 raise
         self.update_state_after_step()
 

opendevin/exceptions.py

@@ -5,3 +5,59 @@ def __init__(self, num_of_chars=None, max_chars_limit=None):
         else:
             message = 'Number of characters exceeds MAX_CHARS limit'
         super().__init__(message)
+
+
+class AgentNoActionError(Exception):
+    def __init__(self, message='Agent must return an action'):
+        super().__init__(message)
+
+
+class AgentNoInstructionError(Exception):
+    def __init__(self, message='Instruction must be provided'):
+        super().__init__(message)
+
+
+class AgentEventTypeError(Exception):
+    def __init__(self, message='Event must be a dictionary'):
+        super().__init__(message)
+
+
+class AgentAlreadyRegisteredError(Exception):
+    def __init__(self, name=None):
+        if name is not None:
+            message = f"Agent class already registered under '{name}'"
+        else:
+            message = 'Agent class already registered'
+        super().__init__(message)
+
+
+class AgentNotRegisteredError(Exception):
+    def __init__(self, name=None):
+        if name is not None:
+            message = f"No agent class registered under '{name}'"
+        else:
+            message = 'No agent class registered'
+        super().__init__(message)
+
+
+class LLMOutputError(Exception):
+    def __init__(self, message):
+        super().__init__(message)
+
+
+class SandboxInvalidBackgroundCommandError(Exception):
+    def __init__(self, id=None):
+        if id is not None:
+            message = f'Invalid background command id {id}'
+        else:
+            message = 'Invalid background command id'
+        super().__init__(message)
+
+
+class PlanInvalidStateError(Exception):
+    def __init__(self, state=None):
+        if state is not None:
+            message = f'Invalid state {state}'
+        else:
+            message = 'Invalid state'
+        super().__init__(message)

opendevin/plan.py

@@ -1,5 +1,7 @@
 from typing import List
+
 from opendevin.logger import opendevin_logger as logger
+from opendevin.exceptions import PlanInvalidStateError
 
 OPEN_STATE = 'open'
 COMPLETED_STATE = 'completed'
@@ -87,11 +89,11 @@ def set_state(self, state):
         Args:            state: The new state of the task.
 
         Raises:
-            ValueError: If the provided state is invalid.
+            PlanInvalidStateError: If the provided state is invalid.
         """
         if state not in STATES:
             logger.error('Invalid state: %s', state)
-            raise ValueError('Invalid state:' + state)
+            raise PlanInvalidStateError(state)
         self.state = state
         if state == COMPLETED_STATE or state == ABANDONED_STATE or state == VERIFIED_STATE:
             for subtask in self.subtasks:

opendevin/sandbox/exec_box.py

@@ -13,6 +13,7 @@
 from opendevin.logger import opendevin_logger as logger
 from opendevin.sandbox.sandbox import Sandbox, BackgroundCommand
 from opendevin.schema import ConfigType
+from opendevin.exceptions import SandboxInvalidBackgroundCommandError
 
 InputType = namedtuple('InputType', ['content'])
 OutputType = namedtuple('OutputType', ['content'])
@@ -107,7 +108,7 @@ def get_exec_cmd(self, cmd: str) -> List[str]:
 
     def read_logs(self, id) -> str:
         if id not in self.background_commands:
-            raise ValueError('Invalid background command id')
+            raise SandboxInvalidBackgroundCommandError()
         bg_cmd = self.background_commands[id]
         return bg_cmd.read_logs()
 
@@ -157,7 +158,7 @@ def get_pid(self, cmd):
 
     def kill_background(self, id: int) -> BackgroundCommand:
         if id not in self.background_commands:
-            raise ValueError('Invalid background command id')
+            raise SandboxInvalidBackgroundCommandError()
         bg_cmd = self.background_commands[id]
         if bg_cmd.pid is not None:
             self.container.exec_run(

opendevin/sandbox/ssh_box.py

@@ -15,6 +15,7 @@
 from opendevin.sandbox.sandbox import Sandbox, BackgroundCommand
 from opendevin.schema import ConfigType
 from opendevin.utils import find_available_tcp_port
+from opendevin.exceptions import SandboxInvalidBackgroundCommandError
 
 InputType = namedtuple('InputType', ['content'])
 OutputType = namedtuple('OutputType', ['content'])
@@ -187,7 +188,7 @@ def get_exec_cmd(self, cmd: str) -> List[str]:
 
     def read_logs(self, id) -> str:
         if id not in self.background_commands:
-            raise ValueError('Invalid background command id')
+            raise SandboxInvalidBackgroundCommandError()
         bg_cmd = self.background_commands[id]
         return bg_cmd.read_logs()
 
@@ -238,7 +239,7 @@ def get_pid(self, cmd):
 
     def kill_background(self, id: int) -> BackgroundCommand:
         if id not in self.background_commands:
-            raise ValueError('Invalid background command id')
+            raise SandboxInvalidBackgroundCommandError()
         bg_cmd = self.background_commands[id]
         if bg_cmd.pid is not None:
             self.container.exec_run(