refactor error handling so not all exceptions are caught (#1296)

* refactor error handling so not all exceptions are caught

* revert

* Send the failed decoding back to the LLM (#1322)

* fix quotes

---------

Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>

opendevin/action/__init__.py

@@ -10,6 +10,7 @@ from .agent import (
     AgentSummarizeAction,
 )
 from .tasks import AddTaskAction, ModifyTaskAction
+from ..exceptions import AgentMalformedActionError
 
 actions = (
     CmdKillAction,
@@ -29,17 +30,21 @@ ACTION_TYPE_TO_CLASS = {action_class.action: action_class for action_class in ac
 
 def action_from_dict(action: dict) -> Action:
     if not isinstance(action, dict):
-        raise TypeError('action must be a dictionary')
+        raise AgentMalformedActionError('action must be a dictionary')
     action = action.copy()
     if 'action' not in action:
-        raise KeyError(f"'action' key is not found in {action=}")
+        raise AgentMalformedActionError(f"'action' key is not found in {action=}")
     action_class = ACTION_TYPE_TO_CLASS.get(action['action'])
     if action_class is None:
-        raise KeyError(
+        raise AgentMalformedActionError(
             f"'{action['action']=}' is not defined. Available actions: {ACTION_TYPE_TO_CLASS.keys()}"
         )
     args = action.get('args', {})
-    return action_class(**args)
+    try:
+        decoded_action = action_class(**args)
+    except TypeError:
+        raise AgentMalformedActionError(f'action={action} has the wrong arguments')
+    return decoded_action
 
 
 __all__ = [

opendevin/action/fileop.py

@@ -4,8 +4,10 @@ from dataclasses import dataclass
 from pathlib import Path
 
 from opendevin.observation import (
+    Observation,
     FileReadObservation,
-    FileWriteObservation
+    FileWriteObservation,
+    AgentErrorObservation,
 )
 
 from opendevin.schema import ActionType
@@ -60,7 +62,7 @@ class FileReadAction(ExecutableAction):
             end = -1 if self.end > num_lines else max(begin + 1, self.end)
             return all_lines[begin:end]
 
-    async def run(self, controller) -> FileReadObservation:
+    async def run(self, controller) -> Observation:
         if isinstance(controller.action_manager.sandbox, E2BBox):
             content = controller.action_manager.sandbox.filesystem.read(
                 self.path)
@@ -74,7 +76,7 @@ class FileReadAction(ExecutableAction):
                     read_lines = self._read_lines(file.readlines())
                     code_view = ''.join(read_lines)
             except FileNotFoundError:
-                raise FileNotFoundError(f'File not found: {self.path}')
+                return AgentErrorObservation(f'File not found: {self.path}')
         return FileReadObservation(path=self.path, content=code_view)
 
     @property
@@ -100,7 +102,7 @@ class FileWriteAction(ExecutableAction):
         new_lines += [''] if self.end == -1 else original[self.end:]
         return new_lines
 
-    async def run(self, controller) -> FileWriteObservation:
+    async def run(self, controller) -> Observation:
         insert = self.content.split('\n')
 
         if isinstance(controller.action_manager.sandbox, E2BBox):
@@ -110,7 +112,7 @@ class FileWriteAction(ExecutableAction):
                 new_file = self._insert_lines(self.content.split('\n'), all_lines)
                 controller.action_manager.sandbox.filesystem.write(self.path, ''.join(new_file))
             else:
-                raise FileNotFoundError(f'File not found: {self.path}')
+                return AgentErrorObservation(f'File not found: {self.path}')
         else:
             whole_path = resolve_path(self.path)
             mode = 'w' if not os.path.exists(whole_path) else 'r+'
@@ -126,7 +128,7 @@ class FileWriteAction(ExecutableAction):
                     file.writelines(new_file)
                     file.truncate()
             except FileNotFoundError:
-                raise FileNotFoundError(f'File not found: {self.path}')
+                return AgentErrorObservation(f'File not found: {self.path}')
         return FileWriteObservation(content='', path=self.path)
 
     @property

opendevin/controller/action_manager.py

@@ -1,17 +1,14 @@
 from typing import List
-import traceback
 
 from opendevin import config
 from opendevin.observation import CmdOutputObservation
 from opendevin.sandbox import DockerExecBox, DockerSSHBox, Sandbox, LocalBox, E2BBox
 from opendevin.schema import ConfigType
-from opendevin.logger import opendevin_logger as logger
 from opendevin.action import (
     Action,
 )
 from opendevin.observation import (
     Observation,
-    AgentErrorObservation,
     NullObservation,
 )
 from opendevin.sandbox.plugins import PluginRequirement
@@ -49,12 +46,7 @@ class ActionManager:
         observation: Observation = NullObservation('')
         if not action.executable:
             return observation
-        try:
-            observation = await action.run(agent_controller)
-        except Exception as e:
-            observation = AgentErrorObservation(str(e))
-            logger.error(e)
-            logger.debug(traceback.format_exc())
+        observation = await action.run(agent_controller)
         return observation
 
     def run_command(self, command: str, background=False) -> CmdOutputObservation:

opendevin/controller/agent_controller.py

@@ -1,9 +1,6 @@
 import asyncio
-import traceback
 from typing import Callable, List
 
-from openai import AuthenticationError, APIConnectionError
-from litellm import ContextWindowExceededError
 
 from opendevin import config
 from opendevin.action import (
@@ -12,7 +9,7 @@ from opendevin.action import (
     NullAction,
 )
 from opendevin.agent import Agent
-from opendevin.exceptions import AgentNoActionError, MaxCharsExceedError
+from opendevin.exceptions import AgentMalformedActionError, AgentNoActionError, MaxCharsExceedError
 from opendevin.logger import opendevin_logger as logger
 from opendevin.observation import AgentErrorObservation, NullObservation, Observation
 from opendevin.plan import Plan
@@ -179,21 +176,10 @@ class AgentController:
         try:
             action = self.agent.step(self.state)
             if action is None:
-                raise AgentNoActionError()
-            logger.info(action, extra={'msg_type': 'ACTION'})
-        except Exception as e:
+                raise AgentNoActionError('No action was returned')
+        except (AgentMalformedActionError, AgentNoActionError) as e:
             observation = AgentErrorObservation(str(e))
-            logger.error(e)
-            logger.debug(traceback.format_exc())
-
-            # raise specific exceptions that need to be handled outside
-            # note: we are using classes 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 errors.
-            #    This means we have to catch individual authentication errors
-            #    from different providers, and OpenAI is one of these.
-            if isinstance(e, (AuthenticationError, ContextWindowExceededError, APIConnectionError)):
-                raise
+        logger.info(action, extra={'msg_type': 'ACTION'})
 
         self.update_state_after_step()
 

opendevin/exceptions.py

@@ -7,11 +7,6 @@ class MaxCharsExceedError(Exception):
         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)
@@ -61,3 +56,14 @@ class PlanInvalidStateError(Exception):
         else:
             message = 'Invalid state'
         super().__init__(message)
+
+
+# These exceptions get sent back to the LLM
+class AgentMalformedActionError(Exception):
+    def __init__(self, message='Malformed response'):
+        super().__init__(message)
+
+
+class AgentNoActionError(Exception):
+    def __init__(self, message='Agent must return an action'):
+        super().__init__(message)