From 272b29a017e1ed119155c8d6fb9ed52accb9d118 Mon Sep 17 00:00:00 2001 From: devcrocod Date: Wed, 29 Oct 2025 19:35:16 +0100 Subject: [PATCH] Fix: Return CallToolResult with isError for tool errors instead of throwing exceptions Update server-side tool call handling to conform with MCP specification: - handleCallTool now returns CallToolResult with isError=true for non-existent tools - Added exception handling to catch tool execution errors and return them as CallToolResult with isError=true - Updated client-side tests to expect CallToolResult with isError instead of protocol-level exceptions - Updated server-side integration tests to verify new error handling behavior --- .../kotlin/sdk/server/Server.kt | 25 +++++++-- .../kotlin/AbstractToolIntegrationTest.kt | 39 ++++++++------ .../KotlinClientTsServerEdgeCasesTestSse.kt | 54 +++++++++---------- .../KotlinClientTsServerEdgeCasesTestStdio.kt | 54 +++++++++---------- 4 files changed, 95 insertions(+), 77 deletions(-) diff --git a/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt index 18283e47..8c010560 100644 --- a/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt +++ b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt @@ -21,6 +21,7 @@ import io.modelcontextprotocol.kotlin.sdk.ReadResourceRequest import io.modelcontextprotocol.kotlin.sdk.ReadResourceResult import io.modelcontextprotocol.kotlin.sdk.Resource import io.modelcontextprotocol.kotlin.sdk.ServerCapabilities +import io.modelcontextprotocol.kotlin.sdk.TextContent import io.modelcontextprotocol.kotlin.sdk.Tool import io.modelcontextprotocol.kotlin.sdk.ToolAnnotations import io.modelcontextprotocol.kotlin.sdk.shared.ProtocolOptions @@ -32,6 +33,7 @@ import kotlinx.collections.immutable.minus import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.persistentMapOf import kotlinx.collections.immutable.toPersistentSet +import kotlinx.coroutines.CancellationException private val logger = KotlinLogging.logger {} @@ -515,13 +517,30 @@ public open class Server( private suspend fun handleCallTool(request: CallToolRequest): CallToolResult { logger.debug { "Handling tool call request for tool: ${request.name}" } + + // Check if tool exists val tool = _tools.value[request.name] ?: run { logger.error { "Tool not found: ${request.name}" } - throw IllegalArgumentException("Tool not found: ${request.name}") + return CallToolResult( + content = listOf(TextContent(text = "Tool ${request.name} not found")), + isError = true, + ) } - logger.trace { "Executing tool ${request.name} with input: ${request.arguments}" } - return tool.handler(request) + + // Execute tool handler and catch any errors + return try { + logger.trace { "Executing tool ${request.name} with input: ${request.arguments}" } + tool.handler(request) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + logger.error(e) { "Error executing tool ${request.name}" } + CallToolResult( + content = listOf(TextContent(text = "Error executing tool ${request.name}: ${e.message}")), + isError = true, + ) + } } private suspend fun handleListPrompts(): ListPromptsResult { diff --git a/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt b/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt index 41a951b1..ca88723f 100644 --- a/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt +++ b/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt @@ -21,7 +21,6 @@ import kotlinx.serialization.json.buildJsonArray import kotlinx.serialization.json.buildJsonObject import kotlinx.serialization.json.put import org.junit.jupiter.api.Test -import org.junit.jupiter.api.assertThrows import java.text.DecimalFormat import java.text.DecimalFormatSymbols import java.util.Locale @@ -527,16 +526,18 @@ abstract class AbstractToolIntegrationTest : KotlinTestBase() { "message" to "My exception message", ) - val exception = assertThrows { - runBlocking { - client.callTool(errorToolName, exceptionArgs) - } - } + val exceptionResult = client.callTool(errorToolName, exceptionArgs) as CallToolResultBase - val msg = exception.message ?: "" - val expectedMessage = "JSONRPCError(code=InternalError, message=My exception message, data={})" + assertTrue(exceptionResult.isError ?: false, "isError should be true for exception") - assertEquals(expectedMessage, msg, "Unexpected error message for exception") + val exceptionContent = exceptionResult.content.firstOrNull { it is TextContent } as? TextContent + assertNotNull(exceptionContent, "Error content should be present in the result") + + val exceptionText = exceptionContent.text ?: "" + assertTrue( + exceptionText.contains("Error executing tool") && exceptionText.contains("My exception message"), + "Error message should contain the exception details", + ) } @Test @@ -770,15 +771,21 @@ abstract class AbstractToolIntegrationTest : KotlinTestBase() { val nonExistentToolName = "non-existent-tool" val arguments = mapOf("text" to "Test") - val exception = assertThrows { - runBlocking { - client.callTool(nonExistentToolName, arguments) - } + val result = runBlocking { + client.callTool(nonExistentToolName, arguments) } - val msg = exception.message ?: "" - val expectedMessage = "JSONRPCError(code=InternalError, message=Tool not found: non-existent-tool, data={})" + assertNotNull(result, "Tool call result should not be null") + val callResult = result as CallToolResult + assertTrue(callResult.isError ?: false, "isError should be true for non-existent tool") - assertEquals(expectedMessage, msg, "Unexpected error message for non-existent tool") + val textContent = callResult.content.firstOrNull { it is TextContent } as? TextContent + assertNotNull(textContent, "Error content should be present in the result") + + val errorText = textContent.text ?: "" + assertTrue( + errorText.contains("non-existent-tool") && errorText.contains("not found"), + "Error message should indicate the tool was not found", + ) } } diff --git a/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/typescript/sse/KotlinClientTsServerEdgeCasesTestSse.kt b/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/typescript/sse/KotlinClientTsServerEdgeCasesTestSse.kt index 1585aa5e..de275666 100644 --- a/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/typescript/sse/KotlinClientTsServerEdgeCasesTestSse.kt +++ b/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/typescript/sse/KotlinClientTsServerEdgeCasesTestSse.kt @@ -17,7 +17,6 @@ import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.Timeout -import org.junit.jupiter.api.assertThrows import java.util.concurrent.TimeUnit import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -74,16 +73,19 @@ class KotlinClientTsServerEdgeCasesTestSse : TsTestBase() { val nonExistentToolName = "non-existent-tool" val arguments = mapOf("name" to "TestUser") - val exception = assertThrows { - client.callTool(nonExistentToolName, arguments) - } + val result = client.callTool(nonExistentToolName, arguments) + assertNotNull(result, "Tool call result should not be null") + + val callResult = result as CallToolResult + assertTrue(callResult.isError ?: false, "isError should be true for non-existent tool") - val expectedMessage = - "JSONRPCError(code=InvalidParams, message=MCP error -32602: Tool non-existent-tool not found, data={})" - assertEquals( - expectedMessage, - exception.message, - "Unexpected error message for non-existent tool", + val textContent = callResult.content.firstOrNull { it is TextContent } as? TextContent + assertNotNull(textContent, "Error content should be present in the result") + + val errorText = textContent.text ?: "" + assertTrue( + errorText.contains("non-existent-tool") && errorText.contains("not found"), + "Error message should indicate the tool was not found", ) } } @@ -176,26 +178,20 @@ class KotlinClientTsServerEdgeCasesTestSse : TsTestBase() { "name" to JsonObject(mapOf("nested" to JsonPrimitive("value"))), ) - val exception = assertThrows { - client.callTool("greet", invalidArguments) - } + val result = client.callTool("greet", invalidArguments) + assertNotNull(result, "Tool call result should not be null") + + val callResult = result as CallToolResult + assertTrue(callResult.isError ?: false, "isError should be true for invalid arguments") - val msg = exception.message ?: "" - val expectedMessage = """ - JSONRPCError(code=InvalidParams, message=MCP error -32602: Invalid arguments for tool greet: [ - { - "code": "invalid_type", - "expected": "string", - "received": "object", - "path": [ - "name" - ], - "message": "Expected string, received object" - } - ], data={}) - """.trimIndent() - - assertEquals(expectedMessage, msg, "Unexpected error message for invalid arguments") + val textContent = callResult.content.firstOrNull { it is TextContent } as? TextContent + assertNotNull(textContent, "Error content should be present in the result") + + val errorText = textContent.text ?: "" + assertTrue( + errorText.contains("Invalid arguments") && errorText.contains("greet"), + "Error message should indicate invalid arguments for tool greet", + ) } } diff --git a/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/typescript/stdio/KotlinClientTsServerEdgeCasesTestStdio.kt b/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/typescript/stdio/KotlinClientTsServerEdgeCasesTestStdio.kt index 88a46856..b6e385f9 100644 --- a/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/typescript/stdio/KotlinClientTsServerEdgeCasesTestStdio.kt +++ b/kotlin-sdk-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/typescript/stdio/KotlinClientTsServerEdgeCasesTestStdio.kt @@ -14,7 +14,6 @@ import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive import org.junit.jupiter.api.Test import org.junit.jupiter.api.Timeout -import org.junit.jupiter.api.assertThrows import java.util.concurrent.TimeUnit import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -31,16 +30,19 @@ class KotlinClientTsServerEdgeCasesTestStdio : TsTestBase() { val nonExistentToolName = "non-existent-tool" val arguments = mapOf("name" to "TestUser") - val exception = assertThrows { - client.callTool(nonExistentToolName, arguments) - } + val result = client.callTool(nonExistentToolName, arguments) + assertNotNull(result, "Tool call result should not be null") + + val callResult = result as CallToolResult + assertTrue(callResult.isError ?: false, "isError should be true for non-existent tool") - val expectedMessage = - "JSONRPCError(code=InvalidParams, message=MCP error -32602: Tool non-existent-tool not found, data={})" - assertEquals( - expectedMessage, - exception.message, - "Unexpected error message for non-existent tool", + val textContent = callResult.content.firstOrNull { it is TextContent } as? TextContent + assertNotNull(textContent, "Error content should be present in the result") + + val errorText = textContent.text ?: "" + assertTrue( + errorText.contains("non-existent-tool") && errorText.contains("not found"), + "Error message should indicate the tool was not found", ) } } @@ -133,26 +135,20 @@ class KotlinClientTsServerEdgeCasesTestStdio : TsTestBase() { "name" to JsonObject(mapOf("nested" to JsonPrimitive("value"))), ) - val exception = assertThrows { - client.callTool("greet", invalidArguments) - } + val result = client.callTool("greet", invalidArguments) + assertNotNull(result, "Tool call result should not be null") + + val callResult = result as CallToolResult + assertTrue(callResult.isError ?: false, "isError should be true for invalid arguments") - val msg = exception.message ?: "" - val expectedMessage = """ - JSONRPCError(code=InvalidParams, message=MCP error -32602: Invalid arguments for tool greet: [ - { - "code": "invalid_type", - "expected": "string", - "received": "object", - "path": [ - "name" - ], - "message": "Expected string, received object" - } - ], data={}) - """.trimIndent() - - assertEquals(expectedMessage, msg, "Unexpected error message for invalid arguments") + val textContent = callResult.content.firstOrNull { it is TextContent } as? TextContent + assertNotNull(textContent, "Error content should be present in the result") + + val errorText = textContent.text ?: "" + assertTrue( + errorText.contains("Invalid arguments") && errorText.contains("greet"), + "Error message should indicate invalid arguments for tool greet", + ) } }