mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-04-30 23:02:50 +00:00
Fix tool error causing double event scope pop (#4373)
When a tool raises an error, both ToolUsageErrorEvent and ToolUsageFinishedEvent were being emitted. Since both events pop the event scope stack, this caused the agent scope to be incorrectly popped along with the tool scope.
This commit is contained in:
@@ -814,6 +814,7 @@ class CrewAgentExecutor(CrewAgentExecutorMixin):
|
||||
agent_key=agent_key,
|
||||
),
|
||||
)
|
||||
error_event_emitted = False
|
||||
|
||||
track_delegation_if_needed(func_name, args_dict, self.task)
|
||||
|
||||
@@ -896,6 +897,7 @@ class CrewAgentExecutor(CrewAgentExecutorMixin):
|
||||
error=e,
|
||||
),
|
||||
)
|
||||
error_event_emitted = True
|
||||
elif max_usage_reached and original_tool:
|
||||
# Return error message when max usage limit is reached
|
||||
result = f"Tool '{func_name}' has reached its usage limit of {original_tool.max_usage_count} times and cannot be used anymore."
|
||||
@@ -923,20 +925,20 @@ class CrewAgentExecutor(CrewAgentExecutorMixin):
|
||||
color="red",
|
||||
)
|
||||
|
||||
# Emit tool usage finished event
|
||||
crewai_event_bus.emit(
|
||||
self,
|
||||
event=ToolUsageFinishedEvent(
|
||||
output=result,
|
||||
tool_name=func_name,
|
||||
tool_args=args_dict,
|
||||
from_agent=self.agent,
|
||||
from_task=self.task,
|
||||
agent_key=agent_key,
|
||||
started_at=started_at,
|
||||
finished_at=datetime.now(),
|
||||
),
|
||||
)
|
||||
if not error_event_emitted:
|
||||
crewai_event_bus.emit(
|
||||
self,
|
||||
event=ToolUsageFinishedEvent(
|
||||
output=result,
|
||||
tool_name=func_name,
|
||||
tool_args=args_dict,
|
||||
from_agent=self.agent,
|
||||
from_task=self.task,
|
||||
agent_key=agent_key,
|
||||
started_at=started_at,
|
||||
finished_at=datetime.now(),
|
||||
),
|
||||
)
|
||||
|
||||
# Append tool result message
|
||||
tool_message: LLMMessage = {
|
||||
|
||||
@@ -689,6 +689,7 @@ class AgentExecutor(Flow[AgentReActState], CrewAgentExecutorMixin):
|
||||
agent_key=agent_key,
|
||||
),
|
||||
)
|
||||
error_event_emitted = False
|
||||
|
||||
track_delegation_if_needed(func_name, args_dict, self.task)
|
||||
|
||||
@@ -764,6 +765,7 @@ class AgentExecutor(Flow[AgentReActState], CrewAgentExecutorMixin):
|
||||
error=e,
|
||||
),
|
||||
)
|
||||
error_event_emitted = True
|
||||
elif max_usage_reached and original_tool:
|
||||
# Return error message when max usage limit is reached
|
||||
result = f"Tool '{func_name}' has reached its usage limit of {original_tool.max_usage_count} times and cannot be used anymore."
|
||||
@@ -792,20 +794,20 @@ class AgentExecutor(Flow[AgentReActState], CrewAgentExecutorMixin):
|
||||
color="red",
|
||||
)
|
||||
|
||||
# Emit tool usage finished event
|
||||
crewai_event_bus.emit(
|
||||
self,
|
||||
event=ToolUsageFinishedEvent(
|
||||
output=result,
|
||||
tool_name=func_name,
|
||||
tool_args=args_dict,
|
||||
from_agent=self.agent,
|
||||
from_task=self.task,
|
||||
agent_key=agent_key,
|
||||
started_at=started_at,
|
||||
finished_at=datetime.now(),
|
||||
),
|
||||
)
|
||||
if not error_event_emitted:
|
||||
crewai_event_bus.emit(
|
||||
self,
|
||||
event=ToolUsageFinishedEvent(
|
||||
output=result,
|
||||
tool_name=func_name,
|
||||
tool_args=args_dict,
|
||||
from_agent=self.agent,
|
||||
from_task=self.task,
|
||||
agent_key=agent_key,
|
||||
started_at=started_at,
|
||||
finished_at=datetime.now(),
|
||||
),
|
||||
)
|
||||
|
||||
# Append tool result message
|
||||
tool_message: LLMMessage = {
|
||||
|
||||
@@ -270,6 +270,7 @@ class ToolUsage:
|
||||
result = None # type: ignore
|
||||
should_retry = False
|
||||
available_tool = None
|
||||
error_event_emitted = False
|
||||
|
||||
try:
|
||||
if self.tools_handler and self.tools_handler.cache:
|
||||
@@ -408,6 +409,7 @@ class ToolUsage:
|
||||
|
||||
except Exception as e:
|
||||
self.on_tool_error(tool=tool, tool_calling=calling, e=e)
|
||||
error_event_emitted = True
|
||||
self._run_attempts += 1
|
||||
if self._run_attempts > self._max_parsing_attempts:
|
||||
self._telemetry.tool_usage_error(llm=self.function_calling_llm)
|
||||
@@ -435,7 +437,7 @@ class ToolUsage:
|
||||
result = self._format_result(result=result)
|
||||
|
||||
finally:
|
||||
if started_event_emitted:
|
||||
if started_event_emitted and not error_event_emitted:
|
||||
self.on_tool_use_finished(
|
||||
tool=tool,
|
||||
tool_calling=calling,
|
||||
@@ -500,6 +502,7 @@ class ToolUsage:
|
||||
result = None # type: ignore
|
||||
should_retry = False
|
||||
available_tool = None
|
||||
error_event_emitted = False
|
||||
|
||||
try:
|
||||
if self.tools_handler and self.tools_handler.cache:
|
||||
@@ -638,6 +641,7 @@ class ToolUsage:
|
||||
|
||||
except Exception as e:
|
||||
self.on_tool_error(tool=tool, tool_calling=calling, e=e)
|
||||
error_event_emitted = True
|
||||
self._run_attempts += 1
|
||||
if self._run_attempts > self._max_parsing_attempts:
|
||||
self._telemetry.tool_usage_error(llm=self.function_calling_llm)
|
||||
@@ -665,7 +669,7 @@ class ToolUsage:
|
||||
result = self._format_result(result=result)
|
||||
|
||||
finally:
|
||||
if started_event_emitted:
|
||||
if started_event_emitted and not error_event_emitted:
|
||||
self.on_tool_use_finished(
|
||||
tool=tool,
|
||||
tool_calling=calling,
|
||||
|
||||
@@ -177,4 +177,40 @@ class TestTriggeredByScope:
|
||||
raise ValueError("test error")
|
||||
except ValueError:
|
||||
pass
|
||||
assert get_triggering_event_id() is None
|
||||
assert get_triggering_event_id() is None
|
||||
|
||||
|
||||
def test_agent_scope_preserved_after_tool_error_event() -> None:
|
||||
from crewai.events import crewai_event_bus
|
||||
from crewai.events.types.tool_usage_events import (
|
||||
ToolUsageErrorEvent,
|
||||
ToolUsageStartedEvent,
|
||||
)
|
||||
|
||||
push_event_scope("crew-1", "crew_kickoff_started")
|
||||
push_event_scope("task-1", "task_started")
|
||||
push_event_scope("agent-1", "agent_execution_started")
|
||||
|
||||
crewai_event_bus.emit(
|
||||
None,
|
||||
ToolUsageStartedEvent(
|
||||
tool_name="test_tool",
|
||||
tool_args={},
|
||||
agent_key="test_agent",
|
||||
)
|
||||
)
|
||||
|
||||
crewai_event_bus.emit(
|
||||
None,
|
||||
ToolUsageErrorEvent(
|
||||
tool_name="test_tool",
|
||||
tool_args={},
|
||||
agent_key="test_agent",
|
||||
error=ValueError("test error"),
|
||||
)
|
||||
)
|
||||
|
||||
crewai_event_bus.flush()
|
||||
|
||||
assert get_current_parent_id() == "agent-1"
|
||||
|
||||
|
||||
@@ -10,7 +10,9 @@ from crewai import Agent, Task
|
||||
from crewai.events.event_bus import crewai_event_bus
|
||||
from crewai.events.types.tool_usage_events import (
|
||||
ToolSelectionErrorEvent,
|
||||
ToolUsageErrorEvent,
|
||||
ToolUsageFinishedEvent,
|
||||
ToolUsageStartedEvent,
|
||||
ToolValidateInputErrorEvent,
|
||||
)
|
||||
from crewai.tools import BaseTool
|
||||
@@ -744,3 +746,78 @@ def test_tool_usage_finished_event_with_cached_result():
|
||||
assert isinstance(event.started_at, datetime.datetime)
|
||||
assert isinstance(event.finished_at, datetime.datetime)
|
||||
assert event.type == "tool_usage_finished"
|
||||
|
||||
|
||||
def test_tool_error_does_not_emit_finished_event():
|
||||
from crewai.tools.tool_calling import ToolCalling
|
||||
|
||||
class FailingTool(BaseTool):
|
||||
name: str = "Failing Tool"
|
||||
description: str = "A tool that always fails"
|
||||
|
||||
def _run(self, **kwargs) -> str:
|
||||
raise ValueError("Intentional failure")
|
||||
|
||||
failing_tool = FailingTool().to_structured_tool()
|
||||
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.key = "test_agent_key"
|
||||
mock_agent.role = "test_agent_role"
|
||||
mock_agent._original_role = "test_agent_role"
|
||||
mock_agent.verbose = False
|
||||
mock_agent.fingerprint = None
|
||||
mock_agent.i18n.tools.return_value = {"name": "Add Image"}
|
||||
mock_agent.i18n.errors.return_value = "Error: {error}"
|
||||
mock_agent.i18n.slice.return_value = "Available tools: {tool_names}"
|
||||
|
||||
mock_task = MagicMock()
|
||||
mock_task.delegations = 0
|
||||
mock_task.name = "Test Task"
|
||||
mock_task.description = "A test task"
|
||||
mock_task.id = "test-task-id"
|
||||
|
||||
mock_action = MagicMock()
|
||||
mock_action.tool = "failing_tool"
|
||||
mock_action.tool_input = "{}"
|
||||
|
||||
tool_usage = ToolUsage(
|
||||
tools_handler=MagicMock(cache=None, last_used_tool=None),
|
||||
tools=[failing_tool],
|
||||
task=mock_task,
|
||||
function_calling_llm=None,
|
||||
agent=mock_agent,
|
||||
action=mock_action,
|
||||
)
|
||||
|
||||
started_events = []
|
||||
error_events = []
|
||||
finished_events = []
|
||||
error_received = threading.Event()
|
||||
|
||||
@crewai_event_bus.on(ToolUsageStartedEvent)
|
||||
def on_started(source, event):
|
||||
if event.tool_name == "failing_tool":
|
||||
started_events.append(event)
|
||||
|
||||
@crewai_event_bus.on(ToolUsageErrorEvent)
|
||||
def on_error(source, event):
|
||||
if event.tool_name == "failing_tool":
|
||||
error_events.append(event)
|
||||
error_received.set()
|
||||
|
||||
@crewai_event_bus.on(ToolUsageFinishedEvent)
|
||||
def on_finished(source, event):
|
||||
if event.tool_name == "failing_tool":
|
||||
finished_events.append(event)
|
||||
|
||||
tool_calling = ToolCalling(tool_name="failing_tool", arguments={})
|
||||
tool_usage.use(calling=tool_calling, tool_string="Action: failing_tool")
|
||||
|
||||
assert error_received.wait(timeout=5), "Timeout waiting for error event"
|
||||
crewai_event_bus.flush()
|
||||
|
||||
assert len(started_events) >= 1, "Expected at least one ToolUsageStartedEvent"
|
||||
assert len(error_events) >= 1, "Expected at least one ToolUsageErrorEvent"
|
||||
assert len(finished_events) == 0, (
|
||||
"ToolUsageFinishedEvent should NOT be emitted after ToolUsageErrorEvent"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user