Better exception handling in StdioClientTransport (#444)
# Better exception handling in StdioClientTransport Refactor `McpException` for improved handling with convenience constructors and enhanced exception wrapping logic in `StdioClientTransport`. Update tests to use `kotest` matchers and add JUnit parameterized tests for exception handling. ## Motivation and Context #442 follow up ## How Has This Been Tested? <!-- Have you tested this in a real application? Which scenarios were tested? --> ## Breaking Changes <!-- Will users need to update their code or configurations? --> ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [ ] My code follows the repository's style guidelines - [ ] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions -->
This commit is contained in:
parent
bb8de718d9
commit
6ec6d3c52f
2 changed files with 121 additions and 15 deletions
|
|
@ -15,6 +15,7 @@ import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCMessage
|
|||
import io.modelcontextprotocol.kotlin.sdk.types.McpException
|
||||
import io.modelcontextprotocol.kotlin.sdk.types.RPCError.ErrorCode.CONNECTION_CLOSED
|
||||
import io.modelcontextprotocol.kotlin.sdk.types.RPCError.ErrorCode.INTERNAL_ERROR
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.CoroutineName
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
|
|
@ -71,12 +72,14 @@ import kotlin.jvm.JvmOverloads
|
|||
* @param input The input stream where messages are received.
|
||||
* @param output The output stream where messages are sent.
|
||||
* @param error Optional error stream for stderr monitoring.
|
||||
* @param sendChannel Channel for outbound messages. Default: buffered channel (capacity 64).
|
||||
* @param sendChannel Channel for outbound messages. Default: buffered channel
|
||||
* (<a jref="https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.channels/-channel/-factory/-b-u-f-f-e-r-e-d.html">implementation-default capacity</a>).
|
||||
* @param classifyStderr Callback to classify stderr lines. Return [StderrSeverity.FATAL] to fail transport,
|
||||
* or [StderrSeverity.WARNING] / [StderrSeverity.INFO] / [StderrSeverity.DEBUG]
|
||||
* to log, or [StderrSeverity.IGNORE] to discard.
|
||||
* Default value: [StderrSeverity.DEBUG].
|
||||
* @see <a href="https://modelcontextprotocol.io/specification/2025-11-25/basic/transports#stdio">MCP Specification</a>
|
||||
* @see <a href="https://modelcontextprotocol.io/specification/2025-06-18/basic/transports#stdio">MCP Specification</a>
|
||||
* @see [Channel.BUFFERED]
|
||||
*/
|
||||
@OptIn(ExperimentalAtomicApi::class)
|
||||
public class StdioClientTransport @JvmOverloads public constructor(
|
||||
|
|
@ -232,15 +235,25 @@ public class StdioClientTransport @JvmOverloads public constructor(
|
|||
@Suppress("TooGenericExceptionCaught", "SwallowedException")
|
||||
try {
|
||||
sendChannel.send(message)
|
||||
} catch (e: CancellationException) {
|
||||
throw e // MUST rethrow immediately - don't log, don't wrap
|
||||
} catch (e: ClosedSendChannelException) {
|
||||
logger.debug(e) { "Cannot send message: transport is closed" }
|
||||
throw McpException(CONNECTION_CLOSED, "Transport is closed")
|
||||
throw McpException(
|
||||
code = CONNECTION_CLOSED,
|
||||
message = "Transport is closed",
|
||||
cause = e,
|
||||
)
|
||||
} catch (e: McpException) {
|
||||
logger.debug(e) { "Error while sending message: ${e.message}" }
|
||||
throw e
|
||||
} catch (e: Exception) {
|
||||
} catch (e: Throwable) {
|
||||
logger.error(e) { "Error while sending message: ${e.message}" }
|
||||
throw McpException(INTERNAL_ERROR, "Error while sending message: ${e.message}")
|
||||
throw McpException(
|
||||
code = INTERNAL_ERROR,
|
||||
message = "Error while sending message: ${e.message}",
|
||||
cause = e,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,12 +1,28 @@
|
|||
package io.modelcontextprotocol.kotlin.sdk.client.stdio
|
||||
|
||||
import io.kotest.assertions.throwables.shouldThrow
|
||||
import io.kotest.matchers.booleans.shouldBeFalse
|
||||
import io.kotest.matchers.shouldBe
|
||||
import io.kotest.matchers.types.shouldBeInstanceOf
|
||||
import io.kotest.matchers.types.shouldBeSameInstanceAs
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.mockk
|
||||
import io.modelcontextprotocol.kotlin.sdk.client.StdioClientTransport
|
||||
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCMessage
|
||||
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCRequest
|
||||
import io.modelcontextprotocol.kotlin.sdk.types.McpException
|
||||
import io.modelcontextprotocol.kotlin.sdk.types.RPCError.ErrorCode
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.channels.Channel
|
||||
import kotlinx.coroutines.channels.ClosedSendChannelException
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlinx.io.Buffer
|
||||
import kotlinx.io.writeString
|
||||
import org.junit.jupiter.params.ParameterizedTest
|
||||
import org.junit.jupiter.params.provider.Arguments
|
||||
import org.junit.jupiter.params.provider.MethodSource
|
||||
import java.util.stream.Stream
|
||||
import kotlin.concurrent.atomics.AtomicBoolean
|
||||
import kotlin.concurrent.atomics.ExperimentalAtomicApi
|
||||
import kotlin.test.Test
|
||||
|
|
@ -95,4 +111,66 @@ class StdioClientTransportErrorHandlingTest {
|
|||
// Empty input should close cleanly without error
|
||||
errorCalled.shouldBeFalse()
|
||||
}
|
||||
|
||||
companion object {
|
||||
@JvmStatic
|
||||
fun exceptions(): Stream<Arguments> = Stream.of(
|
||||
Arguments.of(
|
||||
CancellationException(),
|
||||
false, // should not wrap, propagate
|
||||
null,
|
||||
),
|
||||
Arguments.of(
|
||||
McpException(-1, "dummy"),
|
||||
false, // should not wrap, propagate
|
||||
null,
|
||||
),
|
||||
Arguments.of(
|
||||
ClosedSendChannelException("dummy"),
|
||||
true, // should wrap in McpException
|
||||
ErrorCode.CONNECTION_CLOSED,
|
||||
),
|
||||
Arguments.of(
|
||||
Exception(),
|
||||
true,
|
||||
ErrorCode.INTERNAL_ERROR,
|
||||
),
|
||||
Arguments.of(
|
||||
OutOfMemoryError(),
|
||||
true,
|
||||
ErrorCode.INTERNAL_ERROR,
|
||||
),
|
||||
|
||||
)
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@MethodSource("exceptions")
|
||||
fun `Send should handle exceptions`(throwable: Throwable, shouldWrap: Boolean, expectedCode: Int?) = runTest {
|
||||
val sendChannel: Channel<JSONRPCMessage> = mockk(relaxed = true)
|
||||
|
||||
transport = StdioClientTransport(
|
||||
input = Buffer(),
|
||||
output = Buffer(),
|
||||
sendChannel = sendChannel,
|
||||
)
|
||||
|
||||
coEvery { sendChannel.send(any()) } throws throwable
|
||||
|
||||
transport.start()
|
||||
|
||||
// Cancel the coroutine while it's suspended in send()
|
||||
val exception = shouldThrow<Throwable> {
|
||||
transport.send(JSONRPCRequest(id = "test-1", method = "test/method"))
|
||||
}
|
||||
|
||||
if (shouldWrap) {
|
||||
exception.shouldBeInstanceOf<McpException> {
|
||||
it.cause shouldBeSameInstanceAs throwable
|
||||
it.code shouldBe expectedCode
|
||||
}
|
||||
} else {
|
||||
exception shouldBeSameInstanceAs throwable
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue