Skip to content

Commit 29bf4df

Browse files
committed
refactor: improve MCP client timeout handling and test structure
Add separate initializationTimeout configuration parameter distinct from requestTimeout Change ping() method to return Void instead of Object for better API clarity Refactor tests to consistently use StepVerifier instead of mixing assertion styles Rename ServletSse* test classes to HttpSse* for better naming consistency Remove redundant getTimeoutDuration() method in WebFlux tests Signed-off-by: Christian Tzolov <[email protected]>
1 parent c56ab94 commit 29bf4df

File tree

8 files changed

+198
-149
lines changed

8 files changed

+198
-149
lines changed

mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ public class McpAsyncClient {
8888

8989
/**
9090
* The max timeout to await for the client-server connection to be initialized.
91-
* Usually x2 the request timeout. // TODO should we make it configurable?
9291
*/
9392
private final Duration initializationTimeout;
9493

@@ -151,18 +150,21 @@ public class McpAsyncClient {
151150
* timeout.
152151
* @param transport the transport to use.
153152
* @param requestTimeout the session request-response timeout.
153+
* @param initializationTimeout the max timeout to await for the client-server
154154
* @param features the MCP Client supported features.
155155
*/
156-
McpAsyncClient(ClientMcpTransport transport, Duration requestTimeout, McpClientFeatures.Async features) {
156+
McpAsyncClient(ClientMcpTransport transport, Duration requestTimeout, Duration initializationTimeout,
157+
McpClientFeatures.Async features) {
157158

158159
Assert.notNull(transport, "Transport must not be null");
159160
Assert.notNull(requestTimeout, "Request timeout must not be null");
161+
Assert.notNull(initializationTimeout, "Initialization timeout must not be null");
160162

161163
this.clientInfo = features.clientInfo();
162164
this.clientCapabilities = features.clientCapabilities();
163165
this.transport = transport;
164166
this.roots = new ConcurrentHashMap<>(features.roots());
165-
this.initializationTimeout = requestTimeout.multipliedBy(2);
167+
this.initializationTimeout = initializationTimeout;
166168

167169
// Request Handlers
168170
Map<String, RequestHandler<?>> requestHandlers = new HashMap<>();
@@ -367,12 +369,13 @@ private <T> Mono<T> withInitializationCheck(String actionName,
367369

368370
/**
369371
* Sends a ping request to the server.
370-
* @return A Mono that completes with the server's ping response
372+
* @return A Mono that completes when the server responds to the ping
371373
*/
372-
public Mono<Object> ping() {
374+
public Mono<Void> ping() {
373375
return this.withInitializationCheck("pinging the server", initializedResult -> this.mcpSession
374376
.sendRequest(McpSchema.METHOD_PING, null, new TypeReference<Object>() {
375-
}));
377+
})
378+
.then());
376379
}
377380

378381
// --------------------------
@@ -771,7 +774,9 @@ private NotificationHandler asyncLoggingNotificationHandler(
771774
* @see McpSchema.LoggingLevel
772775
*/
773776
public Mono<Void> setLoggingLevel(LoggingLevel loggingLevel) {
774-
Assert.notNull(loggingLevel, "Logging level must not be null");
777+
if (loggingLevel == null) {
778+
return Mono.error(new McpError("Logging level must not be null"));
779+
}
775780

776781
return this.withInitializationCheck("setting logging level", initializedResult -> {
777782
String levelName = this.transport.unmarshalFrom(loggingLevel, new TypeReference<String>() {

mcp/src/main/java/io/modelcontextprotocol/client/McpClient.java

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ class SyncSpec {
157157

158158
private Duration requestTimeout = Duration.ofSeconds(20); // Default timeout
159159

160+
private Duration initializationTimeout = Duration.ofSeconds(20);
161+
160162
private ClientCapabilities capabilities;
161163

162164
private Implementation clientInfo = new Implementation("Java SDK MCP Client", "1.0.0");
@@ -193,6 +195,18 @@ public SyncSpec requestTimeout(Duration requestTimeout) {
193195
return this;
194196
}
195197

198+
/**
199+
* @param initializationTimeout The duration to wait for the initializaiton
200+
* lifecycle step to complete.
201+
* @return This builder instance for method chaining
202+
* @throws IllegalArgumentException if initializationTimeout is null
203+
*/
204+
public SyncSpec initializationTimeout(Duration initializationTimeout) {
205+
Assert.notNull(initializationTimeout, "Initialization timeout must not be null");
206+
this.initializationTimeout = initializationTimeout;
207+
return this;
208+
}
209+
196210
/**
197211
* Sets the client capabilities that will be advertised to the server during
198212
* connection initialization. Capabilities define what features the client
@@ -354,7 +368,8 @@ public McpSyncClient build() {
354368

355369
McpClientFeatures.Async asyncFeatures = McpClientFeatures.Async.fromSync(syncFeatures);
356370

357-
return new McpSyncClient(new McpAsyncClient(transport, this.requestTimeout, asyncFeatures));
371+
return new McpSyncClient(
372+
new McpAsyncClient(transport, this.requestTimeout, this.initializationTimeout, asyncFeatures));
358373
}
359374

360375
}
@@ -381,6 +396,8 @@ class AsyncSpec {
381396

382397
private Duration requestTimeout = Duration.ofSeconds(20); // Default timeout
383398

399+
private Duration initializationTimeout = Duration.ofSeconds(20);
400+
384401
private ClientCapabilities capabilities;
385402

386403
private Implementation clientInfo = new Implementation("Spring AI MCP Client", "0.3.1");
@@ -417,6 +434,18 @@ public AsyncSpec requestTimeout(Duration requestTimeout) {
417434
return this;
418435
}
419436

437+
/**
438+
* @param initializationTimeout The duration to wait for the initializaiton
439+
* lifecycle step to complete.
440+
* @return This builder instance for method chaining
441+
* @throws IllegalArgumentException if initializationTimeout is null
442+
*/
443+
public AsyncSpec initializationTimeout(Duration initializationTimeout) {
444+
Assert.notNull(initializationTimeout, "Initialization timeout must not be null");
445+
this.initializationTimeout = initializationTimeout;
446+
return this;
447+
}
448+
420449
/**
421450
* Sets the client capabilities that will be advertised to the server during
422451
* connection initialization. Capabilities define what features the client
@@ -574,7 +603,7 @@ public AsyncSpec loggingConsumers(
574603
* @return a new instance of {@link McpAsyncClient}.
575604
*/
576605
public McpAsyncClient build() {
577-
return new McpAsyncClient(this.transport, this.requestTimeout,
606+
return new McpAsyncClient(this.transport, this.requestTimeout, this.initializationTimeout,
578607
new McpClientFeatures.Async(this.clientInfo, this.capabilities, this.roots,
579608
this.toolsChangeConsumers, this.resourcesChangeConsumers, this.promptsChangeConsumers,
580609
this.loggingConsumers, this.samplingHandler));

mcp/src/main/java/io/modelcontextprotocol/client/McpSyncClient.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,10 @@ public void removeRoot(String rootUri) {
179179
}
180180

181181
/**
182-
* Send a synchronous ping request.
183-
* @return
182+
* Send a synchronous ping request to the server.
184183
*/
185-
public Object ping() {
186-
return this.delegate.ping().block();
184+
public void ping() {
185+
this.delegate.ping().block();
187186
}
188187

189188
// --------------------------

0 commit comments

Comments
 (0)