[Feature][Java] Added retires to remote calls in MCP server#531
[Feature][Java] Added retires to remote calls in MCP server#531yanand0909 wants to merge 3 commits intoapache:mainfrom
Conversation
github-actions
bot
added
priority/major
yanand0909
force-pushed
the
add_retries
branch
from
b4dd9e3 to
06c7b6a
Compare
|
Hey @xintongsong, can you take a look at this PR. Thanks |
|
PR title has typo: "retires" should be "retries". |
| } | ||
| String message = e.getMessage(); | ||
| if (message != null) { | ||
| if (message.contains("503")) { |
There was a problem hiding this comment.
The string-contains check for "503" and "429" could match unrelated messages - e.g., a tool named "room-503-monitor" or an error like "processed 429 items". Would a more specific pattern like "HTTP 503" or a word-boundary check work here? Alternatively, if the MCP SDK throws typed exceptions for HTTP errors, checking the exception type would be more robust.
| private static final String FIELD_ENDPOINT = "endpoint"; | ||
| private static final String FIELD_HEADERS = "headers"; | ||
| private static final String FIELD_TIMEOUT_SECONDS = "timeoutSeconds"; | ||
| private static final String FIELD_TIMEOUT = "timeout"; |
There was a problem hiding this comment.
nit: There seems to be an inconsistency between FIELD_TIMEOUT ("timeout") and the existing FIELD_TIMEOUT_SECONDS ("timeoutSeconds"). The @JsonCreator constructor uses FIELD_TIMEOUT_SECONDS, but the descriptor-based constructor reads FIELD_TIMEOUT. This means the field name used depends on how the MCPServer is constructed (JSON deserialization vs descriptor), which could confuse users. Would it make sense to unify these to a single field name?
2 participants
Linked issue: 521
Purpose of change
Add user configurable retries for MCPServer remote calls
Tests
Added retry test and existing UT and IT
API
Yes
Documentation
doc-neededdoc-not-neededdoc-included