fix: enhance agent loader exception handler and expose precise error information

PiperOrigin-RevId: 766876662

src/google/adk/cli/utils/agent_loader.py

@@ -61,15 +61,23 @@ class AgentLoader:
               "Root agent found is not an instance of BaseAgent. But a type %s",
               type(module_candidate.root_agent),
           )
+      else:
+        logger.debug(
+            "Module %s has no root_agent. Trying next pattern.",
+            agent_name,
+        )
 
-    except ModuleNotFoundError:
-      logger.debug("Module %s itself not found.", agent_name)
-      # Re-raise as ValueError to be caught by the final error message construction
-      raise ValueError(
-          f"Module {agent_name} not found during import attempts."
-      ) from None
-    except ImportError as e:
-      logger.warning("Error importing %s: %s", agent_name, e)
+    except ModuleNotFoundError as e:
+      if e.name == agent_name:
+        logger.debug("Module %s itself not found.", agent_name)
+      else:
+        # it's the case the module imported by {agent_name}.agent module is not
+        # found
+        e.msg = f"Fail to load '{agent_name}' module. " + e.msg
+        raise e
+    except Exception as e:
+      e.msg = f"Fail to load '{agent_name}' module. " + e.msg
+      raise e
 
     return None
 
@@ -87,12 +95,23 @@ class AgentLoader:
               "Root agent found is not an instance of BaseAgent. But a type %s",
               type(module_candidate.root_agent),
           )
-    except ModuleNotFoundError:
-      logger.debug(
-          "Module %s.agent not found, trying next pattern.", agent_name
-      )
-    except ImportError as e:
-      logger.warning("Error importing %s.agent: %s", agent_name, e)
+      else:
+        logger.debug(
+            "Module %s.agent has no root_agent.",
+            agent_name,
+        )
+    except ModuleNotFoundError as e:
+      # if it's agent module not found, it's fine, search for next pattern
+      if e.name == f"{agent_name}.agent" or e.name == agent_name:
+        logger.debug("Module %s.agent not found.", agent_name)
+      else:
+        # it's the case the module imported by {agent_name}.agent module is not
+        # found
+        e.msg = f"Fail to load '{agent_name}.agent' module. " + e.msg
+        raise e
+    except Exception as e:
+      e.msg = f"Fail to load '{agent_name}.agent' module. " + e.msg
+      raise e
 
     return None
 
@@ -107,10 +126,10 @@ class AgentLoader:
     )
     envs.load_dotenv_for_agent(agent_name, str(self.agents_dir))
 
-    if root_agent := self._load_from_submodule(agent_name):
+    if root_agent := self._load_from_module_or_package(agent_name):
       return root_agent
 
-    if root_agent := self._load_from_module_or_package(agent_name):
+    if root_agent := self._load_from_submodule(agent_name):
       return root_agent
 
     # If no root_agent was found by any pattern

tests/unittests/cli/utils/test_agent_loader.py

@@ -219,6 +219,40 @@ class TestAgentLoader:
       assert agent.config == "production"
       assert os.environ.get("AGENT_SECRET") == "test_secret_123"
 
+  def test_loading_order_preference(self):
+    """Test that module/package is preferred over agent.py in a sub-package."""
+    with tempfile.TemporaryDirectory() as temp_dir:
+      temp_path = Path(temp_dir)
+      agent_name = "order_test_agent"
+
+      # Create structure 1: agents_dir/agent_name.py (expected to be loaded)
+      agent_module_file = temp_path / f"{agent_name}.py"
+      agent_module_file.write_text(dedent(f"""
+                from google.adk.agents.base_agent import BaseAgent
+                class ModuleAgent(BaseAgent):
+                    def __init__(self):
+                        super().__init__(name="{agent_name}_module_version")
+                root_agent = ModuleAgent()
+            """))
+
+      # Create structure 2: agents_dir/agent_name/agent.py (should be ignored)
+      agent_package_dir = temp_path / agent_name
+      agent_package_dir.mkdir()
+      agent_submodule_file = agent_package_dir / "agent.py"
+      agent_submodule_file.write_text(dedent(f"""
+                from google.adk.agents.base_agent import BaseAgent
+                class SubmoduleAgent(BaseAgent):
+                    def __init__(self):
+                        super().__init__(name="{agent_name}_submodule_version")
+                root_agent = SubmoduleAgent()
+            """))
+
+      loader = AgentLoader(str(temp_path))
+      agent = loader.load_agent(agent_name)
+
+      # Assert that the module version was loaded due to the new loading order
+      assert agent.name == f"{agent_name}_module_version"
+
   def test_load_multiple_different_agents(self):
     """Test loading multiple different agents."""
     with tempfile.TemporaryDirectory() as temp_dir:
