fix!(server): #575 Rethrow CancellationException in StdioServerTransport, fix ReadBuffer by kpavlov · Pull Request #571 · modelcontextprotocol/kotlin-sdk

kpavlov · 2026-03-02T08:08:25Z

Rethrow CancellationException in StdioServerTransport, fix ReadBuffer

Changes

StdioServerTransport
- Replace generic Throwable catches with specific CancellationException and Exception for clarity and correctness. breaking change Throwable is not handled any more!
- Refactor launching jobs to separate methods
- Introduce READ_BUFFER_SIZE constant to replace inline buffer size literals.
- Add suppress annotations for clearer intent and constructor documentation for better usability.
- Extend StdioServerTransportTest
ReadBuffer

Previously, readMessage() returned null after consuming an unparseable line even when more complete lines existed in the buffer. This caused valid messages following a bad line in the same chunk to be silently dropped until the next chunk arrived (or forever, in tests).

Fix the loop so null means only "no complete line available", not "encountered a parse failure". Blank/whitespace-only lines are now silently skipped via isBlank() rather than forwarded to the deserializer and logged as errors.

Refactor the method into three focused helpers:
- readMessage() — outer loop over lines
- readLine() — consume the next newline-delimited line from the buffer
- tryRecover() — attempt deserialization from the first '{' onward
Add utility method runIntegrationTest for integration testing (runBlocking + withTimeout). Use it in StdioServerTransportTest
Store processing Job reference and calcelAndJoin() it on close. Partially addressing StdioServerTransport: The coroutine scope that's used in this transport is completely detached from the coroutine tree #574

Motivation and Context

See #575, #564, #242

How Has This Been Tested?

Unit test added

Breaking Changes

Semantic change: Throwable and CancellationException are not handled any more!

Types of changes

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

I have read the MCP Documentation
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

devcrocod

lgtm

I’ve left a few comments, but they’re minor and mostly related to code style

codecov-commenter · 2026-03-02T16:14:07Z

Codecov Report

❌ Patch coverage is 93.22034% with 4 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...protocol/kotlin/sdk/server/StdioServerTransport.kt	87.09%	1 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

kpavlov · 2026-03-02T16:14:13Z

Thank you, @devcrocod, for your comments! I tried to address most of them, please have a look

kpavlov · 2026-03-02T22:10:12Z

-
            readChannel.close()
+
+            processingJob?.cancelAndJoin()


e5l

please check the comments before merging

e5l · 2026-03-03T05:37:33Z

