fix(tool): honor execution config retries for tool exceptions - #2830
Open
kevinyang03 wants to merge 4 commits into
Open
fix(tool): honor execution config retries for tool exceptions#2830kevinyang03 wants to merge 4 commits into
kevinyang03 wants to merge 4 commits into
Conversation
- Remove redundant null check in ExceptionUtils.unwrapExecutionWrapper - Fix inaccessible ToolExecutor javadoc link in McpTool - Trim verbose comments to match repository style
- Search the full cause chain (identity-guarded) for ToolSuspendException so suspensions hidden behind arbitrary wrappers are never retried - Drop the redundant Mono.defer in McpTool.callAsyncForExecution and the unreachable external-tool branch in ReflectiveFunctionTool - Document the execution-channel contract on AgentTool and clarify TOOL_DEFAULTS retry semantics when no retry predicate is configured - Add regression tests for Mono/Future/wrapped suspensions and for RETRYABLE_ERRORS not retrying deterministic failures
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AgentScope-Java Version
2.0.3-SNAPSHOT
Description
Fixes #2829.
Background: Tool calls never honored the retry settings in
ExecutionConfig.ToolExecutor.executeCore(...)converted tool failures into completedToolResultBlockerror results before the timeout/retry layers ran, soapplyRetryonly ever saw completed results. Verified onmainwith the reproduction from #2829: a tool throwingIOExceptiononce was invoked only once (maxAttempts=3withRETRYABLE_ERRORSconfigured), and in the timeout comparison the retry layer ran 3 attempts (Retries exhausted: 2/2) without ever re-invoking the tool.Changes:
AgentTool.callAsyncForExecution(...)(newdefaultmethod, backward compatible) plusToolMethodInvoker.invokeRawAsync(...), which keep failures as reactive error signals.ToolExecutor.executeWithInfrastructure(...)now runs this channel through scheduling → timeout → retry → shutdown and converts the final error into an error result only after retries are exhausted. The existingcallAsync(...)contract — failures surface as error results on direct calls — is unchanged and covered by tests for custom, annotation-based, and MCP tools.TimeoutException, whichExecutionConfig.RETRYABLE_ERRORSrecognizes.onRetryExhaustedThrow((spec, signal) -> signal.failure()), instead of the genericRetries exhausted: N/Nmessage.ToolSuspendExceptionanywhere in the cause chain (ExceptionUtils.findToolSuspendException(...), identity-guarded against circular causes) so suspensions — even wrapped in reflection/future/arbitrary exceptions — become suspended results and are never retried, including under the retry-everything fallback when noretryOnpredicate is configured.isError=true) as completed error results so they are never retried.AgentTooland the tool retry semantics onExecutionConfig.TOOL_DEFAULTS.How to test:
mvn -pl agentscope-core test -Dtest=ToolExecutorTest— 18 new end-to-end tests (class total: 30), covering retry of annotation tools (sync/Mono/CompletableFuture) and MCP transport failures, no-retry for protocol-level business errors andToolSuspendException(raw and wrapped), retry-exhaustion error results with id/name, timeout retries, retry isolation in parallel batches, and the direct-call compatibility contract.state=RUNNING, output["recovered"]), and the timeout comparison re-invokes the tool on every attempt with the root cause preserved (Tool execution timeout after PT0.1S).Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test) —mvn -pl agentscope-core clean verifypasses locally: 2312 tests, 0 failures, 0 errors (9 pre-existing skips); the full-reactorclean verifyruns in CImvn -pl agentscope-core javadoc:javadocpasses withdoclint=all,-missingdefaultmethod, and theExecutionConfigjavadoc is updated in code