@@ -249,12 +283,24 @@ class TestAgentLoader:
     """Test that appropriate error is raised when agent is not found."""
     with tempfile.TemporaryDirectory() as temp_dir:
       loader = AgentLoader(temp_dir)
+      agents_dir = temp_dir  # For use in the expected message string
 
       # Try to load non-existent agent
       with pytest.raises(ValueError) as exc_info:
         loader.load_agent("nonexistent_agent")
 
-      assert "Module nonexistent_agent not found" in str(exc_info.value)
+      expected_msg_part_1 = "No root_agent found for 'nonexistent_agent'."
+      expected_msg_part_2 = (
+          "Searched in 'nonexistent_agent.agent.root_agent',"
+          " 'nonexistent_agent.root_agent'."
+      )
+      expected_msg_part_3 = (
+          f"Ensure '{agents_dir}/nonexistent_agent' is structured correctly"
+      )
+
+      assert expected_msg_part_1 in str(exc_info.value)
+      assert expected_msg_part_2 in str(exc_info.value)
+      assert expected_msg_part_3 in str(exc_info.value)
 
   def test_agent_without_root_agent_error(self):
     """Test that appropriate error is raised when agent has no root_agent."""
@@ -279,6 +325,65 @@ class TestAgentLoader:
 
       assert "No root_agent found for 'broken_agent'" in str(exc_info.value)
 
+  def test_agent_internal_module_not_found_error(self):
+    """Test error when an agent tries to import a non-existent module."""
+    with tempfile.TemporaryDirectory() as temp_dir:
+      temp_path = Path(temp_dir)
+      agent_name = "importer_agent"
+
+      # Create agent that imports a non-existent module
+      agent_file = temp_path / f"{agent_name}.py"
+      agent_file.write_text(dedent(f"""
+                from google.adk.agents.base_agent import BaseAgent
+                import non_existent_module  # This will fail
+
+                class {agent_name.title()}Agent(BaseAgent):
+                    def __init__(self):
+                        super().__init__(name="{agent_name}")
+
+                root_agent = {agent_name.title()}Agent()
+            """))
+
+      loader = AgentLoader(str(temp_path))
+      with pytest.raises(ModuleNotFoundError) as exc_info:
+        loader.load_agent(agent_name)
+
+      assert f"Fail to load '{agent_name}' module." in str(exc_info.value)
+      assert "No module named 'non_existent_module'" in str(exc_info.value)
+
+  def test_agent_internal_import_error(self):
+    """Test other import errors within an agent's code (e.g., SyntaxError)."""
+    with tempfile.TemporaryDirectory() as temp_dir:
+      temp_path = Path(temp_dir)
+      agent_name = "syntax_error_agent"
+
+      # Create agent with a syntax error (which leads to ImportError)
+      agent_file = temp_path / f"{agent_name}.py"
+      agent_file.write_text(dedent(f"""
+                from google.adk.agents.base_agent import BaseAgent
+
+                # Invalid syntax
+                this is not valid python code
+
+                class {agent_name.title()}Agent(BaseAgent):
+                    def __init__(self):
+                        super().__init__(name="{agent_name}")
+
+                root_agent = {agent_name.title()}Agent()
+            """))
+
+      loader = AgentLoader(str(temp_path))
+      # SyntaxError is a subclass of Exception, and importlib might wrap it
+      # The loader is expected to prepend its message and re-raise.
+      with pytest.raises(
+          SyntaxError
+      ) as exc_info:  # Or potentially ImportError depending on Python version specifics with importlib
+        loader.load_agent(agent_name)
+
+      assert f"Fail to load '{agent_name}' module." in str(exc_info.value)
+      # Check for part of the original SyntaxError message
+      assert "invalid syntax" in str(exc_info.value).lower()
+
   def test_sys_path_modification(self):
     """Test that agents_dir is added to sys.path correctly."""
     with tempfile.TemporaryDirectory() as temp_dir: