Skip to content

Commit d23bfde

Browse files
tzolovtzolov
committed
refactor: improve error handling with standard Java exceptions
Replace generic McpError with appropriate standard exceptions: - Use IllegalArgumentException for invalid input parameters - Use IllegalStateException for state-related issues - Use RuntimeException wrapper for initialization failures - Use McpError.builder() with proper error codes for protocol errors This change improves error handling clarity and follows Java best practices by using more specific exception types that better describe the error conditions. Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
1 parent eaed951 commit d23bfde

File tree

6 files changed

+30
-29
lines changed

6 files changed

+30
-29
lines changed

‎mcp/src/main/java/io/modelcontextprotocol/client/LifecycleInitializer.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,7 @@ public <T> Mono<T> withIntitialization(String actionName, Function<Initializatio
289289
return initializationJob.map(initializeResult -> this.initializationRef.get())
290290
.timeout(this.initializationTimeout)
291291
.onErrorResume(ex -> {
292-
logger.warn("Failed to initialize", ex);
293-
return Mono.error(
294-
new McpError("Client failed to initialize " + actionName + " due to: " + ex.getMessage()));
292+
return Mono.error(new RuntimeException("Client failed to initialize " + actionName, ex));
295293
})
296294
.flatMap(operation);
297295
});
@@ -316,8 +314,10 @@ private Mono<McpSchema.InitializeResult> doInitialize(DefaultInitialization init
316314
initializeResult.instructions());
317315

318316
if (!this.protocolVersions.contains(initializeResult.protocolVersion())) {
319-
return Mono.error(new McpError(
320-
"Unsupported protocol version from the server: " + initializeResult.protocolVersion()));
317+
return Mono.error(McpError.builder(-32602)
318+
.message("Unsupported protocol version")
319+
.data("Unsupported protocol version from the server: " + initializeResult.protocolVersion())
320+
.build());
321321
}
322322

