Skip to content

Commit 4ca7e6b

Browse files
committed
fix: keep stdio client reading after a malformed response
The stdio client's inbound loop stopped at the first message it could not deserialize. It then marked the transport as closing and completed the inbound stream, so the request answered by that message timed out and so did every later request on the same client, including ping. A message that fails to deserialize no longer ends the loop. If it is a response with a usable id, the transport emits an error response for that id, so the matching request fails right away with an internal error. A line that is not JSON at all is logged and skipped. The new test runs a server process that sends a response without jsonrpc, a response with both result and error, a line that is not JSON, and then a valid response, and checks that the two malformed responses fail their requests and the valid one still arrives. Fixes #1157
1 parent 1cf7903 commit 4ca7e6b

2 files changed

Lines changed: 92 additions & 1 deletion

File tree

‎mcp-core/src/main/java/io/modelcontextprotocol/client/transport/StdioClientTransport.java‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
import java.time.Duration;
1212
import java.util.ArrayList;
1313
import java.util.EnumSet;
14+
import java.util.HashMap;
1415
import java.util.List;
16+
import java.util.Map;
1517
import java.util.Set;
1618
import java.util.concurrent.Executors;
1719
import java.util.function.Consumer;
@@ -23,6 +25,7 @@
2325
import io.modelcontextprotocol.spec.McpClientTransport;
2426
import io.modelcontextprotocol.spec.McpSchema;
2527
import io.modelcontextprotocol.spec.McpSchema.JSONRPCMessage;
28+
import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse;
2629
import io.modelcontextprotocol.util.Assert;
2730
import org.slf4j.Logger;
2831
import org.slf4j.LoggerFactory;
@@ -290,7 +293,15 @@ private void startInboundProcessing() {
290293
if (!isClosing) {
291294
logger.error("Error processing inbound message for line: {}", line, e);
292295
}
293-
break;
296+
// One malformed message must not end the session. Fail the
297+
// request it answers, if any, and keep reading.
298+
JSONRPCResponse failure = failureForMalformedResponse(line, e);
299+
if (failure != null && !this.inboundSink.tryEmitNext(failure).isSuccess()) {
300+
if (!isClosing) {
301+
logger.error("Failed to enqueue inbound message: {}", failure);
302+
}
303+
break;
304+
}
294305
}
295306
}
296307
}
@@ -311,6 +322,32 @@ private void startInboundProcessing() {
311322
});
312323
}
313324

325+
/**
326+
* Builds an error response for a response that could not be deserialized, so that the
327+
* request it answers fails right away instead of waiting for its timeout.
328+
* @param line the raw message
329+
* @param cause the deserialization failure
330+
* @return the error response, or {@code null} if the line is not a response with a
331+
* usable id
332+
*/
333+
private JSONRPCResponse failureForMalformedResponse(String line, Exception cause) {
334+
try {
335+
Map<String, Object> message = this.jsonMapper.readValue(line, new TypeRef<HashMap<String, Object>>() {
336+
});
337+
Object id = message.get("id");
338+
boolean isResponse = !message.containsKey("method")
339+
&& (message.containsKey("result") || message.containsKey("error"));
340+
if (isResponse && (id instanceof String || id instanceof Integer || id instanceof Long)) {
341+
return JSONRPCResponse.error(id, new JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR,
342+
"Received a malformed JSON-RPC response", cause.getMessage()));
343+
}
344+
}
345+
catch (Exception ignored) {
346+
// not JSON, so there is no request to fail
347+
}
348+
return null;
349+
}
350+
314351
/**
315352
* Reads a single line, mirroring {@link BufferedReader#readLine()}, but aborting once
316353
* more than {@code maxSize} characters have been read without encountering a line

‎mcp-test/src/test/java/io/modelcontextprotocol/client/transport/StdioClientTransportTests.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,20 @@
66

77
import java.io.ByteArrayOutputStream;
88
import java.io.PrintStream;
9+
import java.nio.file.Files;
10+
import java.nio.file.Path;
911
import java.time.Duration;
12+
import java.util.List;
13+
import java.util.concurrent.CopyOnWriteArrayList;
1014

15+
import io.modelcontextprotocol.spec.McpSchema;
16+
import io.modelcontextprotocol.spec.McpSchema.JSONRPCMessage;
17+
import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse;
1118
import org.awaitility.Awaitility;
1219
import org.junit.jupiter.api.AfterEach;
1320
import org.junit.jupiter.api.BeforeEach;
1421
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.api.io.TempDir;
1523
import reactor.test.StepVerifier;
1624

1725
import static io.modelcontextprotocol.util.McpJsonMapperUtils.JSON_MAPPER;
@@ -67,4 +75,50 @@ void shouldRejectInboundMessageExceedingMaxSize() throws Exception {
6775
}
6876
}
6977

78+
@Test
79+
void shouldFailMalformedResponsesAndKeepProcessing(@TempDir Path tempDir) throws Exception {
80+
// A server process that answers with two malformed responses and a line that
81+
// is not JSON, followed by a valid response. It then stays alive until its
82+
// stdin is closed.
83+
Path serverOutput = tempDir.resolve("server-output.jsonl");
84+
Files.write(serverOutput,
85+
List.of("{\"id\":\"missing-jsonrpc\",\"result\":{}}",
86+
"{\"jsonrpc\":\"2.0\",\"id\":2,\"result\":{},\"error\":{\"code\":-32000,\"message\":\"boom\"}}",
87+
"this is not json", "{\"jsonrpc\":\"2.0\",\"id\":\"valid\",\"result\":{}}"));
88+
ServerParameters params = ServerParameters.builder("sh")
89+
.args("-c", "cat '" + serverOutput.toString().replace('\\', '/') + "'; cat > /dev/null")
90+
.build();
91+
92+
List<JSONRPCMessage> received = new CopyOnWriteArrayList<>();
93+
StdioClientTransport transport = new StdioClientTransport(params, JSON_MAPPER);
94+
try {
95+
StepVerifier.create(transport.connect(msg -> msg.doOnNext(received::add))).verifyComplete();
96+
97+
Awaitility.await()
98+
.atMost(Duration.ofSeconds(5))
99+
.pollInterval(Duration.ofMillis(100))
100+
.untilAsserted(() -> assertThat(received).hasSize(3));
101+
102+
// each malformed response fails the request it answers
103+
assertThat(received.get(0)).isInstanceOfSatisfying(JSONRPCResponse.class, response -> {
104+
assertThat(response.id()).isEqualTo("missing-jsonrpc");
105+
assertThat(response.result()).isNull();
106+
assertThat(response.error().code()).isEqualTo(McpSchema.ErrorCodes.INTERNAL_ERROR);
107+
});
108+
assertThat(received.get(1)).isInstanceOfSatisfying(JSONRPCResponse.class, response -> {
109+
assertThat(response.id()).isEqualTo(2);
110+
assertThat(response.result()).isNull();
111+
assertThat(response.error().code()).isEqualTo(McpSchema.ErrorCodes.INTERNAL_ERROR);
112+
});
113+
// the transport is still reading, so the valid response gets through
114+
assertThat(received.get(2)).isInstanceOfSatisfying(JSONRPCResponse.class, response -> {
115+
assertThat(response.id()).isEqualTo("valid");
116+
assertThat(response.error()).isNull();
117+
});
118+
}
119+
finally {
120+
StepVerifier.create(transport.closeGracefully()).verifyComplete();
121+
}
122+
}
123+
70124
}

0 commit comments

Comments
 (0)