Refactor errors, add tests, improve code style #370
+331
−158
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,7 +46,7 @@ kover { | |
| } | ||
| verify { | ||
| rule { | ||
| minBound(75) | ||
| } | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| @file:Suppress("unused", "EnumEntryName") | ||
|
|
||
| package io.modelcontextprotocol.kotlin.sdk | ||
|
|
||
| import kotlinx.serialization.json.JsonObject | ||
|
|
||
| @Deprecated("Use McpException instead", ReplaceWith("McpException")) | ||
| public typealias McpError = McpException | ||
|
|
||
| /** | ||
| * Represents an error specific to the MCP protocol. | ||
| * | ||
| * @property code The error code. | ||
| * @property message The error message. | ||
| * @property data Additional error data as a JSON object. | ||
| */ | ||
| public class McpException(public val code: Int, message: String, public val data: JsonObject = EmptyJsonObject) : | ||
| Exception("MCP error $code: \"$message\"") | ||
|
|
||
| /** | ||
| * Converts a `JSONRPCError` instance to an [McpException] instance. | ||
| * | ||
| * @return An [McpException] containing the code, message, and data from the `JSONRPCError`. | ||
| */ | ||
| internal fun JSONRPCError.toMcpException(): McpException = McpException( | ||
| code = this.code.code, | ||
| message = this.message, | ||
| data = this.data, | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package io.modelcontextprotocol.kotlin.sdk.shared | ||
|
|
||
| import io.modelcontextprotocol.kotlin.sdk.JSONRPCMessage | ||
| import kotlinx.coroutines.CompletableDeferred | ||
|
|
||
| /** | ||
| * Implements [onClose], [onError] and [onMessage] functions of [Transport] providing | ||
| * corresponding [_onClose], [_onError] and [_onMessage] properties to use for an implementation. | ||
| */ | ||
| @Suppress("PropertyName") | ||
| public abstract class AbstractTransport : Transport { | ||
| protected var _onClose: (() -> Unit) = {} | ||
| private set | ||
| protected var _onError: ((Throwable) -> Unit) = {} | ||
| private set | ||
|
|
||
| // to not skip messages | ||
| private val _onMessageInitialized = CompletableDeferred<Unit>() | ||
| protected var _onMessage: (suspend ((JSONRPCMessage) -> Unit)) = { | ||
| _onMessageInitialized.await() | ||
| _onMessage.invoke(it) | ||
| } | ||
| private set | ||
|
|
||
| override fun onClose(block: () -> Unit) { | ||
| val old = _onClose | ||
| _onClose = { | ||
| old() | ||
| block() | ||
| } | ||
| } | ||
|
|
||
| override fun onError(block: (Throwable) -> Unit) { | ||
| val old = _onError | ||
| _onError = { e -> | ||
| old(e) | ||
| block(e) | ||
| } | ||
| } | ||
|
|
||
| override fun onMessage(block: suspend (JSONRPCMessage) -> Unit) { | ||
| val old: suspend (JSONRPCMessage) -> Unit = when (_onMessageInitialized.isCompleted) { | ||
| true -> _onMessage | ||
| false -> { _ -> } | ||
| } | ||
|
|
||
| _onMessage = { message -> | ||
| old(message) | ||
| block(message) | ||
| } | ||
|
|
||
| _onMessageInitialized.complete(Unit) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ import io.modelcontextprotocol.kotlin.sdk.JSONRPCError | |
| import io.modelcontextprotocol.kotlin.sdk.JSONRPCNotification | ||
| import io.modelcontextprotocol.kotlin.sdk.JSONRPCRequest | ||
| import io.modelcontextprotocol.kotlin.sdk.JSONRPCResponse | ||
| import io.modelcontextprotocol.kotlin.sdk.McpException | ||
| import io.modelcontextprotocol.kotlin.sdk.Method | ||
| import io.modelcontextprotocol.kotlin.sdk.Notification | ||
| import io.modelcontextprotocol.kotlin.sdk.PingRequest | ||
|
|
@@ -19,6 +19,7 @@ import io.modelcontextprotocol.kotlin.sdk.RequestId | |
| import io.modelcontextprotocol.kotlin.sdk.RequestResult | ||
| import io.modelcontextprotocol.kotlin.sdk.fromJSON | ||
| import io.modelcontextprotocol.kotlin.sdk.toJSON | ||
| import io.modelcontextprotocol.kotlin.sdk.toMcpException | ||
| import kotlinx.atomicfu.AtomicRef | ||
| import kotlinx.atomicfu.atomic | ||
| import kotlinx.atomicfu.getAndUpdate | ||
|
|
@@ -97,7 +98,7 @@ public data class RequestOptions( | |
| val onProgress: ProgressCallback? = null, | ||
|
|
||
| /** | ||
| * A timeout for this request. If exceeded, an [McpException] with code `RequestTimeout` | ||
| * will be raised from request(). | ||
| * | ||
| * If not specified, `DEFAULT_REQUEST_TIMEOUT` will be used as the timeout. | ||
|
|
@@ -116,6 +117,7 @@ internal val COMPLETED = CompletableDeferred(Unit).also { it.complete(Unit) } | |
| * Implements MCP protocol framing on top of a pluggable transport, including | ||
| * features like request/response linking, notifications, and progress. | ||
| */ | ||
| @Suppress("TooManyFunctions") | ||
| public abstract class Protocol(@PublishedApi internal val options: ProtocolOptions?) { | ||
| public var transport: Transport? = null | ||
| private set | ||
|
|
@@ -190,7 +192,9 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
| /** | ||
| * Attaches to the given transport, starts it, and starts listening for messages. | ||
| * | ||
| * The Protocol object assumes ownership of the Transport, | ||
| * replacing any callbacks that have already been set, | ||
| * and expects that it is the only user of the Transport instance going forward. | ||
| */ | ||
| public open suspend fun connect(transport: Transport) { | ||
| this.transport = transport | ||
|
|
@@ -222,7 +226,7 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
| transport = null | ||
| onClose() | ||
|
|
||
| val error = McpException(ErrorCode.Defined.ConnectionClosed.code, "Connection closed") | ||
| for (handler in handlersToNotify) { | ||
| handler(null, error) | ||
| } | ||
|
|
@@ -237,6 +241,7 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
| logger.trace { "No handler found for notification: ${notification.method}" } | ||
| return | ||
| } | ||
| @Suppress("TooGenericExceptionCaught") | ||
| try { | ||
| handler(notification) | ||
| } catch (cause: Throwable) { | ||
|
|
@@ -252,6 +257,7 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
|
|
||
| if (handler === null) { | ||
| logger.trace { "No handler found for request: ${request.method}" } | ||
| @Suppress("TooGenericExceptionCaught") | ||
| try { | ||
| transport?.send( | ||
| JSONRPCResponse( | ||
|
|
@@ -268,7 +274,7 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
| } | ||
| return | ||
| } | ||
| @Suppress("TooGenericExceptionCaught") | ||
| try { | ||
| val result = handler(request, RequestHandlerExtra()) | ||
| logger.trace { "Request handled successfully: ${request.method} (id: ${request.id})" } | ||
|
|
@@ -303,7 +309,8 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
|
|
||
| private fun onProgress(notification: ProgressNotification) { | ||
| logger.trace { | ||
| "Received progress notification: token=${notification.params.progressToken}, " + | ||
| "progress=${notification.params.progress}/${notification.params.total}" | ||
| } | ||
| val progress = notification.params.progress | ||
| val total = notification.params.total | ||
|
|
@@ -347,7 +354,7 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
| handler(response, null) | ||
| } else { | ||
| check(error != null) | ||
| val error = McpException( | ||
| error.code.code, | ||
| error.message, | ||
| error.data, | ||
|
|
@@ -392,7 +399,7 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
| public suspend fun <T : RequestResult> request(request: Request, options: RequestOptions? = null): T { | ||
| logger.trace { "Sending request: ${request.method}" } | ||
| val result = CompletableDeferred<T>() | ||
| val transport = transport ?: throw McpException(ErrorCode.Defined.ConnectionClosed.code, "Not connected") | ||
|
|
||
| if ([email protected]?.enforceStrictCapabilities == true) { | ||
| assertCapabilityForMethod(request.method) | ||
|
|
@@ -415,11 +422,12 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
| return@put | ||
| } | ||
|
|
||
| response?.error?.let { | ||
| result.completeExceptionally(it.toMcpException()) | ||
| return@put | ||
| } | ||
|
|
||
| @Suppress("TooGenericExceptionCaught") | ||
| try { | ||
| @Suppress("UNCHECKED_CAST") | ||
| result.complete(response!!.result as T) | ||
|
|
@@ -459,7 +467,7 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio | |
| } catch (cause: TimeoutCancellationException) { | ||
| logger.error { "Request timed out after ${timeout.inWholeMilliseconds}ms: ${request.method}" } | ||
| cancel( | ||
| McpException( | ||
| ErrorCode.Defined.RequestTimeout.code, | ||
| "Request timed out", | ||
| JsonObject(mutableMapOf("timeout" to JsonPrimitive(timeout.inWholeMilliseconds))), | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rise the bar