323323
return mcpClientSession.sendNotification(McpSchema.METHOD_NOTIFICATION_INITIALIZED, null)

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

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,8 @@ public class McpAsyncClient {
190190
// Sampling Handler
191191
if (this.clientCapabilities.sampling() != null) {
192192
if (features.samplingHandler() == null) {
193-
throw new McpError("Sampling handler must not be null when client capabilities include sampling");
193+
throw new IllegalArgumentException(
194+
"Sampling handler must not be null when client capabilities include sampling");
194195
}
195196
this.samplingHandler = features.samplingHandler();
196197
requestHandlers.put(McpSchema.METHOD_SAMPLING_CREATE_MESSAGE, samplingCreateMessageHandler());
@@ -199,7 +200,8 @@ public class McpAsyncClient {
199200
// Elicitation Handler
200201
if (this.clientCapabilities.elicitation() != null) {
201202
if (features.elicitationHandler() == null) {
202-
throw new McpError("Elicitation handler must not be null when client capabilities include elicitation");
203+
throw new IllegalArgumentException(
204+
"Elicitation handler must not be null when client capabilities include elicitation");
203205
}
204206
this.elicitationHandler = features.elicitationHandler();
205207
requestHandlers.put(McpSchema.METHOD_ELICITATION_CREATE, elicitationCreateHandler());
@@ -413,15 +415,15 @@ public Mono<Object> ping() {
413415
public Mono<Void> addRoot(Root root) {
414416

415417
if (root == null) {
416-
return Mono.error(new McpError("Root must not be null"));
418+
return Mono.error(new IllegalArgumentException("Root must not be null"));
417419
}
418420

419421
if (this.clientCapabilities.roots() == null) {
420-
return Mono.error(new McpError("Client must be configured with roots capabilities"));
422+
return Mono.error(new IllegalStateException("Client must be configured with roots capabilities"));
421423
}
422424

423425
if (this.roots.containsKey(root.uri())) {
424-
return Mono.error(new McpError("Root with uri '" + root.uri() + "' already exists"));
426+
return Mono.error(new IllegalStateException("Root with uri '" + root.uri() + "' already exists"));
425427
}
426428

427429
this.roots.put(root.uri(), root);
@@ -447,11 +449,11 @@ public Mono<Void> addRoot(Root root) {
447449
public Mono<Void> removeRoot(String rootUri) {
448450

449451
if (rootUri == null) {
450-
return Mono.error(new McpError("Root uri must not be null"));
452+
return Mono.error(new IllegalArgumentException("Root uri must not be null"));
451453
}
452454

453455
if (this.clientCapabilities.roots() == null) {
454-
return Mono.error(new McpError("Client must be configured with roots capabilities"));
456+
return Mono.error(new IllegalStateException("Client must be configured with roots capabilities"));
455457
}
456458

457459
Root removed = this.roots.remove(rootUri);
@@ -469,7 +471,7 @@ public Mono<Void> removeRoot(String rootUri) {
469471
}
470472
return Mono.empty();
471473
}
472-
return Mono.error(new McpError("Root with uri '" + rootUri + "' not found"));
474+
return Mono.error(new IllegalStateException("Root with uri '" + rootUri + "' not found"));
473475
}
474476

475477
/**
@@ -540,7 +542,7 @@ private RequestHandler<ElicitResult> elicitationCreateHandler() {
540542
public Mono<McpSchema.CallToolResult> callTool(McpSchema.CallToolRequest callToolRequest) {
541543
return this.initializer.withIntitialization("calling tools", init -> {
542544
if (init.initializeResult().capabilities().tools() == null) {
543-
return Mono.error(new McpError("Server does not provide tools capability"));
545+
return Mono.error(new IllegalStateException("Server does not provide tools capability"));
544546
}
545547
return init.mcpSession()
546548
.sendRequest(McpSchema.METHOD_TOOLS_CALL, callToolRequest, CALL_TOOL_RESULT_TYPE_REF);
@@ -569,7 +571,7 @@ public Mono<McpSchema.ListToolsResult> listTools() {
569571
public Mono<McpSchema.ListToolsResult> listTools(String cursor) {
570572
return this.initializer.withIntitialization("listing tools", init -> {
571573
if (init.initializeResult().capabilities().tools() == null) {
572-
return Mono.error(new McpError("Server does not provide tools capability"));
574+
return Mono.error(new IllegalStateException("Server does not provide tools capability"));
573575
}
574576
return init.mcpSession()
575577
.sendRequest(McpSchema.METHOD_TOOLS_LIST, new McpSchema.PaginatedRequest(cursor),
@@ -633,7 +635,7 @@ public Mono<McpSchema.ListResourcesResult> listResources() {
633635
public Mono<McpSchema.ListResourcesResult> listResources(String cursor) {
634636
return this.initializer.withIntitialization("listing resources", init -> {
635637
if (init.initializeResult().capabilities().resources() == null) {
636-
return Mono.error(new McpError("Server does not provide the resources capability"));
638+
return Mono.error(new IllegalStateException("Server does not provide the resources capability"));
637639
}
638640
return init.mcpSession()
639641
.sendRequest(McpSchema.METHOD_RESOURCES_LIST, new McpSchema.PaginatedRequest(cursor),
@@ -665,7 +667,7 @@ public Mono<McpSchema.ReadResourceResult> readResource(McpSchema.Resource resour
665667
public Mono<McpSchema.ReadResourceResult> readResource(McpSchema.ReadResourceRequest readResourceRequest) {
666668
return this.initializer.withIntitialization("reading resources", init -> {
667669
if (init.initializeResult().capabilities().resources() == null) {
668-
return Mono.error(new McpError("Server does not provide the resources capability"));
670+
return Mono.error(new IllegalStateException("Server does not provide the resources capability"));
669671
}
670672
return init.mcpSession()
671673
.sendRequest(McpSchema.METHOD_RESOURCES_READ, readResourceRequest, READ_RESOURCE_RESULT_TYPE_REF);
@@ -703,7 +705,7 @@ public Mono<McpSchema.ListResourceTemplatesResult> listResourceTemplates() {
703705
public Mono<McpSchema.ListResourceTemplatesResult> listResourceTemplates(String cursor) {
704706
return this.initializer.withIntitialization("listing resource templates", init -> {
705707
if (init.initializeResult().capabilities().resources() == null) {
706-
return Mono.error(new McpError("Server does not provide the resources capability"));
708+
return Mono.error(new IllegalStateException("Server does not provide the resources capability"));
707709
}
708710
return init.mcpSession()
709711
.sendRequest(McpSchema.METHOD_RESOURCES_TEMPLATES_LIST, new McpSchema.PaginatedRequest(cursor),
@@ -863,7 +865,7 @@ private NotificationHandler asyncLoggingNotificationHandler(
863865
*/
864866
public Mono<Void> setLoggingLevel(LoggingLevel loggingLevel) {
865867
if (loggingLevel == null) {
866-
return Mono.error(new McpError("Logging level must not be null"));
868+
return Mono.error(new IllegalArgumentException("Logging level must not be null"));
867869
}
868870

869871
return this.initializer.withIntitialization("setting logging level", init -> {

‎mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,8 @@ void testAddRoot() {
487487
void testAddRootWithNullValue() {
488488
withClient(createMcpTransport(), mcpAsyncClient -> {
489489
StepVerifier.create(mcpAsyncClient.addRoot(null))
490-
.consumeErrorWith(e -> assertThat(e).isInstanceOf(McpError.class).hasMessage("Root must not be null"))
490+
.consumeErrorWith(e -> assertThat(e).isInstanceOf(IllegalArgumentException.class)
491+
.hasMessage("Root must not be null"))
491492
.verify();
492493
});
493494
}
@@ -506,7 +507,7 @@ void testRemoveRoot() {
506507
void testRemoveNonExistentRoot() {
507508
withClient(createMcpTransport(), mcpAsyncClient -> {
508509
StepVerifier.create(mcpAsyncClient.removeRoot("nonexistent-uri"))
509-
.consumeErrorWith(e -> assertThat(e).isInstanceOf(McpError.class)
510+
.consumeErrorWith(e -> assertThat(e).isInstanceOf(IllegalStateException.class)
510511
.hasMessage("Root with uri 'nonexistent-uri' not found"))
511512
.verify();
512513
});

‎mcp/src/test/java/io/modelcontextprotocol/client/LifecycleInitializerTests.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import org.mockito.MockitoAnnotations;
1717

1818
import io.modelcontextprotocol.spec.McpClientSession;
19-
import io.modelcontextprotocol.spec.McpError;
2019
import io.modelcontextprotocol.spec.McpSchema;
2120
import io.modelcontextprotocol.spec.McpTransportSessionNotFoundException;
2221
import reactor.core.publisher.Mono;
@@ -154,7 +153,7 @@ void shouldFailForUnsupportedProtocolVersion() {
154153
.thenReturn(Mono.just(unsupportedResult));
155154

156155
StepVerifier.create(initializer.withIntitialization("test", init -> Mono.just(init.initializeResult())))
157-
.expectError(McpError.class)
156+
.expectError(RuntimeException.class)
158157
.verify();
159158

160159
verify(mockClientSession, never()).sendNotification(eq(McpSchema.METHOD_NOTIFICATION_INITIALIZED), any());
@@ -178,7 +177,7 @@ void shouldTimeoutOnSlowInitialization() {
178177
init -> Mono.just(init.initializeResult())), () -> virtualTimeScheduler, Long.MAX_VALUE)
179178
.expectSubscription()
180179
.expectNoEvent(INITIALIZE_TIMEOUT)
181-
.expectError(McpError.class)
180+
.expectError(RuntimeException.class)
182181
.verify();
183182
}
184183

@@ -234,7 +233,7 @@ void shouldHandleInitializationFailure() {
234233
.thenReturn(Mono.error(new RuntimeException("Connection failed")));
235234

236235
StepVerifier.create(initializer.withIntitialization("test", init -> Mono.just(init.initializeResult())))
237-
.expectError(McpError.class)
236+
.expectError(RuntimeException.class)
238237
.verify();
239238

240239
assertThat(initializer.isInitialized()).isFalse();

‎mcp/src/test/java/io/modelcontextprotocol/client/McpAsyncClientResponseHandlerTests.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import com.fasterxml.jackson.core.type.TypeReference;
1414
import com.fasterxml.jackson.databind.ObjectMapper;
1515
import io.modelcontextprotocol.MockMcpClientTransport;
16-
import io.modelcontextprotocol.spec.McpError;
1716
import io.modelcontextprotocol.spec.McpSchema;
1817
import io.modelcontextprotocol.spec.McpSchema.ClientCapabilities;
1918
import io.modelcontextprotocol.spec.McpSchema.InitializeResult;
@@ -373,7 +372,7 @@ void testSamplingCreateMessageRequestHandlingWithNullHandler() {
373372
// Create client with sampling capability but null handler
374373
assertThatThrownBy(
375374
() -> McpClient.async(transport).capabilities(ClientCapabilities.builder().sampling().build()).build())
376-
.isInstanceOf(McpError.class)
375+
.isInstanceOf(IllegalArgumentException.class)
377376
.hasMessage("Sampling handler must not be null when client capabilities include sampling");
378377
}
379378

@@ -521,7 +520,7 @@ void testElicitationCreateRequestHandlingWithNullHandler() {
521520
// Create client with elicitation capability but null handler
522521
assertThatThrownBy(() -> McpClient.async(transport)
523522
.capabilities(ClientCapabilities.builder().elicitation().build())
524-
.build()).isInstanceOf(McpError.class)
523+
.build()).isInstanceOf(IllegalArgumentException.class)
525524
.hasMessage("Elicitation handler must not be null when client capabilities include elicitation");
526525
}
527526

‎mcp/src/test/java/io/modelcontextprotocol/client/McpClientProtocolVersionTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ void shouldFailForUnsupportedVersion() {
113113
new McpSchema.InitializeResult(unsupportedVersion, null,
114114
new McpSchema.Implementation("test-server", "1.0.0"), null),
115115
null));
116-
}).expectError(McpError.class).verify();
116+
}).expectError(RuntimeException.class).verify();
117117
}
118118
finally {
119119
StepVerifier.create(client.closeGracefully()).verifyComplete();

0 commit comments

Comments
 (0)