Skip to content

Fix: Return CallToolResult with isError for tool errors to conform with MCP spec #354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2025
+95 −77
Merged

Fix: Return CallToolResult with isError for tool errors to conform with MCP spec #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {}

Expand Down Expand Up @@ -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")),
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent error message format. The log message uses 'Tool not found: ${request.name}' (line 524), but the returned content uses 'Tool ${request.name} not found'. These should match for consistency. Consider changing to 'Tool not found: ${request.name}' to align with the logging and other error messages in the codebase (e.g., 'Prompt not found: ${request.name}' on line 556).

Suggested change
content = listOf(TextContent(text = "Tool ${request.name} not found")),
content = listOf(TextContent(text = "Tool not found: ${request.name}")),

Copilot uses AI. Check for mistakes.
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 {
Expand Down
39 changes: 23 additions & 16 deletions ...tlin/io/modelcontextprotocol/kotlin/sdk/integration/kotlin/AbstractToolIntegrationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -527,16 +526,18 @@ abstract class AbstractToolIntegrationTest : KotlinTestBase() {
"message" to "My exception message",
)

val exception = assertThrows<IllegalStateException> {
runBlocking {
client.callTool(errorToolName, exceptionArgs)
}
}
val exceptionResult = client.callTool(errorToolName, exceptionArgs) as CallToolResultBase
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing runBlocking wrapper for this coroutine call. Unlike line 774 where runBlocking is used to call client.callTool, this call is not wrapped in runBlocking. This inconsistency could lead to issues if the surrounding context changes. Add runBlocking wrapper for consistency with the pattern used elsewhere in the file.

Copilot uses AI. Check for mistakes.

val msg = exception.message ?: ""
val expectedMessage = "JSONRPCError(code=InternalError, message=My exception message, data={})"
assertTrue(exceptionResult.isError ?: false, "isError should be true for exception")
Copy link
Contributor

@kpavlov kpavlov Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertTrue(exceptionResult.isError ?: false, "isError should be true for exception")
assertTrue(exceptionResult.isError == true, "isError should be true for exception")

Elvis operator looks crypric in asserts.


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",
)
Comment on lines +533 to +540
Copy link
Contributor

@kpavlov kpavlov Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it would be more readable with kotest assertions

}

@Test
Expand Down Expand Up @@ -770,15 +771,21 @@ abstract class AbstractToolIntegrationTest : KotlinTestBase() {
val nonExistentToolName = "non-existent-tool"
val arguments = mapOf("text" to "Test")

val exception = assertThrows<IllegalStateException> {
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")
Copy link
Contributor

@kpavlov kpavlov Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above


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",
)
}
}
54 changes: 25 additions & 29 deletions ...extprotocol/kotlin/sdk/integration/typescript/sse/KotlinClientTsServerEdgeCasesTestSse.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -74,16 +73,19 @@ class KotlinClientTsServerEdgeCasesTestSse : TsTestBase() {
val nonExistentToolName = "non-existent-tool"
val arguments = mapOf("name" to "TestUser")

val exception = assertThrows<IllegalStateException> {
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",
)
}
}
Expand Down Expand Up @@ -176,26 +178,20 @@ class KotlinClientTsServerEdgeCasesTestSse : TsTestBase() {
"name" to JsonObject(mapOf("nested" to JsonPrimitive("value"))),
)

val exception = assertThrows<IllegalStateException> {
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",
)
}
}

Expand Down
54 changes: 25 additions & 29 deletions ...rotocol/kotlin/sdk/integration/typescript/stdio/KotlinClientTsServerEdgeCasesTestStdio.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -31,16 +30,19 @@ class KotlinClientTsServerEdgeCasesTestStdio : TsTestBase() {
val nonExistentToolName = "non-existent-tool"
val arguments = mapOf("name" to "TestUser")

val exception = assertThrows<IllegalStateException> {
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",
)
}
}
Expand Down Expand Up @@ -133,26 +135,20 @@ class KotlinClientTsServerEdgeCasesTestStdio : TsTestBase() {
"name" to JsonObject(mapOf("nested" to JsonPrimitive("value"))),
)

val exception = assertThrows<IllegalStateException> {
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",
)
}
}

Expand Down
Loading