Skip to content

Commit a252295

Browse files
committed
Ignore unsupported SSE event
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
1 parent bbb3330 commit a252295

3 files changed

Lines changed: 17 additions & 10 deletions

File tree

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,8 @@ private int indexOfLineTerminator(int from) {
356356
* fields until a blank line dispatches the event. Per the SSE spec, {@code id} and
357357
* {@code event} persist across events until re-set; {@code data} is reset after each
358358
* dispatch, and a blank line dispatches only when a {@code data:} field was seen,
359-
* whether or not it carried a value.
359+
* whether or not it carried a value. Comments and fields the parser does not handle,
360+
* such as {@code retry:}, are ignored as the spec requires.
360361
*/
361362
static final class SseEventParser {
362363

@@ -421,7 +422,9 @@ else if (line.startsWith(":")) {
421422
logger.debug("Ignoring comment line: {}", line);
422423
}
423424
else {
424-
throw new McpTransportException("Invalid SSE response line: " + line);
425+
// The SSE spec mandates that fields the client does not know about, such
426+
// as `retry:`, are ignored rather than treated as a protocol error.
427+
logger.debug("Ignoring unknown SSE field line: {}", line);
425428
}
426429
return Optional.empty();
427430
}

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@
1010

1111
import io.modelcontextprotocol.client.transport.ResponseSubscribers.SseEvent;
1212
import io.modelcontextprotocol.client.transport.ResponseSubscribers.SseEventParser;
13-
import io.modelcontextprotocol.spec.McpTransportException;
1413

1514
import static org.assertj.core.api.Assertions.assertThat;
16-
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1715

1816
class SseEventParserTests {
1917

@@ -100,11 +98,16 @@ void blankLineWithNoPendingDataIsEmpty() {
10098
}
10199

102100
@Test
103-
void unknownFieldThrowsMcpTransportException() {
101+
void unknownFieldsAreIgnored() {
102+
// The SSE spec mandates that unknown fields are ignored, so neither a standard
103+
// field the parser does not act on nor a malformed line may fail the stream.
104104
SseEventParser p = new SseEventParser(Integer.MAX_VALUE);
105-
assertThatThrownBy(() -> p.feed("bogus line")).isInstanceOf(McpTransportException.class)
106-
.hasMessageContaining("Invalid SSE response line")
107-
.hasMessageContaining("bogus line");
105+
assertThat(p.feed("retry: 3000")).isEmpty();
106+
assertThat(p.feed("bogus line")).isEmpty();
107+
assertThat(p.feed("data: hello")).isEmpty();
108+
Optional<SseEvent> event = p.feed("");
109+
assertThat(event).isPresent();
110+
assertThat(event.get().data()).isEqualTo("hello");
108111
}
109112

110113
@Test

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,9 @@ void loneCrTerminatesLine() {
178178
// SSE takes its line endings from HTML, which terminates on CRLF, CR and LF
179179
// alike, and HttpResponse.BodySubscribers#fromLineSubscriber -- the path this
180180
// decoder replaces -- splits on all three. Splitting on LF alone leaves a
181-
// CR-framed stream as one unterminated run: downstream an "Invalid SSE response
182-
// line", or past BoundedLineBodySubscriber's bound an aborted response.
181+
// CR-framed stream as one unterminated run: downstream a single unparseable line
182+
// whose SSE fields are silently dropped, leaving the request the stream answers
183+
// hanging, or past BoundedLineBodySubscriber's bound an aborted response.
183184
Utf8LineDecoder dec = new Utf8LineDecoder();
184185
assertThat(dec.decode(chunk("one\rtwo\rthree\r"))).containsExactly("one", "two", "three");
185186
assertThat(dec.flush()).isEmpty();

0 commit comments

Comments
 (0)