Skip to content

Commit 86c49aa

Browse files
chensk0601chensk0601
committed
fix: handle tool execution timeout/error causing IllegalStateException (#951)
ReActAgent throws IllegalStateException when tool calls timeout or fail, because no tool result is written to memory, leaving orphaned pending tool call states that crash the agent on subsequent requests. Root cause: - Tool execution timeout/error propagates without writing results to memory - Pending tool call state remains, blocking subsequent doCall() invocations - validateAndAddToolResults() throws when user message has no tool results Changes: - doCall(): detect pending tool calls without user-provided results and auto-generate error results to clear the pending state - executeToolCalls(): add onErrorResume to catch tool execution failures and generate error tool results instead of propagating exceptions - Add generateAndAddErrorToolResults() helper to create error results for orphaned pending tool calls This ensures the agent recovers gracefully from tool failures instead of crashing, and the model receives proper error feedback to continue processing. Closes #951
1 parent 7b68ae0 commit 86c49aa

File tree

2 files changed

+125
-24
lines changed

2 files changed

+125
-24
lines changed

‎agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java‎

Lines changed: 105 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,72 @@ protected Mono<Msg> doCall(List<Msg> msgs) {
252252
return executeIteration(0);
253253
}
254254

255-
// Has pending tools -> validate and add tool results
256-
validateAndAddToolResults(msgs, pendingIds);
257-
return hasPendingToolUse() ? acting(0) : executeIteration(0);
255+
// Has pending tools but no input -> resume (execute pending tools directly)
256+
if (msgs == null || msgs.isEmpty()) {
257+
return hasPendingToolUse() ? acting(0) : executeIteration(0);
258+
}
259+
260+
// Has pending tools + input -> check if user provided tool results
261+
List<ToolResultBlock> providedResults =
262+
msgs.stream()
263+
.flatMap(m -> m.getContentBlocks(ToolResultBlock.class).stream())
264+
.toList();
265+
266+
if (!providedResults.isEmpty()) {
267+
// User provided tool results -> validate and add
268+
validateAndAddToolResults(msgs, pendingIds);
269+
return hasPendingToolUse() ? acting(0) : executeIteration(0);
270+
}
271+
272+
// User sent a new message without tool results -> auto-recover from orphaned pending state
273+
log.warn(
274+
"Pending tool calls detected without results, auto-generating error results."
275+
+ " Pending IDs: {}",
276+
pendingIds);
277+
generateAndAddErrorToolResults(pendingIds);
278+
addToMemory(msgs);
279+
return executeIteration(0);
280+
}
281+
282+
/**
283+
* Generate error tool results for pending tool calls and add them to memory.
284+
* This is used to recover from situations where tool execution failed without
285+
* properly writing results to memory.
286+
*
287+
* @param pendingIds The set of pending tool use IDs
288+
*/
289+
private void generateAndAddErrorToolResults(Set<String> pendingIds) {
290+
Msg lastAssistant = findLastAssistantMsg();
291+
if (lastAssistant == null) {
292+
return;
293+
}
294+
295+
List<ToolUseBlock> pendingToolCalls =
296+
lastAssistant.getContentBlocks(ToolUseBlock.class).stream()
297+
.filter(toolUse -> pendingIds.contains(toolUse.getId()))
298+
.toList();
299+
300+
for (ToolUseBlock toolCall : pendingToolCalls) {
301+
ToolResultBlock errorResult =
302+
ToolResultBlock.builder()
303+
.id(toolCall.getId())
304+
.output(
305+
List.of(
306+
TextBlock.builder()
307+
.text(
308+
"[ERROR] Previous tool execution failed"
309+
+ " or was interrupted. Tool: "
310+
+ toolCall.getName())
311+
.build()))
312+
.build();
313+
Msg toolResultMsg =
314+
ToolResultMessageBuilder.buildToolResultMsg(errorResult, toolCall, getName());
315+
memory.addMessage(toolResultMsg);
316+
log.info(
317+
"Auto-generated error result for pending tool call: {} ({})",
318+
toolCall.getName(),
319+
toolCall.getId());
320+
}
258321
}
259322

