From 25619c5a9385b4a2146c81da0dd8d1ebe67a2814 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Fri, 16 May 2025 11:01:39 -0400 Subject: [PATCH] Fix #8510: Improve error messages for invalid microagent format (#8511) Co-authored-by: openhands --- openhands/microagent/microagent.py | 17 +++++++++++++-- openhands/server/session/session.py | 28 +++++++++++++++++++----- tests/unit/test_microagent_utils.py | 34 +++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/openhands/microagent/microagent.py b/openhands/microagent/microagent.py index c1b18ca794..6f5722fd53 100644 --- a/openhands/microagent/microagent.py +++ b/openhands/microagent/microagent.py @@ -65,7 +65,14 @@ class BaseMicroagent(BaseModel): try: metadata = MicroagentMetadata(**metadata_dict) except Exception as e: - raise MicroagentValidationError(f'Error loading metadata: {e}') from e + # Provide more detailed error message for validation errors + error_msg = f'Error validating microagent metadata in {path.name}: {str(e)}' + if 'type' in metadata_dict and metadata_dict['type'] not in [ + t.value for t in MicroagentType + ]: + valid_types = ', '.join([f'"{t.value}"' for t in MicroagentType]) + error_msg += f'. Invalid "type" value: "{metadata_dict["type"]}". Valid types are: {valid_types}' + raise MicroagentValidationError(error_msg) from e # Create appropriate subclass based on type subclass_map = { @@ -184,7 +191,13 @@ def load_microagents_from_dir( elif isinstance(agent, KnowledgeMicroagent): knowledge_agents[agent.name] = agent logger.debug(f'Loaded agent {agent.name} from {file}') + except MicroagentValidationError as e: + # For validation errors, include the original exception + error_msg = f'Error loading microagent from {file}: {str(e)}' + raise MicroagentValidationError(error_msg) from e except Exception as e: - raise ValueError(f'Error loading agent from {file}: {e}') + # For other errors, wrap in a ValueError with detailed message + error_msg = f'Error loading microagent from {file}: {str(e)}' + raise ValueError(error_msg) from e return repo_agents, knowledge_agents diff --git a/openhands/server/session/session.py b/openhands/server/session/session.py index 5ed3ef277a..c5239eafe6 100644 --- a/openhands/server/session/session.py +++ b/openhands/server/session/session.py @@ -12,6 +12,7 @@ from openhands.core.config.condenser_config import ( CondenserPipelineConfig, LLMSummarizingCondenserConfig, ) +from openhands.core.exceptions import MicroagentValidationError from openhands.core.logger import OpenHandsLoggerAdapter from openhands.core.schema import AgentState from openhands.events.action import MessageAction, NullAction @@ -176,16 +177,33 @@ class Session: initial_message=initial_message, replay_json=replay_json, ) + except MicroagentValidationError as e: + self.logger.exception(f'Error creating agent_session: {e}') + # For microagent validation errors, provide more helpful information + await self.send_error(f'Failed to create agent session: {str(e)}') + return + except ValueError as e: + self.logger.exception(f'Error creating agent_session: {e}') + error_message = str(e) + # For ValueError related to microagents, provide more helpful information + if 'microagent' in error_message.lower(): + await self.send_error( + f'Failed to create agent session: {error_message}' + ) + else: + # For other ValueErrors, just show the error class + await self.send_error('Failed to create agent session: ValueError') + return except Exception as e: self.logger.exception(f'Error creating agent_session: {e}') - err_class = e.__class__.__name__ - await self.send_error(f'Failed to create agent session: {err_class}') + # For other errors, just show the error class to avoid exposing sensitive information + await self.send_error( + f'Failed to create agent session: {e.__class__.__name__}' + ) return def _create_llm(self, agent_cls: str | None) -> LLM: - """ - Initialize LLM, extracted for testing. - """ + """Initialize LLM, extracted for testing.""" agent_name = agent_cls if agent_cls is not None else 'agent' return LLM( config=self.config.get_llm_config_from_agent(agent_name), diff --git a/tests/unit/test_microagent_utils.py b/tests/unit/test_microagent_utils.py index 13aedd442e..e5e7b2437d 100644 --- a/tests/unit/test_microagent_utils.py +++ b/tests/unit/test_microagent_utils.py @@ -166,3 +166,37 @@ Testing loading with trailing slashes. assert isinstance(agent_t, KnowledgeMicroagent) assert agent_t.type == MicroagentType.KNOWLEDGE # Check inferred type assert 'trailing' in agent_t.triggers + + +def test_invalid_microagent_type(temp_microagents_dir): + """Test loading a microagent with an invalid type.""" + # Create a microagent with an invalid type + invalid_agent = """--- +name: invalid_type_agent +type: task +version: 1.0.0 +agent: CodeActAgent +triggers: + - test +--- + +# Invalid Type Test + +This microagent has an invalid type. +""" + invalid_file = temp_microagents_dir / 'invalid_type.md' + invalid_file.write_text(invalid_agent) + + # Attempt to load the microagent should raise a MicroagentValidationError + from openhands.core.exceptions import MicroagentValidationError + + with pytest.raises(MicroagentValidationError) as excinfo: + load_microagents_from_dir(temp_microagents_dir) + + # Check that the error message contains helpful information + error_msg = str(excinfo.value) + assert 'invalid_type.md' in error_msg + assert 'Invalid "type" value: "task"' in error_msg + assert 'Valid types are:' in error_msg + assert '"knowledge"' in error_msg + assert '"repo"' in error_msg