#107 Monitor STDERR and refactor StdioClientTransport to use flows (#442)

This commit is contained in:
Konstantin Pavlov 2025-12-04 09:18:32 +02:00 • committed by GitHub
commit 191b76863d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 660 additions and 87 deletions

View file

@ -5,10 +5,12 @@ import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCMessage
import io.modelcontextprotocol.kotlin.sdk.types.PingRequest
import io.modelcontextprotocol.kotlin.sdk.types.toJSON
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.delay
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue
import kotlin.test.fail
import kotlin.time.Duration.Companion.seconds
abstract class BaseTransportTest {
@ -21,6 +23,8 @@ abstract class BaseTransportTest {
transport.onClose { didClose = true }
transport.start()
delay(1.seconds)
assertFalse(didClose, "Transport should not be closed immediately after start")
transport.close()

View file

@ -1,13 +1,68 @@
package io.modelcontextprotocol.kotlin.sdk.client
import io.modelcontextprotocol.kotlin.sdk.shared.BaseTransportTest
import io.modelcontextprotocol.kotlin.sdk.types.Implementation
import io.modelcontextprotocol.kotlin.sdk.types.McpException
import kotlinx.coroutines.delay
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
import kotlinx.io.asSink
import kotlinx.io.asSource
import kotlinx.io.buffered
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.concurrent.atomics.AtomicBoolean
import kotlin.concurrent.atomics.ExperimentalAtomicApi
import kotlin.test.assertFalse
import kotlin.test.assertTrue
import kotlin.test.fail
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds
@Timeout(30, unit = TimeUnit.SECONDS)
class StdioClientTransportTest : BaseTransportTest() {
@Test
fun `handle stdio error`(): Unit = runBlocking {
val processBuilder = if (System.getProperty("os.name").lowercase().contains("win")) {
ProcessBuilder("cmd", "/c", "pause 1 && echo simulated error 1>&2 && exit 1")
} else {
ProcessBuilder("sh", "-c", "sleep 1 && echo 'simulated error' >&2 && exit 1")
}
val process = processBuilder.start()
val stdin = process.inputStream.asSource().buffered()
val stdout = process.outputStream.asSink().buffered()
val stderr = process.errorStream.asSource().buffered()
val transport = StdioClientTransport(
input = stdin,
output = stdout,
error = stderr,
) {
println("💥Ah-oh!, error: \"$it\"")
StdioClientTransport.StderrSeverity.FATAL
}
val client = Client(
clientInfo = Implementation(
name = "test-client",
version = "1.0",
),
)
// The error in stderr should cause connecting to fail
assertThrows<McpException> {
client.connect(transport)
}
process.destroyForcibly()
}
@OptIn(ExperimentalAtomicApi::class)
@Test
fun `should start then close cleanly`() = runTest {
// Run process "/usr/bin/tee"
@ -16,15 +71,33 @@ class StdioClientTransportTest : BaseTransportTest() {
val input = process.inputStream.asSource().buffered()
val output = process.outputStream.asSink().buffered()
val error = process.errorStream.asSource().buffered()
val transport = StdioClientTransport(
input = input,
output = output,
error = error,
)
testTransportOpenClose(transport)
transport.onError { error ->
fail("Unexpected error: $error")
}
process.destroy()
val didClose = AtomicBoolean(false)
transport.onClose { didClose.store(true) }
transport.start()
delay(1.seconds)
assertFalse(didClose.load(), "Transport should not be closed immediately after start")
// Destroy process BEFORE close() to unblock stdin reader
process.destroyForcibly()
delay(100.milliseconds) // Give time for EOF to propagate
transport.close()
assertTrue(didClose.load(), "Transport should be closed after close() call")
}
@Test
@ -43,7 +116,7 @@ class StdioClientTransportTest : BaseTransportTest() {
testTransportRead(transport)
process.waitFor()
process.destroy()
process.destroyForcibly()
}
@Test
@ -63,6 +136,6 @@ class StdioClientTransportTest : BaseTransportTest() {
testTransportRead(transport)
process.waitFor()
process.destroy()
process.destroyForcibly()
}
}

View file

@ -89,7 +89,7 @@ class KotlinClientTsServerEdgeCasesTestStdio : TsTestBase() {
}
@Test
@Timeout(60, unit = TimeUnit.SECONDS)
@Timeout(30, unit = TimeUnit.SECONDS)
fun testConcurrentRequestsOverStdio(): Unit = runBlocking(Dispatchers.IO) {
withClientStdio { client: Client, _ ->
val concurrentCount = 5
@ -102,8 +102,7 @@ class KotlinClientTsServerEdgeCasesTestStdio : TsTestBase() {
val result = client.callTool("greet", arguments)
assertNotNull(result, "Tool call result should not be null for client $i")
val callResult = result
val textContent = callResult.content.firstOrNull { it is TextContent } as? TextContent
val textContent = result.content.firstOrNull { it is TextContent } as? TextContent
assertNotNull(textContent, "Text content should be present for client $i")
textContent.text