260323
/**
@@ -592,6 +655,10 @@ private Msg buildSuspendedMsg(List<Map.Entry<ToolUseBlock, ToolResultBlock>> pen
592655
/**
593656
* Execute tool calls and return paired results.
594657
*
658+
* <p>If tool execution fails (timeout, error, etc.), this method generates error tool results
659+
* for all pending tool calls instead of propagating the error. This ensures the agent can
660+
* continue processing and the model receives proper error feedback.
661+
*
595662
* @param toolCalls The list of tool calls (potentially modified by PreActingEvent hooks)
596663
* @return Mono containing list of (ToolUseBlock, ToolResultBlock) pairs
597664
*/
@@ -602,7 +669,41 @@ private Mono<List<Map.Entry<ToolUseBlock, ToolResultBlock>>> executeToolCalls(
602669
results ->
603670
IntStream.range(0, toolCalls.size())
604671
.mapToObj(i -> Map.entry(toolCalls.get(i), results.get(i)))
605-
.toList());
672+
.toList())
673+
.onErrorResume(
674+
error -> {
675+
// Generate error tool results for all pending tool calls
676+
log.error(
677+
"Tool execution failed, generating error results for {} tool"
678+
+ " calls: {}",
679+
toolCalls.size(),
680+
error.getMessage());
681+
List<Map.Entry<ToolUseBlock, ToolResultBlock>> errorResults =
682+
toolCalls.stream()
683+
.map(
684+
toolCall -> {
685+
ToolResultBlock errorResult =
686+
ToolResultBlock.builder()
687+
.id(toolCall.getId())
688+
.output(
689+
List.of(
690+
TextBlock
691+
.builder()
692+
.text(
693+
"[ERROR]"
694+
+ " Tool"
695+
+ " execution"
696+
+ " failed:"
697+
+ " "
698+
+ error
699+
.getMessage())
700+
.build()))
701+
.build();
702+
return Map.entry(toolCall, errorResult);
703+
})
704+
.toList();
705+
return Mono.just(errorResults);
706+
});
606707
}
607708

608709
/**

‎agentscope-core/src/test/java/io/agentscope/core/hook/HookStopAgentTest.java‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import org.junit.jupiter.api.Test;
5353
import reactor.core.publisher.Flux;
5454
import reactor.core.publisher.Mono;
55-
import reactor.test.StepVerifier;
5655

5756
/**
5857
* Comprehensive tests for the Hook Stop Agent feature.
@@ -345,10 +344,15 @@ void testResumeWithToolResultMsg() {
345344
}
346345

347346
@Test
348-
@DisplayName("New message with pending tool calls throws error")
347+
@DisplayName("New message with pending tool calls auto-recovers")
349348
void testNewMsgWithPendingToolUseContinuesActing() {
350349
Msg toolUseMsg = createToolUseMsg("tool1", "test_tool", Map.of());
351-
setupModelToReturnToolUse(toolUseMsg);
350+
Msg textResponse =
351+
createAssistantTextMsg("Recovered after auto-generated error results");
352+
353+
when(mockModel.stream(anyList(), anyList(), any()))
354+
.thenReturn(createFluxFromMsg(toolUseMsg))
355+
.thenReturn(createFluxFromMsg(textResponse));
352356

353357
Hook stopHook = createPostReasoningStopHook();
354358

@@ -368,15 +372,11 @@ void testNewMsgWithPendingToolUseContinuesActing() {
368372
result1.hasContentBlocks(ToolUseBlock.class),
369373
"First call should return ToolUse message");
370374

371-
// Send a new regular message - should throw error due to pending tool calls
375+
// Send a new regular message - should auto-recover by generating error results
372376
Msg newMsg = createUserMsg("new message");
377+
Msg result2 = agent.call(newMsg).block(TEST_TIMEOUT);
373378

374-
StepVerifier.create(agent.call(newMsg))
375-
.expectErrorMatches(
376-
e ->
377-
e instanceof IllegalStateException
378-
&& e.getMessage().contains("pending tool calls"))
379-
.verify();
379+
assertNotNull(result2, "Agent should auto-recover and return a result");
380380
}
381381
}
382382

@@ -642,10 +642,14 @@ void testNormalCallAfterCompletion() {
642642
}
643643

644644
@Test
645-
@DisplayName("Agent throws error when adding regular message with pending tool calls")
645+
@DisplayName("Agent auto-recovers when adding regular message with pending tool calls")
646646
void testAgentHandlesPendingToolCallsGracefully() {
647647
Msg toolUseMsg = createToolUseMsg("tool1", "test_tool", Map.of());
648-
setupModelToReturnToolUse(toolUseMsg);
648+
Msg textResponse = createAssistantTextMsg("Recovered");
649+
650+
when(mockModel.stream(anyList(), anyList(), any()))
651+
.thenReturn(createFluxFromMsg(toolUseMsg))
652+
.thenReturn(createFluxFromMsg(textResponse));
649653

650654
Hook stopHook = createPostReasoningStopHook();
651655

@@ -661,14 +665,10 @@ void testAgentHandlesPendingToolCallsGracefully() {
661665

662666
agent.call(createUserMsg("test")).block(TEST_TIMEOUT);
663667

664-
// With new design, agent will throw error when adding regular message
665-
// with pending tool calls
666-
StepVerifier.create(agent.call(createUserMsg("new")))
667-
.expectErrorMatches(
668-
e ->
669-
e instanceof IllegalStateException
670-
&& e.getMessage().contains("pending tool calls"))
671-
.verify();
668+
// With new design, agent will auto-recover by generating error results
669+
// for pending tool calls and continue processing
670+
Msg result = agent.call(createUserMsg("new")).block(TEST_TIMEOUT);
671+
assertNotNull(result, "Agent should auto-recover and return a result");
672672
}
673673
}
674674

0 commit comments

Comments
 (0)