Skip to content

Commit 30dc4d3

Browse files
chemicLKehrlann
authored andcommitted
Refactor HttpClient use to avoid capturing the sseSink that leads to leaking the httpClient reference due to a cycle
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf> Signed-off-by: Dariusz Jędrzejczyk <2554306+chemicL@users.noreply.github.com>
1 parent f5aab36 commit 30dc4d3

11 files changed

Lines changed: 1044 additions & 584 deletions

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

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import java.util.function.Consumer;
1717
import java.util.function.Function;
1818

19-
import io.modelcontextprotocol.client.transport.ResponseSubscribers.ResponseEvent;
19+
import io.modelcontextprotocol.client.transport.ResponseSubscribers.SseEvent;
2020
import io.modelcontextprotocol.client.transport.customizer.McpAsyncHttpClientRequestCustomizer;
2121
import io.modelcontextprotocol.client.transport.customizer.McpSyncHttpClientRequestCustomizer;
2222
import io.modelcontextprotocol.common.McpTransportContext;
@@ -353,59 +353,59 @@ public Mono<Void> connect(Function<Mono<JSONRPCMessage>, Mono<JSONRPCMessage>> h
353353
var transportContext = ctx.getOrDefault(McpTransportContext.KEY, McpTransportContext.EMPTY);
354354
return Mono.from(this.httpRequestCustomizer.customize(builder, "GET", uri, null, transportContext));
355355
}).flatMap(requestBuilder -> Mono.create(sink -> {
356-
Disposable connection = Flux.<ResponseEvent>create(sseSink -> this.httpClient
357-
.sendAsync(requestBuilder.build(),
358-
responseInfo -> ResponseSubscribers.sseToBodySubscriber(responseInfo, sseSink))
359-
.exceptionallyCompose(e -> {
360-
sseSink.error(e);
361-
return CompletableFuture.failedFuture(e);
362-
}))
363-
.map(responseEvent -> (ResponseSubscribers.SseResponseEvent) responseEvent)
364-
.flatMap(responseEvent -> {
356+
Disposable connection = Mono
357+
.fromFuture(() -> this.httpClient.sendAsync(requestBuilder.build(),
358+
HttpResponse.BodyHandlers.ofPublisher()))
359+
.flatMapMany(response -> {
365360
if (isClosing) {
366-
return Mono.empty();
361+
return Flux.empty();
367362
}
368363

369-
int statusCode = responseEvent.responseInfo().statusCode();
364+
int statusCode = response.statusCode();
370365

371366
if (statusCode >= 200 && statusCode < 300) {
372-
try {
373-
if (ENDPOINT_EVENT_TYPE.equals(responseEvent.sseEvent().event())) {
374-
String messageEndpointUri = responseEvent.sseEvent().data();
375-
try {
376-
messageEndpointValidator.validate(uri, messageEndpointUri);
377-
}
378-
catch (InvalidSseMessageEndpointException e) {
379-
sink.error(e);
380-
this.messageEndpointSink.tryEmitError(e);
381-
return Flux.error(e);
382-
}
383-
if (this.messageEndpointSink.tryEmitValue(messageEndpointUri).isSuccess()) {
384-
sink.success();
385-
return Flux.empty(); // No further processing needed
386-
}
387-
else {
388-
sink.error(new RuntimeException("Failed to handle SSE endpoint event"));
389-
}
367+
Flux<String> lines = ResponseSubscribers.decodeLines(response.body());
368+
return ResponseSubscribers.decodeSseResponse(lines);
369+
}
370+
else {
371+
return ResponseSubscribers.drainThenError(response.body(),
372+
new RuntimeException("Failed to connect to SSE stream: " + statusCode));
373+
}
374+
})
375+
.flatMap(sseEvent -> {
376+
try {
377+
if (ENDPOINT_EVENT_TYPE.equals(sseEvent.event())) {
378+
String messageEndpointUri = sseEvent.data();
379+
try {
380+
messageEndpointValidator.validate(uri, messageEndpointUri);
390381
}
391-
else if (MESSAGE_EVENT_TYPE.equals(responseEvent.sseEvent().event())) {
392-
JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper,
393-
responseEvent.sseEvent().data());
382+
catch (InvalidSseMessageEndpointException e) {
383+
sink.error(e);
384+
this.messageEndpointSink.tryEmitError(e);
385+
return Flux.error(e);
386+
}
387+
if (this.messageEndpointSink.tryEmitValue(messageEndpointUri).isSuccess()) {
394388
sink.success();
395-
return Flux.just(message);
389+
return Flux.empty(); // No further processing needed
396390
}
397391
else {
398-
logger.debug("Received unrecognized SSE event type: {}", responseEvent.sseEvent());
399-
sink.success();
392+
sink.error(new RuntimeException("Failed to handle SSE endpoint event"));
400393
}
401394
}
402-
catch (IOException e) {
403-
sink.error(new McpTransportException("Error processing SSE event", e));
395+
else if (MESSAGE_EVENT_TYPE.equals(sseEvent.event())) {
396+
JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper, sseEvent.data());
397+
sink.success();
398+
return Flux.just(message);
399+
}
400+
else {
401+
logger.debug("Received unrecognized SSE event type: {}", sseEvent);
402+
sink.success();
404403
}
405404
}
406-
return Flux.<McpSchema.JSONRPCMessage>error(
407-
new RuntimeException("Failed to send message: " + responseEvent));
408-
405+
catch (IOException e) {
406+
sink.error(new McpTransportException("Error processing SSE event", e));
407+
}
408+
return Flux.<McpSchema.JSONRPCMessage>empty();
409409
})
410410
.flatMap(jsonRpcMessage -> handler.apply(Mono.just(jsonRpcMessage)))
411411
.onErrorComplete(t -> {
@@ -490,7 +490,7 @@ private Mono<HttpResponse<String>> sendHttpPost(final String endpoint, final Str
490490
return Mono.from(this.httpRequestCustomizer.customize(builder, "POST", requestUri, body, transportContext));
491491
}).flatMap(customizedBuilder -> {
492492
var request = customizedBuilder.build();
493-
return Mono.fromFuture(httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString()));
493+
return Mono.fromFuture(this.httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString()));
494494
});
495495
}
496496

0 commit comments

Comments
 (0)