From 5a6be1831df9ef9feff2978cc3f97ea79791a064 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Wed, 16 Jul 2025 11:06:20 +0200 Subject: [PATCH 1/2] refactor: replace generic McpError with specific exception types BREAKING CHANGE: McpError is no longer used for validation and internal errors. Client code catching McpError should now catch specific exception types like IllegalArgumentException, IllegalStateException, McpTransportException, or McpClientInternalException. - Add McpTransportException for transport layer errors - Add McpClientInternalException for internal client failures - Use standard Java exceptions for validation errors - Implement structured error builder with JSON-RPC error codes - Change tool not found errors to be returned in CallToolResult - Update logging levels for unregistered notification handlers Signed-off-by: Christian Tzolov --- .../WebClientStreamableHttpTransport.java | 5 +- .../transport/WebFluxSseClientTransport.java | 10 ++- .../WebFluxSseServerTransportProvider.java | 25 +++--- .../WebMvcSseServerTransportProvider.java | 16 +++- .../client/AbstractMcpAsyncClientTests.java | 5 +- .../server/AbstractMcpAsyncServerTests.java | 22 ++--- .../server/AbstractMcpSyncServerTests.java | 26 +++--- .../client/LifecycleInitializer.java | 11 ++- .../client/McpAsyncClient.java | 35 ++++---- .../HttpClientSseClientTransport.java | 16 ++-- .../HttpClientStreamableHttpTransport.java | 18 ++--- .../server/McpAsyncServer.java | 81 ++++++++++--------- .../server/McpAsyncServerExchange.java | 16 ++-- ...HttpServletSseServerTransportProvider.java | 25 +++--- .../StdioServerTransportProvider.java | 9 ++- .../spec/McpClientInternalException.java | 24 ++++++ .../spec/McpClientSession.java | 2 +- .../modelcontextprotocol/spec/McpError.java | 38 ++++++++- .../spec/McpServerSession.java | 2 +- .../spec/McpTransportException.java | 27 +++++++ .../client/AbstractMcpAsyncClientTests.java | 5 +- .../client/LifecycleInitializerTests.java | 7 +- .../McpAsyncClientResponseHandlerTests.java | 5 +- .../client/McpClientProtocolVersionTests.java | 4 +- .../server/AbstractMcpAsyncServerTests.java | 22 ++--- .../server/AbstractMcpSyncServerTests.java | 26 +++--- .../server/McpAsyncServerExchangeTests.java | 44 +++++----- .../server/McpCompletionTests.java | 3 +- .../server/McpSyncServerExchangeTests.java | 16 ++-- .../server/StdioMcpSyncServerTests.java | 12 +++ ...rverTransportProviderIntegrationTests.java | 1 - 31 files changed, 349 insertions(+), 209 deletions(-) create mode 100644 mcp/src/main/java/io/modelcontextprotocol/spec/McpClientInternalException.java create mode 100644 mcp/src/main/java/io/modelcontextprotocol/spec/McpTransportException.java diff --git a/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/client/transport/WebClientStreamableHttpTransport.java b/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/client/transport/WebClientStreamableHttpTransport.java index d5ac8e95c..cb835988f 100644 --- a/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/client/transport/WebClientStreamableHttpTransport.java +++ b/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/client/transport/WebClientStreamableHttpTransport.java @@ -26,6 +26,7 @@ import io.modelcontextprotocol.spec.McpClientTransport; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpTransportException; import io.modelcontextprotocol.spec.McpTransportSession; import io.modelcontextprotocol.spec.McpTransportSessionNotFoundException; import io.modelcontextprotocol.spec.McpTransportStream; @@ -428,11 +429,11 @@ private Tuple2, Iterable> parse(Serve return Tuples.of(Optional.ofNullable(event.id()), List.of(message)); } catch (IOException ioException) { - throw new McpError("Error parsing JSON-RPC message: " + event.data()); + throw new McpTransportException("Error parsing JSON-RPC message: " + event.data(), ioException); } } else { - throw new McpError("Received unrecognized SSE event type: " + event.event()); + throw new McpTransportException("Received unrecognized SSE event type: " + event.event()); } } diff --git a/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/client/transport/WebFluxSseClientTransport.java b/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/client/transport/WebFluxSseClientTransport.java index 128cda4c3..12603895d 100644 --- a/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/client/transport/WebFluxSseClientTransport.java +++ b/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/client/transport/WebFluxSseClientTransport.java @@ -9,9 +9,12 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; + +import io.modelcontextprotocol.spec.McpClientInternalException; import io.modelcontextprotocol.spec.McpClientTransport; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpTransportException; import io.modelcontextprotocol.spec.McpSchema.JSONRPCMessage; import io.modelcontextprotocol.util.Assert; import org.slf4j.Logger; @@ -197,13 +200,14 @@ public Mono connect(Function, Mono> h this.inboundSubscription = events.concatMap(event -> Mono.just(event).handle((e, s) -> { if (ENDPOINT_EVENT_TYPE.equals(event.event())) { String messageEndpointUri = event.data(); - if (messageEndpointSink.tryEmitValue(messageEndpointUri).isSuccess()) { + var emitResult = messageEndpointSink.tryEmitValue(messageEndpointUri); + if (emitResult.isSuccess()) { s.complete(); } else { // TODO: clarify with the spec if multiple events can be // received - s.error(new McpError("Failed to handle SSE endpoint event")); + s.error(new McpClientInternalException("Failed to handle SSE endpoint event")); } } else if (MESSAGE_EVENT_TYPE.equals(event.event())) { @@ -216,7 +220,7 @@ else if (MESSAGE_EVENT_TYPE.equals(event.event())) { } } else { - s.error(new McpError("Received unrecognized SSE event type: " + event.event())); + s.error(new McpTransportException("Received unrecognized SSE event type: " + event.event())); } }).transform(handler)).subscribe(); diff --git a/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/server/transport/WebFluxSseServerTransportProvider.java b/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/server/transport/WebFluxSseServerTransportProvider.java index fde067f03..21fe516ed 100644 --- a/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/server/transport/WebFluxSseServerTransportProvider.java +++ b/mcp-spring/mcp-spring-webflux/src/main/java/io/modelcontextprotocol/server/transport/WebFluxSseServerTransportProvider.java @@ -7,9 +7,12 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.ErrorCodes; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse.JSONRPCError; import io.modelcontextprotocol.spec.McpServerSession; import io.modelcontextprotocol.spec.McpServerTransport; import io.modelcontextprotocol.spec.McpServerTransportProvider; +import io.modelcontextprotocol.spec.McpTransportException; import io.modelcontextprotocol.util.Assert; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -300,31 +303,29 @@ private Mono handleMessage(ServerRequest request) { } if (request.queryParam("sessionId").isEmpty()) { - return ServerResponse.badRequest().bodyValue(new McpError("Session ID missing in message endpoint")); + return ServerResponse.badRequest() + .bodyValue(McpError.builder(ErrorCodes.INVALID_REQUEST).message("Missing session ID param").build()); } - McpServerSession session = sessions.get(request.queryParam("sessionId").get()); + String sessionId = request.queryParam("sessionId").get(); + McpServerSession session = sessions.get(sessionId); if (session == null) { return ServerResponse.status(HttpStatus.NOT_FOUND) - .bodyValue(new McpError("Session not found: " + request.queryParam("sessionId").get())); + .bodyValue(McpError.builder(ErrorCodes.INVALID_REQUEST) + .message("SessionId not found") + .data("Empty sessionId: " + sessionId) + .build()); } return request.bodyToMono(String.class).flatMap(body -> { try { McpSchema.JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(objectMapper, body); - return session.handle(message).flatMap(response -> ServerResponse.ok().build()).onErrorResume(error -> { - logger.error("Error processing message: {}", error.getMessage()); - // TODO: instead of signalling the error, just respond with 200 OK - // - the error is signalled on the SSE connection - // return ServerResponse.ok().build(); - return ServerResponse.status(HttpStatus.INTERNAL_SERVER_ERROR) - .bodyValue(new McpError(error.getMessage())); - }); + return session.handle(message).flatMap(response -> ServerResponse.ok().build()); } catch (IllegalArgumentException | IOException e) { logger.error("Failed to deserialize message: {}", e.getMessage()); - return ServerResponse.badRequest().bodyValue(new McpError("Invalid message format")); + return ServerResponse.badRequest().bodyValue(new McpTransportException("Invalid message format", e)); } }); } diff --git a/mcp-spring/mcp-spring-webmvc/src/main/java/io/modelcontextprotocol/server/transport/WebMvcSseServerTransportProvider.java b/mcp-spring/mcp-spring-webmvc/src/main/java/io/modelcontextprotocol/server/transport/WebMvcSseServerTransportProvider.java index 114eff607..3502bbf54 100644 --- a/mcp-spring/mcp-spring-webmvc/src/main/java/io/modelcontextprotocol/server/transport/WebMvcSseServerTransportProvider.java +++ b/mcp-spring/mcp-spring-webmvc/src/main/java/io/modelcontextprotocol/server/transport/WebMvcSseServerTransportProvider.java @@ -13,8 +13,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.ErrorCodes; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse.JSONRPCError; import io.modelcontextprotocol.spec.McpServerTransport; import io.modelcontextprotocol.spec.McpServerTransportProvider; +import io.modelcontextprotocol.spec.McpTransportException; import io.modelcontextprotocol.spec.McpServerSession; import io.modelcontextprotocol.util.Assert; import org.slf4j.Logger; @@ -300,14 +303,19 @@ private ServerResponse handleMessage(ServerRequest request) { } if (request.param("sessionId").isEmpty()) { - return ServerResponse.badRequest().body(new McpError("Session ID missing in message endpoint")); + return ServerResponse.badRequest() + .body(McpError.builder(ErrorCodes.INVALID_REQUEST).message("Missing session ID param").build()); } String sessionId = request.param("sessionId").get(); McpServerSession session = sessions.get(sessionId); if (session == null) { - return ServerResponse.status(HttpStatus.NOT_FOUND).body(new McpError("Session not found: " + sessionId)); + return ServerResponse.status(HttpStatus.NOT_FOUND) + .body(McpError.builder(ErrorCodes.INVALID_REQUEST) + .message("SessionId not found") + .data("Empty sessionId: " + sessionId) + .build()); } try { @@ -321,11 +329,11 @@ private ServerResponse handleMessage(ServerRequest request) { } catch (IllegalArgumentException | IOException e) { logger.error("Failed to deserialize message: {}", e.getMessage()); - return ServerResponse.badRequest().body(new McpError("Invalid message format")); + return ServerResponse.badRequest().body(new McpTransportException("Invalid message format", e)); } catch (Exception e) { logger.error("Error handling message: {}", e.getMessage()); - return ServerResponse.status(HttpStatus.INTERNAL_SERVER_ERROR).body(new McpError(e.getMessage())); + return ServerResponse.status(HttpStatus.INTERNAL_SERVER_ERROR).body(new McpTransportException(e)); } } diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java index 067fbac2c..ea3739da5 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java @@ -486,7 +486,8 @@ void testAddRoot() { void testAddRootWithNullValue() { withClient(createMcpTransport(), mcpAsyncClient -> { StepVerifier.create(mcpAsyncClient.addRoot(null)) - .consumeErrorWith(e -> assertThat(e).isInstanceOf(McpError.class).hasMessage("Root must not be null")) + .consumeErrorWith(e -> assertThat(e).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Root must not be null")) .verify(); }); } @@ -505,7 +506,7 @@ void testRemoveRoot() { void testRemoveNonExistentRoot() { withClient(createMcpTransport(), mcpAsyncClient -> { StepVerifier.create(mcpAsyncClient.removeRoot("nonexistent-uri")) - .consumeErrorWith(e -> assertThat(e).isInstanceOf(McpError.class) + .consumeErrorWith(e -> assertThat(e).isInstanceOf(IllegalStateException.class) .hasMessage("Root with uri 'nonexistent-uri' not found")) .verify(); }); diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java index eb08bdcde..58d95cf2b 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java @@ -147,7 +147,7 @@ void testAddDuplicateTool() { .create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolSpecification(duplicateTool, (exchange, args) -> Mono.just(new CallToolResult(List.of(), false))))) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); }); @@ -168,7 +168,7 @@ void testAddDuplicateToolCall() { .tool(duplicateTool) .callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) .build())).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); }); @@ -254,7 +254,8 @@ void testRemoveNonexistentTool() { .build(); StepVerifier.create(mcpAsyncServer.removeTool("nonexistent-tool")).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Tool with name 'nonexistent-tool' not found"); + assertThat(error).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'nonexistent-tool' not found"); }); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); @@ -326,7 +327,7 @@ void testAddResourceWithNullSpecification() { StepVerifier.create(mcpAsyncServer.addResource((McpServerFeatures.AsyncResourceSpecification) null)) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Resource must not be null"); + assertThat(error).isInstanceOf(IllegalArgumentException.class).hasMessage("Resource must not be null"); }); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); @@ -345,7 +346,7 @@ void testAddResourceWithoutCapability() { resource, (exchange, req) -> Mono.just(new ReadResourceResult(List.of()))); StepVerifier.create(serverWithoutResources.addResource(specification)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with resource capabilities"); }); } @@ -358,7 +359,7 @@ void testRemoveResourceWithoutCapability() { .build(); StepVerifier.create(serverWithoutResources.removeResource(TEST_RESOURCE_URI)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with resource capabilities"); }); } @@ -385,7 +386,8 @@ void testAddPromptWithNullSpecification() { StepVerifier.create(mcpAsyncServer.addPrompt((McpServerFeatures.AsyncPromptSpecification) null)) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Prompt specification must not be null"); + assertThat(error).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Prompt specification must not be null"); }); } @@ -402,7 +404,7 @@ void testAddPromptWithoutCapability() { .of(new PromptMessage(McpSchema.Role.ASSISTANT, new McpSchema.TextContent("Test content")))))); StepVerifier.create(serverWithoutPrompts.addPrompt(specification)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); }); } @@ -415,7 +417,7 @@ void testRemovePromptWithoutCapability() { .build(); StepVerifier.create(serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); }); } @@ -448,7 +450,7 @@ void testRemoveNonexistentPrompt() { .build(); StepVerifier.create(mcpAsyncServer2.removePrompt("nonexistent-prompt")).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalArgumentException.class) .hasMessage("Prompt with name 'nonexistent-prompt' not found"); }); diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java index 4d5f9f772..6aac31b58 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java @@ -153,7 +153,7 @@ void testAddDuplicateTool() { assertThatThrownBy(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolSpecification(duplicateTool, (exchange, args) -> new CallToolResult(List.of(), false)))) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); @@ -172,7 +172,7 @@ void testAddDuplicateToolCall() { assertThatThrownBy(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder() .tool(duplicateTool) .callHandler((exchange, request) -> new CallToolResult(List.of(), false)) - .build())).isInstanceOf(McpError.class) + .build())).isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); @@ -256,7 +256,8 @@ void testRemoveNonexistentTool() { .capabilities(ServerCapabilities.builder().tools(true).build()) .build(); - assertThatThrownBy(() -> mcpSyncServer.removeTool("nonexistent-tool")).isInstanceOf(McpError.class) + assertThatThrownBy(() -> mcpSyncServer.removeTool("nonexistent-tool")) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name 'nonexistent-tool' not found"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); @@ -320,7 +321,7 @@ void testAddResourceWithNullSpecification() { .build(); assertThatThrownBy(() -> mcpSyncServer.addResource((McpServerFeatures.SyncResourceSpecification) null)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Resource must not be null"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); @@ -337,7 +338,8 @@ void testAddResourceWithoutCapability() { McpServerFeatures.SyncResourceSpecification specification = new McpServerFeatures.SyncResourceSpecification( resource, (exchange, req) -> new ReadResourceResult(List.of())); - assertThatThrownBy(() -> serverWithoutResources.addResource(specification)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutResources.addResource(specification)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with resource capabilities"); } @@ -347,7 +349,8 @@ void testRemoveResourceWithoutCapability() { .serverInfo("test-server", "1.0.0") .build(); - assertThatThrownBy(() -> serverWithoutResources.removeResource(TEST_RESOURCE_URI)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutResources.removeResource(TEST_RESOURCE_URI)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with resource capabilities"); } @@ -372,7 +375,7 @@ void testAddPromptWithNullSpecification() { .build(); assertThatThrownBy(() -> mcpSyncServer.addPrompt((McpServerFeatures.SyncPromptSpecification) null)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Prompt specification must not be null"); } @@ -387,7 +390,8 @@ void testAddPromptWithoutCapability() { (exchange, req) -> new GetPromptResult("Test prompt description", List .of(new PromptMessage(McpSchema.Role.ASSISTANT, new McpSchema.TextContent("Test content"))))); - assertThatThrownBy(() -> serverWithoutPrompts.addPrompt(specification)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutPrompts.addPrompt(specification)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); } @@ -397,7 +401,8 @@ void testRemovePromptWithoutCapability() { .serverInfo("test-server", "1.0.0") .build(); - assertThatThrownBy(() -> serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); } @@ -426,7 +431,8 @@ void testRemoveNonexistentPrompt() { .capabilities(ServerCapabilities.builder().prompts(true).build()) .build(); - assertThatThrownBy(() -> mcpSyncServer.removePrompt("nonexistent-prompt")).isInstanceOf(McpError.class) + assertThatThrownBy(() -> mcpSyncServer.removePrompt("nonexistent-prompt")) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Prompt with name 'nonexistent-prompt' not found"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); diff --git a/mcp/src/main/java/io/modelcontextprotocol/client/LifecycleInitializer.java b/mcp/src/main/java/io/modelcontextprotocol/client/LifecycleInitializer.java index e33fafa6a..c1a0ce51c 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/client/LifecycleInitializer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/client/LifecycleInitializer.java @@ -10,10 +10,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import io.modelcontextprotocol.spec.McpClientInternalException; import io.modelcontextprotocol.spec.McpClientSession; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpTransportSessionNotFoundException; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse.JSONRPCError; import io.modelcontextprotocol.util.Assert; import reactor.core.publisher.Mono; import reactor.core.publisher.Sinks; @@ -285,8 +287,7 @@ public Mono withIntitialization(String actionName, Function this.initializationRef.get()) .timeout(this.initializationTimeout) .onErrorResume(ex -> { - logger.warn("Failed to initialize", ex); - return Mono.error(new McpError("Client failed to initialize " + actionName)); + return Mono.error(new McpClientInternalException("Client failed to initialize " + actionName, ex)); }) .flatMap(operation); }); @@ -311,8 +312,10 @@ private Mono doInitialize(DefaultInitialization init initializeResult.instructions()); if (!this.protocolVersions.contains(initializeResult.protocolVersion())) { - return Mono.error(new McpError( - "Unsupported protocol version from the server: " + initializeResult.protocolVersion())); + return Mono.error(McpError.builder(-32602) + .message("Unsupported protocol version") + .data("Unsupported protocol version from the server: " + initializeResult.protocolVersion()) + .build()); } return mcpClientSession.sendNotification(McpSchema.METHOD_NOTIFICATION_INITIALIZED, null) diff --git a/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java b/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java index 9e861deba..5d90da5f3 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java +++ b/mcp/src/main/java/io/modelcontextprotocol/client/McpAsyncClient.java @@ -18,8 +18,9 @@ import com.fasterxml.jackson.core.type.TypeReference; import io.modelcontextprotocol.spec.McpClientSession; +import io.modelcontextprotocol.spec.McpClientSession.NotificationHandler; +import io.modelcontextprotocol.spec.McpClientSession.RequestHandler; import io.modelcontextprotocol.spec.McpClientTransport; -import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.ClientCapabilities; import io.modelcontextprotocol.spec.McpSchema.CreateMessageRequest; @@ -33,8 +34,6 @@ import io.modelcontextprotocol.spec.McpSchema.LoggingMessageNotification; import io.modelcontextprotocol.spec.McpSchema.PaginatedRequest; import io.modelcontextprotocol.spec.McpSchema.Root; -import io.modelcontextprotocol.spec.McpClientSession.NotificationHandler; -import io.modelcontextprotocol.spec.McpClientSession.RequestHandler; import io.modelcontextprotocol.util.Assert; import io.modelcontextprotocol.util.Utils; import reactor.core.publisher.Flux; @@ -184,7 +183,8 @@ public class McpAsyncClient { // Sampling Handler if (this.clientCapabilities.sampling() != null) { if (features.samplingHandler() == null) { - throw new McpError("Sampling handler must not be null when client capabilities include sampling"); + throw new IllegalArgumentException( + "Sampling handler must not be null when client capabilities include sampling"); } this.samplingHandler = features.samplingHandler(); requestHandlers.put(McpSchema.METHOD_SAMPLING_CREATE_MESSAGE, samplingCreateMessageHandler()); @@ -193,7 +193,8 @@ public class McpAsyncClient { // Elicitation Handler if (this.clientCapabilities.elicitation() != null) { if (features.elicitationHandler() == null) { - throw new McpError("Elicitation handler must not be null when client capabilities include elicitation"); + throw new IllegalArgumentException( + "Elicitation handler must not be null when client capabilities include elicitation"); } this.elicitationHandler = features.elicitationHandler(); requestHandlers.put(McpSchema.METHOD_ELICITATION_CREATE, elicitationCreateHandler()); @@ -400,15 +401,15 @@ public Mono ping() { public Mono addRoot(Root root) { if (root == null) { - return Mono.error(new McpError("Root must not be null")); + return Mono.error(new IllegalArgumentException("Root must not be null")); } if (this.clientCapabilities.roots() == null) { - return Mono.error(new McpError("Client must be configured with roots capabilities")); + return Mono.error(new IllegalStateException("Client must be configured with roots capabilities")); } if (this.roots.containsKey(root.uri())) { - return Mono.error(new McpError("Root with uri '" + root.uri() + "' already exists")); + return Mono.error(new IllegalStateException("Root with uri '" + root.uri() + "' already exists")); } this.roots.put(root.uri(), root); @@ -434,11 +435,11 @@ public Mono addRoot(Root root) { public Mono removeRoot(String rootUri) { if (rootUri == null) { - return Mono.error(new McpError("Root uri must not be null")); + return Mono.error(new IllegalArgumentException("Root uri must not be null")); } if (this.clientCapabilities.roots() == null) { - return Mono.error(new McpError("Client must be configured with roots capabilities")); + return Mono.error(new IllegalStateException("Client must be configured with roots capabilities")); } Root removed = this.roots.remove(rootUri); @@ -456,7 +457,7 @@ public Mono removeRoot(String rootUri) { } return Mono.empty(); } - return Mono.error(new McpError("Root with uri '" + rootUri + "' not found")); + return Mono.error(new IllegalStateException("Root with uri '" + rootUri + "' not found")); } /** @@ -527,7 +528,7 @@ private RequestHandler elicitationCreateHandler() { public Mono callTool(McpSchema.CallToolRequest callToolRequest) { return this.initializer.withIntitialization("calling tools", init -> { if (init.initializeResult().capabilities().tools() == null) { - return Mono.error(new McpError("Server does not provide tools capability")); + return Mono.error(new IllegalStateException("Server does not provide tools capability")); } return init.mcpSession() .sendRequest(McpSchema.METHOD_TOOLS_CALL, callToolRequest, CALL_TOOL_RESULT_TYPE_REF); @@ -556,7 +557,7 @@ public Mono listTools() { public Mono listTools(String cursor) { return this.initializer.withIntitialization("listing tools", init -> { if (init.initializeResult().capabilities().tools() == null) { - return Mono.error(new McpError("Server does not provide tools capability")); + return Mono.error(new IllegalStateException("Server does not provide tools capability")); } return init.mcpSession() .sendRequest(McpSchema.METHOD_TOOLS_LIST, new McpSchema.PaginatedRequest(cursor), @@ -620,7 +621,7 @@ public Mono listResources() { public Mono listResources(String cursor) { return this.initializer.withIntitialization("listing resources", init -> { if (init.initializeResult().capabilities().resources() == null) { - return Mono.error(new McpError("Server does not provide the resources capability")); + return Mono.error(new IllegalStateException("Server does not provide the resources capability")); } return init.mcpSession() .sendRequest(McpSchema.METHOD_RESOURCES_LIST, new McpSchema.PaginatedRequest(cursor), @@ -652,7 +653,7 @@ public Mono readResource(McpSchema.Resource resour public Mono readResource(McpSchema.ReadResourceRequest readResourceRequest) { return this.initializer.withIntitialization("reading resources", init -> { if (init.initializeResult().capabilities().resources() == null) { - return Mono.error(new McpError("Server does not provide the resources capability")); + return Mono.error(new IllegalStateException("Server does not provide the resources capability")); } return init.mcpSession() .sendRequest(McpSchema.METHOD_RESOURCES_READ, readResourceRequest, READ_RESOURCE_RESULT_TYPE_REF); @@ -690,7 +691,7 @@ public Mono listResourceTemplates() { public Mono listResourceTemplates(String cursor) { return this.initializer.withIntitialization("listing resource templates", init -> { if (init.initializeResult().capabilities().resources() == null) { - return Mono.error(new McpError("Server does not provide the resources capability")); + return Mono.error(new IllegalStateException("Server does not provide the resources capability")); } return init.mcpSession() .sendRequest(McpSchema.METHOD_RESOURCES_TEMPLATES_LIST, new McpSchema.PaginatedRequest(cursor), @@ -850,7 +851,7 @@ private NotificationHandler asyncLoggingNotificationHandler( */ public Mono setLoggingLevel(LoggingLevel loggingLevel) { if (loggingLevel == null) { - return Mono.error(new McpError("Logging level must not be null")); + return Mono.error(new IllegalArgumentException("Logging level must not be null")); } return this.initializer.withIntitialization("setting logging level", init -> { diff --git a/mcp/src/main/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransport.java b/mcp/src/main/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransport.java index 8598e3164..4f884086a 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransport.java +++ b/mcp/src/main/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransport.java @@ -21,9 +21,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.modelcontextprotocol.client.transport.ResponseSubscribers.ResponseEvent; +import io.modelcontextprotocol.spec.McpClientInternalException; import io.modelcontextprotocol.spec.McpClientTransport; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpTransportException; import io.modelcontextprotocol.spec.McpSchema.JSONRPCMessage; import io.modelcontextprotocol.util.Assert; import io.modelcontextprotocol.util.Utils; @@ -356,7 +358,7 @@ public Mono connect(Function, Mono> h return Flux.empty(); // No further processing needed } else { - sink.error(new McpError("Failed to handle SSE endpoint event")); + sink.error(new McpClientInternalException("Failed to handle SSE endpoint event")); } } else if (MESSAGE_EVENT_TYPE.equals(responseEvent.sseEvent().event())) { @@ -366,19 +368,16 @@ else if (MESSAGE_EVENT_TYPE.equals(responseEvent.sseEvent().event())) { return Flux.just(message); } else { - logger.error("Received unrecognized SSE event type: {}", - responseEvent.sseEvent().event()); - sink.error(new McpError( + sink.error(new McpTransportException( "Received unrecognized SSE event type: " + responseEvent.sseEvent().event())); } } catch (IOException e) { - logger.error("Error processing SSE event", e); - sink.error(new McpError("Error processing SSE event")); + sink.error(new McpTransportException("Error processing SSE event", e)); } } return Flux.error( - new RuntimeException("Failed to send message: " + responseEvent)); + new McpClientInternalException("Failed to send message: " + responseEvent)); }) .flatMap(jsonRpcMessage -> handler.apply(Mono.just(jsonRpcMessage))) @@ -447,8 +446,7 @@ private Mono serializeMessage(final JSONRPCMessage message) { return Mono.just(objectMapper.writeValueAsString(message)); } catch (IOException e) { - // TODO: why McpError and not RuntimeException? - return Mono.error(new McpError("Failed to serialize message")); + return Mono.error(new McpTransportException("Failed to serialize message", e)); } }); } diff --git a/mcp/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java b/mcp/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java index 12baa1706..1b2fdcd24 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java +++ b/mcp/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java @@ -29,8 +29,8 @@ import io.modelcontextprotocol.spec.DefaultMcpTransportSession; import io.modelcontextprotocol.spec.DefaultMcpTransportStream; import io.modelcontextprotocol.spec.McpClientTransport; -import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpTransportException; import io.modelcontextprotocol.spec.McpTransportSession; import io.modelcontextprotocol.spec.McpTransportSessionNotFoundException; import io.modelcontextprotocol.spec.McpTransportStream; @@ -260,8 +260,8 @@ private Mono reconnect(McpTransportStream stream) { } catch (IOException ioException) { - return Flux.error(new McpError( - "Error parsing JSON-RPC message: " + responseEvent.sseEvent().data())); + return Flux.error(new McpTransportException( + "Error parsing JSON-RPC message: " + responseEvent, ioException)); } } } @@ -282,8 +282,8 @@ else if (statusCode == BAD_REQUEST) { return Flux.error(exception); } - return Flux.error( - new McpError("Received unrecognized SSE event type: " + responseEvent.sseEvent().event())); + return Flux.error(new McpTransportException( + "Received unrecognized SSE event type: " + responseEvent.sseEvent().event())); }).flatMap(jsonrpcMessage -> this.handler.get().apply(Mono.just(jsonrpcMessage))) @@ -428,8 +428,8 @@ else if (contentType.contains(TEXT_EVENT_STREAM)) { return Flux.from(sessionStream.consumeSseStream(Flux.just(idWithMessages))); } catch (IOException ioException) { - return Flux.error( - new McpError("Error parsing JSON-RPC message: " + sseEvent.data())); + return Flux.error(new McpTransportException( + "Error parsing JSON-RPC message: " + sseEvent, ioException)); } }); } @@ -445,8 +445,8 @@ else if (contentType.contains(APPLICATION_JSON)) { return Mono.just(McpSchema.deserializeJsonRpcMessage(objectMapper, data)); } catch (IOException e) { - // TODO: this should be a McpTransportError - return Mono.error(e); + return Mono.error(new McpTransportException( + "Error deserializing JSON-RPC message: " + responseEvent, e)); } } logger.warn("Unknown media type {} returned for POST in session {}", contentType, diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java index 7131b10fa..c06af71b3 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java @@ -30,7 +30,9 @@ import io.modelcontextprotocol.spec.McpSchema.LoggingMessageNotification; import io.modelcontextprotocol.spec.McpSchema.ResourceTemplate; import io.modelcontextprotocol.spec.McpSchema.SetLevelRequest; +import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse.JSONRPCError; import io.modelcontextprotocol.spec.McpServerSession; import io.modelcontextprotocol.spec.McpServerTransportProvider; import io.modelcontextprotocol.util.Assert; @@ -281,16 +283,16 @@ private McpServerSession.NotificationHandler asyncRootsListChangedNotificationHa */ public Mono addTool(McpServerFeatures.AsyncToolSpecification toolSpecification) { if (toolSpecification == null) { - return Mono.error(new McpError("Tool specification must not be null")); + return Mono.error(new IllegalArgumentException("Tool specification must not be null")); } if (toolSpecification.tool() == null) { - return Mono.error(new McpError("Tool must not be null")); + return Mono.error(new IllegalArgumentException("Tool must not be null")); } if (toolSpecification.call() == null && toolSpecification.callHandler() == null) { - return Mono.error(new McpError("Tool call handler must not be null")); + return Mono.error(new IllegalArgumentException("Tool call handler must not be null")); } if (this.serverCapabilities.tools() == null) { - return Mono.error(new McpError("Server must be configured with tool capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with tool capabilities")); } var wrappedToolSpecification = withStructuredOutputHandling(this.jsonSchemaValidator, toolSpecification); @@ -298,8 +300,8 @@ public Mono addTool(McpServerFeatures.AsyncToolSpecification toolSpecifica return Mono.defer(() -> { // Check for duplicate tool names if (this.tools.stream().anyMatch(th -> th.tool().name().equals(wrappedToolSpecification.tool().name()))) { - return Mono.error( - new McpError("Tool with name '" + wrappedToolSpecification.tool().name() + "' already exists")); + return Mono.error(new IllegalArgumentException( + "Tool with name '" + wrappedToolSpecification.tool().name() + "' already exists")); } this.tools.add(wrappedToolSpecification); @@ -422,10 +424,10 @@ private static McpServerFeatures.AsyncToolSpecification withStructuredOutputHand */ public Mono removeTool(String toolName) { if (toolName == null) { - return Mono.error(new McpError("Tool name must not be null")); + return Mono.error(new IllegalArgumentException("Tool name must not be null")); } if (this.serverCapabilities.tools() == null) { - return Mono.error(new McpError("Server must be configured with tool capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with tool capabilities")); } return Mono.defer(() -> { @@ -438,7 +440,7 @@ public Mono removeTool(String toolName) { } return Mono.empty(); } - return Mono.error(new McpError("Tool with name '" + toolName + "' not found")); + return Mono.error(new IllegalArgumentException("Tool with name '" + toolName + "' not found")); }); } @@ -469,11 +471,17 @@ private McpServerSession.RequestHandler toolsCallRequestHandler( .findAny(); if (toolSpecification.isEmpty()) { - return Mono.error(new McpError("Tool not found: " + callToolRequest.name())); + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + return Mono.just(CallToolResult.builder() + .isError(true) + .content(List.of(new TextContent("Tool not found: " + callToolRequest.name()))) + .build()); + } + else { + return toolSpecification.get().callHandler().apply(exchange, callToolRequest); } - - return toolSpecification.map(tool -> tool.callHandler().apply(exchange, callToolRequest)) - .orElse(Mono.error(new McpError("Tool not found: " + callToolRequest.name()))); }; } @@ -488,16 +496,16 @@ private McpServerSession.RequestHandler toolsCallRequestHandler( */ public Mono addResource(McpServerFeatures.AsyncResourceSpecification resourceSpecification) { if (resourceSpecification == null || resourceSpecification.resource() == null) { - return Mono.error(new McpError("Resource must not be null")); + return Mono.error(new IllegalArgumentException("Resource must not be null")); } if (this.serverCapabilities.resources() == null) { - return Mono.error(new McpError("Server must be configured with resource capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with resource capabilities")); } return Mono.defer(() -> { if (this.resources.putIfAbsent(resourceSpecification.resource().uri(), resourceSpecification) != null) { - return Mono.error(new McpError( + return Mono.error(new IllegalArgumentException( "Resource with URI '" + resourceSpecification.resource().uri() + "' already exists")); } logger.debug("Added resource handler: {}", resourceSpecification.resource().uri()); @@ -515,10 +523,10 @@ public Mono addResource(McpServerFeatures.AsyncResourceSpecification resou */ public Mono removeResource(String resourceUri) { if (resourceUri == null) { - return Mono.error(new McpError("Resource URI must not be null")); + return Mono.error(new IllegalArgumentException("Resource URI must not be null")); } if (this.serverCapabilities.resources() == null) { - return Mono.error(new McpError("Server must be configured with resource capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with resource capabilities")); } return Mono.defer(() -> { @@ -530,7 +538,7 @@ public Mono removeResource(String resourceUri) { } return Mono.empty(); } - return Mono.error(new McpError("Resource with URI '" + resourceUri + "' not found")); + return Mono.error(new IllegalArgumentException("Resource with URI '" + resourceUri + "' not found")); }); } @@ -598,7 +606,7 @@ private McpServerSession.RequestHandler resourcesR .create(resourceSpecification.resource().uri()) .matches(resourceUri)) .findFirst() - .orElseThrow(() -> new McpError("Resource not found: " + resourceUri)); + .orElseThrow(() -> new IllegalArgumentException("Resource not found: " + resourceUri)); return specification.readHandler().apply(exchange, resourceRequest); }; @@ -615,18 +623,18 @@ private McpServerSession.RequestHandler resourcesR */ public Mono addPrompt(McpServerFeatures.AsyncPromptSpecification promptSpecification) { if (promptSpecification == null) { - return Mono.error(new McpError("Prompt specification must not be null")); + return Mono.error(new IllegalArgumentException("Prompt specification must not be null")); } if (this.serverCapabilities.prompts() == null) { - return Mono.error(new McpError("Server must be configured with prompt capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with prompt capabilities")); } return Mono.defer(() -> { McpServerFeatures.AsyncPromptSpecification specification = this.prompts .putIfAbsent(promptSpecification.prompt().name(), promptSpecification); if (specification != null) { - return Mono.error( - new McpError("Prompt with name '" + promptSpecification.prompt().name() + "' already exists")); + return Mono.error(new IllegalArgumentException( + "Prompt with name '" + promptSpecification.prompt().name() + "' already exists")); } logger.debug("Added prompt handler: {}", promptSpecification.prompt().name()); @@ -648,10 +656,10 @@ public Mono addPrompt(McpServerFeatures.AsyncPromptSpecification promptSpe */ public Mono removePrompt(String promptName) { if (promptName == null) { - return Mono.error(new McpError("Prompt name must not be null")); + return Mono.error(new IllegalArgumentException("Prompt name must not be null")); } if (this.serverCapabilities.prompts() == null) { - return Mono.error(new McpError("Server must be configured with prompt capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with prompt capabilities")); } return Mono.defer(() -> { @@ -666,7 +674,7 @@ public Mono removePrompt(String promptName) { } return Mono.empty(); } - return Mono.error(new McpError("Prompt with name '" + promptName + "' not found")); + return Mono.error(new IllegalArgumentException("Prompt with name '" + promptName + "' not found")); }); } @@ -703,7 +711,7 @@ private McpServerSession.RequestHandler promptsGetReq // Implement prompt retrieval logic here McpServerFeatures.AsyncPromptSpecification specification = this.prompts.get(promptRequest.name()); if (specification == null) { - return Mono.error(new McpError("Prompt not found: " + promptRequest.name())); + return Mono.error(new IllegalArgumentException("Prompt not found: " + promptRequest.name())); } return specification.promptHandler().apply(exchange, promptRequest); @@ -729,7 +737,7 @@ private McpServerSession.RequestHandler promptsGetReq public Mono loggingNotification(LoggingMessageNotification loggingMessageNotification) { if (loggingMessageNotification == null) { - return Mono.error(new McpError("Logging message must not be null")); + return Mono.error(new IllegalArgumentException("Logging message must not be null")); } if (loggingMessageNotification.level().level() < minLoggingLevel.level()) { @@ -764,11 +772,11 @@ private McpServerSession.RequestHandler completionComp McpSchema.CompleteRequest request = parseCompletionParams(params); if (request.ref() == null) { - return Mono.error(new McpError("ref must not be null")); + return Mono.error(new IllegalArgumentException("ref must not be null")); } if (request.ref().type() == null) { - return Mono.error(new McpError("type must not be null")); + return Mono.error(new IllegalArgumentException("type must not be null")); } String type = request.ref().type(); @@ -779,7 +787,7 @@ private McpServerSession.RequestHandler completionComp if (type.equals("ref/prompt") && request.ref() instanceof McpSchema.PromptReference promptReference) { McpServerFeatures.AsyncPromptSpecification promptSpec = this.prompts.get(promptReference.name()); if (promptSpec == null) { - return Mono.error(new McpError("Prompt not found: " + promptReference.name())); + return Mono.error(new IllegalArgumentException("Prompt not found: " + promptReference.name())); } if (!promptSpec.prompt() .arguments() @@ -788,19 +796,19 @@ private McpServerSession.RequestHandler completionComp .findFirst() .isPresent()) { - return Mono.error(new McpError("Argument not found: " + argumentName)); + return Mono.error(new IllegalArgumentException("Argument not found: " + argumentName)); } } if (type.equals("ref/resource") && request.ref() instanceof McpSchema.ResourceReference resourceReference) { McpServerFeatures.AsyncResourceSpecification resourceSpec = this.resources.get(resourceReference.uri()); if (resourceSpec == null) { - return Mono.error(new McpError("Resource not found: " + resourceReference.uri())); + return Mono.error(new IllegalArgumentException("Resource not found: " + resourceReference.uri())); } if (!uriTemplateManagerFactory.create(resourceSpec.resource().uri()) .getVariableNames() .contains(argumentName)) { - return Mono.error(new McpError("Argument not found: " + argumentName)); + return Mono.error(new IllegalArgumentException("Argument not found: " + argumentName)); } } @@ -808,7 +816,8 @@ private McpServerSession.RequestHandler completionComp McpServerFeatures.AsyncCompletionSpecification specification = this.completions.get(request.ref()); if (specification == null) { - return Mono.error(new McpError("AsyncCompletionSpecification not found: " + request.ref())); + return Mono + .error(new IllegalStateException("AsyncCompletionSpecification not found: " + request.ref())); } return specification.completionHandler().apply(exchange, request); diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServerExchange.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServerExchange.java index c0923e10e..48a74fc4e 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServerExchange.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServerExchange.java @@ -8,7 +8,7 @@ import java.util.Collections; import com.fasterxml.jackson.core.type.TypeReference; -import io.modelcontextprotocol.spec.McpError; + import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.LoggingLevel; import io.modelcontextprotocol.spec.McpSchema.LoggingMessageNotification; @@ -93,10 +93,11 @@ public McpSchema.Implementation getClientInfo() { */ public Mono createMessage(McpSchema.CreateMessageRequest createMessageRequest) { if (this.clientCapabilities == null) { - return Mono.error(new McpError("Client must be initialized. Call the initialize method first!")); + return Mono + .error(new IllegalStateException("Client must be initialized. Call the initialize method first!")); } if (this.clientCapabilities.sampling() == null) { - return Mono.error(new McpError("Client must be configured with sampling capabilities")); + return Mono.error(new IllegalArgumentException("Client must be configured with sampling capabilities")); } return this.session.sendRequest(McpSchema.METHOD_SAMPLING_CREATE_MESSAGE, createMessageRequest, CREATE_MESSAGE_RESULT_TYPE_REF); @@ -118,10 +119,11 @@ public Mono createMessage(McpSchema.CreateMessage */ public Mono createElicitation(McpSchema.ElicitRequest elicitRequest) { if (this.clientCapabilities == null) { - return Mono.error(new McpError("Client must be initialized. Call the initialize method first!")); + return Mono + .error(new IllegalStateException("Client must be initialized. Call the initialize method first!")); } if (this.clientCapabilities.elicitation() == null) { - return Mono.error(new McpError("Client must be configured with elicitation capabilities")); + return Mono.error(new IllegalArgumentException("Client must be configured with elicitation capabilities")); } return this.session.sendRequest(McpSchema.METHOD_ELICITATION_CREATE, elicitRequest, ELICITATION_RESULT_TYPE_REF); @@ -166,7 +168,7 @@ public Mono listRoots(String cursor) { public Mono loggingNotification(LoggingMessageNotification loggingMessageNotification) { if (loggingMessageNotification == null) { - return Mono.error(new McpError("Logging message must not be null")); + return Mono.error(new IllegalArgumentException("Logging message must not be null")); } return Mono.defer(() -> { @@ -185,7 +187,7 @@ public Mono loggingNotification(LoggingMessageNotification loggingMessageN */ public Mono progressNotification(McpSchema.ProgressNotification progressNotification) { if (progressNotification == null) { - return Mono.error(new McpError("Progress notification must not be null")); + return Mono.error(new IllegalArgumentException("Progress notification must not be null")); } return this.session.sendNotification(McpSchema.METHOD_NOTIFICATION_PROGRESS, progressNotification); } diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java b/mcp/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java index afdbff472..5e6626e8c 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java @@ -11,9 +11,12 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import io.modelcontextprotocol.spec.McpError; + import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpServerSession; import io.modelcontextprotocol.spec.McpServerTransport; @@ -25,8 +28,6 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -255,12 +256,11 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) // Get the session ID from the request parameter String sessionId = request.getParameter("sessionId"); if (sessionId == null) { - response.setContentType(APPLICATION_JSON); + // response.setContentType(APPLICATION_JSON); response.setCharacterEncoding(UTF_8); response.setStatus(HttpServletResponse.SC_BAD_REQUEST); - String jsonError = objectMapper.writeValueAsString(new McpError("Session ID missing in message endpoint")); PrintWriter writer = response.getWriter(); - writer.write(jsonError); + writer.write("Session ID missing in message endpoint"); writer.flush(); return; } @@ -268,12 +268,11 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) // Get the session from the sessions map McpServerSession session = sessions.get(sessionId); if (session == null) { - response.setContentType(APPLICATION_JSON); + // response.setContentType(APPLICATION_JSON); response.setCharacterEncoding(UTF_8); response.setStatus(HttpServletResponse.SC_NOT_FOUND); - String jsonError = objectMapper.writeValueAsString(new McpError("Session not found: " + sessionId)); PrintWriter writer = response.getWriter(); - writer.write(jsonError); + writer.write("Session not found: " + sessionId); writer.flush(); return; } @@ -296,13 +295,13 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) catch (Exception e) { logger.error("Error processing message: {}", e.getMessage()); try { - McpError mcpError = new McpError(e.getMessage()); - response.setContentType(APPLICATION_JSON); + // McpError mcpError = new McpError(e.getMessage()); + // response.setContentType(APPLICATION_JSON); response.setCharacterEncoding(UTF_8); response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - String jsonError = objectMapper.writeValueAsString(mcpError); + // String jsonError = objectMapper.writeValueAsString(mcpError); PrintWriter writer = response.getWriter(); - writer.write(jsonError); + writer.write(e.getMessage()); writer.flush(); } catch (IOException ex) { diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/transport/StdioServerTransportProvider.java b/mcp/src/main/java/io/modelcontextprotocol/server/transport/StdioServerTransportProvider.java index 9ef9c7829..0e28f0931 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/transport/StdioServerTransportProvider.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/transport/StdioServerTransportProvider.java @@ -14,17 +14,18 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import io.modelcontextprotocol.spec.McpError; + import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.JSONRPCMessage; import io.modelcontextprotocol.spec.McpServerSession; import io.modelcontextprotocol.spec.McpServerTransport; import io.modelcontextprotocol.spec.McpServerTransportProvider; import io.modelcontextprotocol.util.Assert; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.publisher.Sinks; @@ -99,7 +100,7 @@ public void setSessionFactory(McpServerSession.Factory sessionFactory) { @Override public Mono notifyClients(String method, Object params) { if (this.session == null) { - return Mono.error(new McpError("No session to close")); + return Mono.error(new IllegalStateException("Session not initialized")); } return this.session.sendNotification(method, params) .doOnError(e -> logger.error("Failed to send notification: {}", e.getMessage())); diff --git a/mcp/src/main/java/io/modelcontextprotocol/spec/McpClientInternalException.java b/mcp/src/main/java/io/modelcontextprotocol/spec/McpClientInternalException.java new file mode 100644 index 000000000..8c396538e --- /dev/null +++ b/mcp/src/main/java/io/modelcontextprotocol/spec/McpClientInternalException.java @@ -0,0 +1,24 @@ +package io.modelcontextprotocol.spec; + +public class McpClientInternalException extends RuntimeException { + + private static final long serialVersionUID = 1L; + + public McpClientInternalException(String message) { + super(message); + } + + public McpClientInternalException(String message, Throwable cause) { + super(message, cause); + } + + public McpClientInternalException(Throwable cause) { + super(cause); + } + + public McpClientInternalException(String message, Throwable cause, boolean enableSuppression, + boolean writableStackTrace) { + super(message, cause, enableSuppression, writableStackTrace); + } + +} diff --git a/mcp/src/main/java/io/modelcontextprotocol/spec/McpClientSession.java b/mcp/src/main/java/io/modelcontextprotocol/spec/McpClientSession.java index cc7d2abf8..033531f05 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/spec/McpClientSession.java +++ b/mcp/src/main/java/io/modelcontextprotocol/spec/McpClientSession.java @@ -221,7 +221,7 @@ private Mono handleIncomingNotification(McpSchema.JSONRPCNotification noti return Mono.defer(() -> { var handler = notificationHandlers.get(notification.method()); if (handler == null) { - logger.error("No handler registered for notification method: {}", notification.method()); + logger.warn("No handler registered for notification: {}", notification); return Mono.empty(); } return handler.handle(notification.params()); diff --git a/mcp/src/main/java/io/modelcontextprotocol/spec/McpError.java b/mcp/src/main/java/io/modelcontextprotocol/spec/McpError.java index 13e43240b..3cd91cc41 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/spec/McpError.java +++ b/mcp/src/main/java/io/modelcontextprotocol/spec/McpError.java @@ -4,6 +4,7 @@ package io.modelcontextprotocol.spec; import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse.JSONRPCError; +import io.modelcontextprotocol.util.Assert; public class McpError extends RuntimeException { @@ -14,12 +15,41 @@ public McpError(JSONRPCError jsonRpcError) { this.jsonRpcError = jsonRpcError; } - public McpError(Object error) { - super(error.toString()); - } - public JSONRPCError getJsonRpcError() { return jsonRpcError; } + public static Builder builder(int errorCode) { + return new Builder(errorCode); + } + + public static class Builder { + + private final int code; + + private String message; + + private Object data; + + private Builder(int code) { + this.code = code; + } + + public Builder message(String message) { + this.message = message; + return this; + } + + public Builder data(Object data) { + this.data = data; + return this; + } + + public McpError build() { + Assert.hasText(message, "message must not be empty"); + return new McpError(new JSONRPCError(code, message, data)); + } + + } + } \ No newline at end of file diff --git a/mcp/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java b/mcp/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java index 86906d859..1fc164ee5 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java +++ b/mcp/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java @@ -248,7 +248,7 @@ private Mono handleIncomingNotification(McpSchema.JSONRPCNotification noti var handler = notificationHandlers.get(notification.method()); if (handler == null) { - logger.error("No handler registered for notification method: {}", notification.method()); + logger.warn("No handler registered for notification: {}", notification); return Mono.empty(); } return this.exchangeSink.asMono().flatMap(exchange -> handler.handle(exchange, notification.params())); diff --git a/mcp/src/main/java/io/modelcontextprotocol/spec/McpTransportException.java b/mcp/src/main/java/io/modelcontextprotocol/spec/McpTransportException.java new file mode 100644 index 000000000..e2731d5e0 --- /dev/null +++ b/mcp/src/main/java/io/modelcontextprotocol/spec/McpTransportException.java @@ -0,0 +1,27 @@ +/* +* Copyright 2024 - 2024 the original author or authors. +*/ +package io.modelcontextprotocol.spec; + +public class McpTransportException extends RuntimeException { + + private static final long serialVersionUID = 1L; + + public McpTransportException(String message) { + super(message); + } + + public McpTransportException(String message, Throwable cause) { + super(message, cause); + } + + public McpTransportException(Throwable cause) { + super(cause); + } + + public McpTransportException(String message, Throwable cause, boolean enableSuppression, + boolean writableStackTrace) { + super(message, cause, enableSuppression, writableStackTrace); + } + +} diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java index e912e1dd6..3626d8ca0 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java @@ -487,7 +487,8 @@ void testAddRoot() { void testAddRootWithNullValue() { withClient(createMcpTransport(), mcpAsyncClient -> { StepVerifier.create(mcpAsyncClient.addRoot(null)) - .consumeErrorWith(e -> assertThat(e).isInstanceOf(McpError.class).hasMessage("Root must not be null")) + .consumeErrorWith(e -> assertThat(e).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Root must not be null")) .verify(); }); } @@ -506,7 +507,7 @@ void testRemoveRoot() { void testRemoveNonExistentRoot() { withClient(createMcpTransport(), mcpAsyncClient -> { StepVerifier.create(mcpAsyncClient.removeRoot("nonexistent-uri")) - .consumeErrorWith(e -> assertThat(e).isInstanceOf(McpError.class) + .consumeErrorWith(e -> assertThat(e).isInstanceOf(IllegalStateException.class) .hasMessage("Root with uri 'nonexistent-uri' not found")) .verify(); }); diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/LifecycleInitializerTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/LifecycleInitializerTests.java index c8d691924..a08dc1a26 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/LifecycleInitializerTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/LifecycleInitializerTests.java @@ -15,6 +15,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import io.modelcontextprotocol.spec.McpClientInternalException; import io.modelcontextprotocol.spec.McpClientSession; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; @@ -154,7 +155,7 @@ void shouldFailForUnsupportedProtocolVersion() { .thenReturn(Mono.just(unsupportedResult)); StepVerifier.create(initializer.withIntitialization("test", init -> Mono.just(init.initializeResult()))) - .expectError(McpError.class) + .expectError(McpClientInternalException.class) .verify(); verify(mockClientSession, never()).sendNotification(eq(McpSchema.METHOD_NOTIFICATION_INITIALIZED), any()); @@ -178,7 +179,7 @@ void shouldTimeoutOnSlowInitialization() { init -> Mono.just(init.initializeResult())), () -> virtualTimeScheduler, Long.MAX_VALUE) .expectSubscription() .expectNoEvent(INITIALIZE_TIMEOUT) - .expectError(McpError.class) + .expectError(McpClientInternalException.class) .verify(); } @@ -234,7 +235,7 @@ void shouldHandleInitializationFailure() { .thenReturn(Mono.error(new RuntimeException("Connection failed"))); StepVerifier.create(initializer.withIntitialization("test", init -> Mono.just(init.initializeResult()))) - .expectError(McpError.class) + .expectError(McpClientInternalException.class) .verify(); assertThat(initializer.isInitialized()).isFalse(); diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/McpAsyncClientResponseHandlerTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/McpAsyncClientResponseHandlerTests.java index e773c8381..592bc8971 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/McpAsyncClientResponseHandlerTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/McpAsyncClientResponseHandlerTests.java @@ -13,7 +13,6 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import io.modelcontextprotocol.MockMcpClientTransport; -import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.ClientCapabilities; import io.modelcontextprotocol.spec.McpSchema.InitializeResult; @@ -373,7 +372,7 @@ void testSamplingCreateMessageRequestHandlingWithNullHandler() { // Create client with sampling capability but null handler assertThatThrownBy( () -> McpClient.async(transport).capabilities(ClientCapabilities.builder().sampling().build()).build()) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Sampling handler must not be null when client capabilities include sampling"); } @@ -521,7 +520,7 @@ void testElicitationCreateRequestHandlingWithNullHandler() { // Create client with elicitation capability but null handler assertThatThrownBy(() -> McpClient.async(transport) .capabilities(ClientCapabilities.builder().elicitation().build()) - .build()).isInstanceOf(McpError.class) + .build()).isInstanceOf(IllegalArgumentException.class) .hasMessage("Elicitation handler must not be null when client capabilities include elicitation"); } diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/McpClientProtocolVersionTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/McpClientProtocolVersionTests.java index bf4738496..26ca00cd1 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/McpClientProtocolVersionTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/McpClientProtocolVersionTests.java @@ -8,7 +8,7 @@ import java.util.List; import io.modelcontextprotocol.MockMcpClientTransport; -import io.modelcontextprotocol.spec.McpError; +import io.modelcontextprotocol.spec.McpClientInternalException; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.InitializeResult; import org.junit.jupiter.api.Test; @@ -111,7 +111,7 @@ void shouldFailForUnsupportedVersion() { new McpSchema.InitializeResult(unsupportedVersion, null, new McpSchema.Implementation("test-server", "1.0.0"), null), null)); - }).expectError(McpError.class).verify(); + }).expectError(McpClientInternalException.class).verify(); } finally { StepVerifier.create(client.closeGracefully()).verifyComplete(); diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java index b5841e755..c04d88975 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java @@ -148,7 +148,7 @@ void testAddDuplicateTool() { .create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolSpecification(duplicateTool, (exchange, args) -> Mono.just(new CallToolResult(List.of(), false))))) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); }); @@ -169,7 +169,7 @@ void testAddDuplicateToolCall() { .tool(duplicateTool) .callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) .build())).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); }); @@ -255,7 +255,8 @@ void testRemoveNonexistentTool() { .build(); StepVerifier.create(mcpAsyncServer.removeTool("nonexistent-tool")).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Tool with name 'nonexistent-tool' not found"); + assertThat(error).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'nonexistent-tool' not found"); }); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); @@ -327,7 +328,7 @@ void testAddResourceWithNullSpecification() { StepVerifier.create(mcpAsyncServer.addResource((McpServerFeatures.AsyncResourceSpecification) null)) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Resource must not be null"); + assertThat(error).isInstanceOf(IllegalArgumentException.class).hasMessage("Resource must not be null"); }); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); @@ -346,7 +347,7 @@ void testAddResourceWithoutCapability() { resource, (exchange, req) -> Mono.just(new ReadResourceResult(List.of()))); StepVerifier.create(serverWithoutResources.addResource(specification)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with resource capabilities"); }); } @@ -359,7 +360,7 @@ void testRemoveResourceWithoutCapability() { .build(); StepVerifier.create(serverWithoutResources.removeResource(TEST_RESOURCE_URI)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with resource capabilities"); }); } @@ -386,7 +387,8 @@ void testAddPromptWithNullSpecification() { StepVerifier.create(mcpAsyncServer.addPrompt((McpServerFeatures.AsyncPromptSpecification) null)) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Prompt specification must not be null"); + assertThat(error).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Prompt specification must not be null"); }); } @@ -403,7 +405,7 @@ void testAddPromptWithoutCapability() { .of(new PromptMessage(McpSchema.Role.ASSISTANT, new McpSchema.TextContent("Test content")))))); StepVerifier.create(serverWithoutPrompts.addPrompt(specification)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); }); } @@ -416,7 +418,7 @@ void testRemovePromptWithoutCapability() { .build(); StepVerifier.create(serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); }); } @@ -449,7 +451,7 @@ void testRemoveNonexistentPrompt() { .build(); StepVerifier.create(mcpAsyncServer2.removePrompt("nonexistent-prompt")).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalArgumentException.class) .hasMessage("Prompt with name 'nonexistent-prompt' not found"); }); diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java index 208d2e749..c018d3172 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java @@ -154,7 +154,7 @@ void testAddDuplicateTool() { assertThatThrownBy(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolSpecification(duplicateTool, (exchange, args) -> new CallToolResult(List.of(), false)))) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); @@ -173,7 +173,7 @@ void testAddDuplicateToolCall() { assertThatThrownBy(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder() .tool(duplicateTool) .callHandler((exchange, request) -> new CallToolResult(List.of(), false)) - .build())).isInstanceOf(McpError.class) + .build())).isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); @@ -257,7 +257,8 @@ void testRemoveNonexistentTool() { .capabilities(ServerCapabilities.builder().tools(true).build()) .build(); - assertThatThrownBy(() -> mcpSyncServer.removeTool("nonexistent-tool")).isInstanceOf(McpError.class) + assertThatThrownBy(() -> mcpSyncServer.removeTool("nonexistent-tool")) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Tool with name 'nonexistent-tool' not found"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); @@ -321,7 +322,7 @@ void testAddResourceWithNullSpecification() { .build(); assertThatThrownBy(() -> mcpSyncServer.addResource((McpServerFeatures.SyncResourceSpecification) null)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Resource must not be null"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); @@ -338,7 +339,8 @@ void testAddResourceWithoutCapability() { McpServerFeatures.SyncResourceSpecification specification = new McpServerFeatures.SyncResourceSpecification( resource, (exchange, req) -> new ReadResourceResult(List.of())); - assertThatThrownBy(() -> serverWithoutResources.addResource(specification)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutResources.addResource(specification)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with resource capabilities"); } @@ -348,7 +350,8 @@ void testRemoveResourceWithoutCapability() { .serverInfo("test-server", "1.0.0") .build(); - assertThatThrownBy(() -> serverWithoutResources.removeResource(TEST_RESOURCE_URI)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutResources.removeResource(TEST_RESOURCE_URI)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with resource capabilities"); } @@ -373,7 +376,7 @@ void testAddPromptWithNullSpecification() { .build(); assertThatThrownBy(() -> mcpSyncServer.addPrompt((McpServerFeatures.SyncPromptSpecification) null)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Prompt specification must not be null"); } @@ -388,7 +391,8 @@ void testAddPromptWithoutCapability() { (exchange, req) -> new GetPromptResult("Test prompt description", List .of(new PromptMessage(McpSchema.Role.ASSISTANT, new McpSchema.TextContent("Test content"))))); - assertThatThrownBy(() -> serverWithoutPrompts.addPrompt(specification)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutPrompts.addPrompt(specification)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); } @@ -398,7 +402,8 @@ void testRemovePromptWithoutCapability() { .serverInfo("test-server", "1.0.0") .build(); - assertThatThrownBy(() -> serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); } @@ -427,7 +432,8 @@ void testRemoveNonexistentPrompt() { .capabilities(ServerCapabilities.builder().prompts(true).build()) .build(); - assertThatThrownBy(() -> mcpSyncServer.removePrompt("nonexistent-prompt")).isInstanceOf(McpError.class) + assertThatThrownBy(() -> mcpSyncServer.removePrompt("nonexistent-prompt")) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Prompt with name 'nonexistent-prompt' not found"); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/McpAsyncServerExchangeTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/McpAsyncServerExchangeTests.java index 39066a9a2..c21d53037 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/McpAsyncServerExchangeTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/McpAsyncServerExchangeTests.java @@ -4,32 +4,33 @@ package io.modelcontextprotocol.server; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; -import com.fasterxml.jackson.core.type.TypeReference; -import io.modelcontextprotocol.spec.McpError; -import io.modelcontextprotocol.spec.McpSchema; -import io.modelcontextprotocol.spec.McpServerSession; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; + +import com.fasterxml.jackson.core.type.TypeReference; + +import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpServerSession; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.reset; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - /** * Tests for {@link McpAsyncServerExchange}. * @@ -214,7 +215,8 @@ void testGetClientInfo() { @Test void testLoggingNotificationWithNullMessage() { StepVerifier.create(exchange.loggingNotification(null)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Logging message must not be null"); + assertThat(error).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Logging message must not be null"); }); } @@ -406,7 +408,7 @@ void testCreateElicitationWithNullCapabilities() { StepVerifier.create(exchangeWithNullCapabilities.createElicitation(elicitRequest)) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Client must be initialized. Call the initialize method first!"); }); @@ -430,7 +432,7 @@ void testCreateElicitationWithoutElicitationCapabilities() { .build(); StepVerifier.create(exchangeWithoutElicitation.createElicitation(elicitRequest)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalArgumentException.class) .hasMessage("Client must be configured with elicitation capabilities"); }); @@ -579,7 +581,7 @@ void testCreateMessageWithNullCapabilities() { StepVerifier.create(exchangeWithNullCapabilities.createMessage(createMessageRequest)) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Client must be initialized. Call the initialize method first!"); }); @@ -604,7 +606,7 @@ void testCreateMessageWithoutSamplingCapabilities() { .build(); StepVerifier.create(exchangeWithoutSampling.createMessage(createMessageRequest)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalArgumentException.class) .hasMessage("Client must be configured with sampling capabilities"); }); @@ -765,13 +767,13 @@ void testPingWithSuccessfulResponse() { @Test void testPingWithMcpError() { // Given - Mock an MCP-specific error during ping - McpError mcpError = new McpError("Server unavailable"); + var mcpError = new IllegalStateException("Server unavailable"); when(mockSession.sendRequest(eq(McpSchema.METHOD_PING), eq(null), any(TypeReference.class))) .thenReturn(Mono.error(mcpError)); // When & Then StepVerifier.create(exchange.ping()).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Server unavailable"); + assertThat(error).isInstanceOf(IllegalStateException.class).hasMessage("Server unavailable"); }); verify(mockSession, times(1)).sendRequest(eq(McpSchema.METHOD_PING), eq(null), any(TypeReference.class)); diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/McpCompletionTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/McpCompletionTests.java index 26b75946b..163439570 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/McpCompletionTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/McpCompletionTests.java @@ -248,7 +248,8 @@ void testCompletionErrorOnMissingContext() { // Check if database context is provided if (request.context() == null || request.context().arguments() == null || !request.context().arguments().containsKey("database")) { - throw new McpError("Please select a database first to see available tables"); + throw new IllegalArgumentException( + "Please select a database first to see available tables"); } // Normal completion if context is provided String db = request.context().arguments().get("database"); diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/McpSyncServerExchangeTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/McpSyncServerExchangeTests.java index 66d7695e8..a24cf1b37 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/McpSyncServerExchangeTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/McpSyncServerExchangeTests.java @@ -10,7 +10,6 @@ import java.util.Map; import com.fasterxml.jackson.core.type.TypeReference; -import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpServerSession; import org.junit.jupiter.api.BeforeEach; @@ -213,7 +212,7 @@ void testGetClientInfo() { @Test void testLoggingNotificationWithNullMessage() { - assertThatThrownBy(() -> exchange.loggingNotification(null)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> exchange.loggingNotification(null)).isInstanceOf(IllegalArgumentException.class) .hasMessage("Logging message must not be null"); } @@ -399,7 +398,7 @@ void testCreateElicitationWithNullCapabilities() { .build(); assertThatThrownBy(() -> exchangeWithNullCapabilities.createElicitation(elicitRequest)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalStateException.class) .hasMessage("Client must be initialized. Call the initialize method first!"); // Verify that sendRequest was never called due to null capabilities @@ -423,7 +422,7 @@ void testCreateElicitationWithoutElicitationCapabilities() { .build(); assertThatThrownBy(() -> exchangeWithoutElicitation.createElicitation(elicitRequest)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Client must be configured with elicitation capabilities"); // Verify that sendRequest was never called due to missing elicitation @@ -577,7 +576,7 @@ void testCreateMessageWithNullCapabilities() { .build(); assertThatThrownBy(() -> exchangeWithNullCapabilities.createMessage(createMessageRequest)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalStateException.class) .hasMessage("Client must be initialized. Call the initialize method first!"); // Verify that sendRequest was never called due to null capabilities @@ -602,7 +601,7 @@ void testCreateMessageWithoutSamplingCapabilities() { .build(); assertThatThrownBy(() -> exchangeWithoutSampling.createMessage(createMessageRequest)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Client must be configured with sampling capabilities"); // Verify that sendRequest was never called due to missing sampling capabilities @@ -763,12 +762,13 @@ void testPingWithSuccessfulResponse() { @Test void testPingWithMcpError() { // Given - Mock an MCP-specific error during ping - McpError mcpError = new McpError("Server unavailable"); + var mcpError = new IllegalStateException("Server unavailable"); when(mockSession.sendRequest(eq(McpSchema.METHOD_PING), eq(null), any(TypeReference.class))) .thenReturn(Mono.error(mcpError)); // When & Then - assertThatThrownBy(() -> exchange.ping()).isInstanceOf(McpError.class).hasMessage("Server unavailable"); + assertThatThrownBy(() -> exchange.ping()).isInstanceOf(IllegalStateException.class) + .hasMessage("Server unavailable"); verify(mockSession, times(1)).sendRequest(eq(McpSchema.METHOD_PING), eq(null), any(TypeReference.class)); } diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/StdioMcpSyncServerTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/StdioMcpSyncServerTests.java index a71c38493..7eea25e54 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/StdioMcpSyncServerTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/StdioMcpSyncServerTests.java @@ -5,7 +5,19 @@ package io.modelcontextprotocol.server; import io.modelcontextprotocol.server.transport.StdioServerTransportProvider; +import io.modelcontextprotocol.spec.McpError; +import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.CallToolResult; +import io.modelcontextprotocol.spec.McpSchema.ServerCapabilities; +import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpServerTransportProvider; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.assertThatCode; + +import java.util.List; + +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; /** diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProviderIntegrationTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProviderIntegrationTests.java index b04ecb3c4..d179818dc 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProviderIntegrationTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProviderIntegrationTests.java @@ -4,7 +4,6 @@ package io.modelcontextprotocol.server.transport; import java.time.Duration; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; From c96a357ec36ce82dcb7f56b2521ec0da1c363a32 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Thu, 24 Jul 2025 09:16:08 +0200 Subject: [PATCH 2/2] minor adjustment Signed-off-by: Christian Tzolov --- .../client/transport/ResponseSubscribers.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mcp/src/main/java/io/modelcontextprotocol/client/transport/ResponseSubscribers.java b/mcp/src/main/java/io/modelcontextprotocol/client/transport/ResponseSubscribers.java index eb9d3c65c..43bbdf8e1 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/client/transport/ResponseSubscribers.java +++ b/mcp/src/main/java/io/modelcontextprotocol/client/transport/ResponseSubscribers.java @@ -12,7 +12,7 @@ import org.reactivestreams.FlowAdapters; import org.reactivestreams.Subscription; -import io.modelcontextprotocol.spec.McpError; +import io.modelcontextprotocol.spec.McpTransportException; import reactor.core.publisher.BaseSubscriber; import reactor.core.publisher.FluxSink; @@ -168,8 +168,7 @@ else if (line.startsWith("event:")) { } else { // If the response is not successful, emit an error - // TODO: This should be a McpTransportError - this.sink.error(new McpError( + this.sink.error(new McpTransportException( "Invalid SSE response. Status code: " + this.responseInfo.statusCode() + " Line: " + line)); }