+            @Suppress("TooGenericExceptionCaught")
+            val message = try {
+                deserializeMessage(line)
+            } catch (e: Exception) {


Suggested change

} catch (e: Exception) {

} catch (e: Throwable) {

e5l · 2026-03-03T05:38:37Z

-            } catch (e: Throwable) {
+            } catch (e: CancellationException) {
+                throw e
+            } catch (e: Exception) {


Suggested change

} catch (e: Exception) {

} catch (e: Throwable) {

e5l · 2026-03-03T05:39:42Z

            val message = try {
                readBuffer.readMessage()
-            } catch (e: Throwable) {
+            } catch (e: Exception) {


Could you tell me why we want to skip logging Error? https://docs.oracle.com/javase/8/docs/api/java/lang/Error.html

Things like OutOfMemoryError are not actionable and usually not handled by the application and propagated.

they still needs to be logged by the configured logger

Logging is never guaranteed in the situation when JVM is crashing.
E.g., an OutOfMemoryError usually means the JVM has insufficient memory to allocate additional objects. Application loggers often allocate new objects (e.g., strings, log event objects, buffers) or interact with subsystems (e.g., disk I/O, network I/O). These operations may fail or behave unpredictably under memory pressure.

Logging to System.err might make sense, but is still questionable.

…ServerTransport - Replace generic `Throwable` catches with specific `CancellationException` and `Exception` for clarity and correctness. **breaking change** `Throwable` is not handled any more! - Refactor launching jobs to separate methods - Introduce `READ_BUFFER_SIZE` constant to replace inline buffer size literals. - Add suppress annotations for clearer intent and constructor documentation for better usability. - Extend StdioServerTransportTest

Previously, readMessage() returned null after consuming an unparseable line even when more complete lines existed in the buffer. This caused valid messages following a bad line in the same chunk to be silently dropped until the next chunk arrived (or forever, in tests). Fix the loop so null means only "no complete line available", not "encountered a parse failure". Blank/whitespace-only lines are now silently skipped via isBlank() rather than forwarded to the deserializer and logged as errors. Refactor the method into three focused helpers: - readMessage() — outer loop over lines - readLine() — consume the next newline-delimited line from the buffer - tryRecover() — attempt deserialization from the first '{' onward

- Add test verifying server continues processing messages despite handler exceptions. - Introduce `ParameterizedTest` for various exception types in message handler. - Ensure `onError` is not invoked for `CancellationException`.

…dioServerTransport tests - Introduced `runIntegrationTest` utility for consistent test execution with timeout configuration. - Adjusted test utils module in Gradle build to include new helper. - Simplified exception handling and concurrency in StdioServerTransportTest.

…verTransport - Refactored job management to include explicit tracking for the processing job. Enhanced resource cleanup logic. - Updated constructor parameters KDoc to source/sink to match actual types. Renamed internal variable `outputWriter` to `outputSink` to reflect updated abstraction. - Added helper method `logJobCompletion` for consistent logging of job lifecycle events.

…t and tests Addressed PR comments: - Updated exception handling in `StdioServerTransport` to catch `Throwable` instead of `Exception`. - Adjusted test methods and helper functions to handle `Throwable`.

kpavlov · 2026-03-03T11:34:03Z

@e5l, I’ve reverted to using Throwables. Could you take a look?

kpavlov changed the title ~~fix!(server): Rethrow CancellationException, add buffer size constant in StdioServerTransport~~ fix!(server): #564 Rethrow CancellationException, add buffer size constant in StdioServerTransport Mar 2, 2026

kpavlov added the bugfix label Mar 2, 2026

kpavlov changed the title ~~fix!(server): #564 Rethrow CancellationException, add buffer size constant in StdioServerTransport~~ fix!(server): #575 Rethrow CancellationException, add buffer size constant in StdioServerTransport Mar 2, 2026

kpavlov force-pushed the kpavlov/564-stdio-server-ce branch from 7c2669c to 91b2d9a Compare March 2, 2026 09:33

kpavlov changed the title ~~fix!(server): #575 Rethrow CancellationException, add buffer size constant in StdioServerTransport~~ fix!(server): #575 Rethrow CancellationException in StdioServerTransport, fix ReadBuffer Mar 2, 2026

kpavlov force-pushed the kpavlov/564-stdio-server-ce branch 2 times, most recently from 18efbcd to 9e2cf25 Compare March 2, 2026 10:02

kpavlov marked this pull request as ready for review March 2, 2026 10:02

kpavlov requested review from devcrocod and e5l March 2, 2026 10:02

kpavlov mentioned this pull request Mar 2, 2026

Cancellation exceptions cause an error to be logged #242

Closed

kpavlov requested a review from Amaneusz March 2, 2026 11:37

devcrocod previously approved these changes Mar 2, 2026

View reviewed changes

kpavlov dismissed devcrocod’s stale review via b16cbab March 2, 2026 15:21

kpavlov force-pushed the kpavlov/564-stdio-server-ce branch 2 times, most recently from b16cbab to aef2c76 Compare March 2, 2026 16:01

kpavlov requested a review from devcrocod March 2, 2026 16:11

kpavlov marked this pull request as draft March 2, 2026 20:16

kpavlov force-pushed the kpavlov/564-stdio-server-ce branch 3 times, most recently from 1d05801 to 61f8392 Compare March 2, 2026 21:41

kpavlov commented Mar 2, 2026

View reviewed changes

kpavlov marked this pull request as ready for review March 2, 2026 22:10

e5l previously approved these changes Mar 3, 2026

View reviewed changes

kpavlov added 4 commits March 3, 2026 09:11

Address PR comments

6318623

test: verify exception handling in StdioServerTransport

2bfefa4

- Add test verifying server continues processing messages despite handler exceptions. - Introduce `ParameterizedTest` for various exception types in message handler. - Ensure `onError` is not invoked for `CancellationException`.

kpavlov added 2 commits March 3, 2026 09:11

kpavlov force-pushed the kpavlov/564-stdio-server-ce branch from 61f8392 to 9d47a71 Compare March 3, 2026 07:11

refactor: replace Exception with Throwable in StdioServerTranspor…

9c0ad9f

…t and tests Addressed PR comments: - Updated exception handling in `StdioServerTransport` to catch `Throwable` instead of `Exception`. - Adjusted test methods and helper functions to handle `Throwable`.

kpavlov dismissed e5l’s stale review via 9c0ad9f March 3, 2026 11:06

e5l approved these changes Mar 3, 2026

View reviewed changes

kpavlov merged commit a388180 into main Mar 3, 2026
13 checks passed

kpavlov deleted the kpavlov/564-stdio-server-ce branch March 3, 2026 14:10

kpavlov mentioned this pull request Mar 3, 2026

StdioServerTransport: Cancellation exceptions are being caught and not rethrown #575

Closed

Uh oh!

Conversation

kpavlov commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rethrow CancellationException in StdioServerTransport, fix ReadBuffer

Changes

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Checklist

Additional context

Uh oh!

devcrocod left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kpavlov commented Mar 2, 2026

Uh oh!

kpavlov Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

e5l left a comment

Choose a reason for hiding this comment

Uh oh!

e5l Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

e5l Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

e5l Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

kpavlov Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

e5l Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

kpavlov Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

kpavlov commented Mar 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kpavlov commented Mar 2, 2026 •

edited

Loading

codecov-commenter commented Mar 2, 2026 •

edited

Loading