feat: WebClient Streamable HTTP support (#292)
An implementation of Streamable HTTP Client with WebFlux WebClient. Aside from implementing the specification, several improvements have been incorporated throughout the client-side of the architecture. The changes cover: - resilience tests using toxiproxy in testcontainers - integration tests using updated everything-server with streamableHttp support - improved logging - session invalidation handling (both transport session and JSON-RPC concept of session) - implicit initialization and burst protection (in case of concurrent `Mcp(Sync|Async)Client` use - more logging, e.g. stdio process lifecycle logs Related #72, #273, #253, #107, #105 Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
This commit is contained in:
Dariusz Jędrzejczyk
GitHub
parent
8a5a591d39
commit
2f944349cc
6 changed files with 1501 additions and 242 deletions
|
|
@ -113,14 +113,16 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
onClose();
|
||||
}
|
||||
|
||||
<T> void verifyInitializationTimeout(Function<McpAsyncClient, Mono<T>> operation, String action) {
|
||||
<T> void verifyNotificationSucceedsWithImplicitInitialization(Function<McpAsyncClient, Mono<T>> operation,
|
||||
String action) {
|
||||
withClient(createMcpTransport(), mcpAsyncClient -> {
|
||||
StepVerifier.withVirtualTime(() -> operation.apply(mcpAsyncClient))
|
||||
.expectSubscription()
|
||||
.thenAwait(getInitializationTimeout())
|
||||
.consumeErrorWith(e -> assertThat(e).isInstanceOf(McpError.class)
|
||||
.hasMessage("Client must be initialized before " + action))
|
||||
.verify();
|
||||
StepVerifier.create(operation.apply(mcpAsyncClient)).verifyComplete();
|
||||
});
|
||||
}
|
||||
|
||||
<T> void verifyCallSucceedsWithImplicitInitialization(Function<McpAsyncClient, Mono<T>> operation, String action) {
|
||||
withClient(createMcpTransport(), mcpAsyncClient -> {
|
||||
StepVerifier.create(operation.apply(mcpAsyncClient)).expectNextCount(1).verifyComplete();
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -136,7 +138,7 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
|
||||
@Test
|
||||
void testListToolsWithoutInitialization() {
|
||||
verifyInitializationTimeout(client -> client.listTools(null), "listing tools");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.listTools(null), "listing tools");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -156,7 +158,7 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
|
||||
@Test
|
||||
void testPingWithoutInitialization() {
|
||||
verifyInitializationTimeout(client -> client.ping(), "pinging the server");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.ping(), "pinging the server");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -171,7 +173,7 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
@Test
|
||||
void testCallToolWithoutInitialization() {
|
||||
CallToolRequest callToolRequest = new CallToolRequest("echo", Map.of("message", ECHO_TEST_MESSAGE));
|
||||
verifyInitializationTimeout(client -> client.callTool(callToolRequest), "calling tools");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.callTool(callToolRequest), "calling tools");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -205,7 +207,7 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
|
||||
@Test
|
||||
void testListResourcesWithoutInitialization() {
|
||||
verifyInitializationTimeout(client -> client.listResources(null), "listing resources");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.listResources(null), "listing resources");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -236,7 +238,7 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
|
||||
@Test
|
||||
void testListPromptsWithoutInitialization() {
|
||||
verifyInitializationTimeout(client -> client.listPrompts(null), "listing " + "prompts");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.listPrompts(null), "listing " + "prompts");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -261,7 +263,7 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
@Test
|
||||
void testGetPromptWithoutInitialization() {
|
||||
GetPromptRequest request = new GetPromptRequest("simple_prompt", Map.of());
|
||||
verifyInitializationTimeout(client -> client.getPrompt(request), "getting " + "prompts");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.getPrompt(request), "getting " + "prompts");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -282,7 +284,7 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
|
||||
@Test
|
||||
void testRootsListChangedWithoutInitialization() {
|
||||
verifyInitializationTimeout(client -> client.rootsListChangedNotification(),
|
||||
verifyNotificationSucceedsWithImplicitInitialization(client -> client.rootsListChangedNotification(),
|
||||
"sending roots list changed notification");
|
||||
}
|
||||
|
||||
|
|
@ -357,7 +359,8 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
|
||||
@Test
|
||||
void testListResourceTemplatesWithoutInitialization() {
|
||||
verifyInitializationTimeout(client -> client.listResourceTemplates(), "listing resource templates");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.listResourceTemplates(),
|
||||
"listing resource templates");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -468,8 +471,8 @@ public abstract class AbstractMcpAsyncClientTests {
|
|||
|
||||
@Test
|
||||
void testLoggingLevelsWithoutInitialization() {
|
||||
verifyInitializationTimeout(client -> client.setLoggingLevel(McpSchema.LoggingLevel.DEBUG),
|
||||
"setting logging level");
|
||||
verifyNotificationSucceedsWithImplicitInitialization(
|
||||
client -> client.setLoggingLevel(McpSchema.LoggingLevel.DEBUG), "setting logging level");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@
|
|||
package io.modelcontextprotocol.client;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
|
@ -12,7 +13,6 @@ import java.util.function.Consumer;
|
|||
import java.util.function.Function;
|
||||
|
||||
import io.modelcontextprotocol.spec.McpClientTransport;
|
||||
import io.modelcontextprotocol.spec.McpError;
|
||||
import io.modelcontextprotocol.spec.McpSchema;
|
||||
import io.modelcontextprotocol.spec.McpSchema.CallToolRequest;
|
||||
import io.modelcontextprotocol.spec.McpSchema.CallToolResult;
|
||||
|
|
@ -113,33 +113,18 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
|
||||
static final Object DUMMY_RETURN_VALUE = new Object();
|
||||
|
||||
<T> void verifyNotificationTimesOut(Consumer<McpSyncClient> operation, String action) {
|
||||
verifyCallTimesOut(client -> {
|
||||
<T> void verifyNotificationSucceedsWithImplicitInitialization(Consumer<McpSyncClient> operation, String action) {
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> {
|
||||
operation.accept(client);
|
||||
return DUMMY_RETURN_VALUE;
|
||||
}, action);
|
||||
}
|
||||
|
||||
<T> void verifyCallTimesOut(Function<McpSyncClient, T> blockingOperation, String action) {
|
||||
<T> void verifyCallSucceedsWithImplicitInitialization(Function<McpSyncClient, T> blockingOperation, String action) {
|
||||
withClient(createMcpTransport(), mcpSyncClient -> {
|
||||
// This scheduler is not replaced by virtual time scheduler
|
||||
Scheduler customScheduler = Schedulers.newBoundedElastic(1, 1, "actualBoundedElastic");
|
||||
|
||||
StepVerifier.withVirtualTime(() -> Mono.fromSupplier(() -> blockingOperation.apply(mcpSyncClient))
|
||||
StepVerifier.create(Mono.fromSupplier(() -> blockingOperation.apply(mcpSyncClient))
|
||||
// Offload the blocking call to the real scheduler
|
||||
.subscribeOn(customScheduler))
|
||||
.expectSubscription()
|
||||
// This works without actually waiting but executes all the
|
||||
// tasks pending execution on the VirtualTimeScheduler.
|
||||
// It is possible to execute the blocking code from the operation
|
||||
// because it is blocked on a dedicated Scheduler and the main
|
||||
// flow is not blocked and uses the VirtualTimeScheduler.
|
||||
.thenAwait(getInitializationTimeout())
|
||||
.consumeErrorWith(e -> assertThat(e).isInstanceOf(McpError.class)
|
||||
.hasMessage("Client must be initialized before " + action))
|
||||
.verify();
|
||||
|
||||
customScheduler.dispose();
|
||||
.subscribeOn(Schedulers.boundedElastic())).expectNextCount(1).verifyComplete();
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -155,7 +140,7 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
|
||||
@Test
|
||||
void testListToolsWithoutInitialization() {
|
||||
verifyCallTimesOut(client -> client.listTools(null), "listing tools");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.listTools(null), "listing tools");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -176,8 +161,8 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
|
||||
@Test
|
||||
void testCallToolsWithoutInitialization() {
|
||||
verifyCallTimesOut(client -> client.callTool(new CallToolRequest("add", Map.of("a", 3, "b", 4))),
|
||||
"calling tools");
|
||||
verifyCallSucceedsWithImplicitInitialization(
|
||||
client -> client.callTool(new CallToolRequest("add", Map.of("a", 3, "b", 4))), "calling tools");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -201,7 +186,7 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
|
||||
@Test
|
||||
void testPingWithoutInitialization() {
|
||||
verifyCallTimesOut(client -> client.ping(), "pinging the server");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.ping(), "pinging the server");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -215,7 +200,7 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
@Test
|
||||
void testCallToolWithoutInitialization() {
|
||||
CallToolRequest callToolRequest = new CallToolRequest("echo", Map.of("message", TEST_MESSAGE));
|
||||
verifyCallTimesOut(client -> client.callTool(callToolRequest), "calling tools");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.callTool(callToolRequest), "calling tools");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -244,7 +229,7 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
|
||||
@Test
|
||||
void testRootsListChangedWithoutInitialization() {
|
||||
verifyNotificationTimesOut(client -> client.rootsListChangedNotification(),
|
||||
verifyNotificationSucceedsWithImplicitInitialization(client -> client.rootsListChangedNotification(),
|
||||
"sending roots list changed notification");
|
||||
}
|
||||
|
||||
|
|
@ -258,7 +243,7 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
|
||||
@Test
|
||||
void testListResourcesWithoutInitialization() {
|
||||
verifyCallTimesOut(client -> client.listResources(null), "listing resources");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.listResources(null), "listing resources");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -334,8 +319,14 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
|
||||
@Test
|
||||
void testReadResourceWithoutInitialization() {
|
||||
Resource resource = new Resource("test://uri", "Test Resource", null, null, null);
|
||||
verifyCallTimesOut(client -> client.readResource(resource), "reading resources");
|
||||
AtomicReference<List<Resource>> resources = new AtomicReference<>();
|
||||
withClient(createMcpTransport(), mcpSyncClient -> {
|
||||
mcpSyncClient.initialize();
|
||||
resources.set(mcpSyncClient.listResources().resources());
|
||||
});
|
||||
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.readResource(resources.get().get(0)),
|
||||
"reading resources");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -356,7 +347,8 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
|
||||
@Test
|
||||
void testListResourceTemplatesWithoutInitialization() {
|
||||
verifyCallTimesOut(client -> client.listResourceTemplates(null), "listing resource templates");
|
||||
verifyCallSucceedsWithImplicitInitialization(client -> client.listResourceTemplates(null),
|
||||
"listing resource templates");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -414,8 +406,8 @@ public abstract class AbstractMcpSyncClientTests {
|
|||
|
||||
@Test
|
||||
void testLoggingLevelsWithoutInitialization() {
|
||||
verifyNotificationTimesOut(client -> client.setLoggingLevel(McpSchema.LoggingLevel.DEBUG),
|
||||
"setting logging level");
|
||||
verifyNotificationSucceedsWithImplicitInitialization(
|
||||
client -> client.setLoggingLevel(McpSchema.LoggingLevel.DEBUG), "setting logging level");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -22,7 +22,8 @@ class HttpSseMcpAsyncClientTests extends AbstractMcpAsyncClientTests {
|
|||
|
||||
// Uses the https://github.com/tzolov/mcp-everything-server-docker-image
|
||||
@SuppressWarnings("resource")
|
||||
GenericContainer<?> container = new GenericContainer<>("docker.io/tzolov/mcp-everything-server:v1")
|
||||
GenericContainer<?> container = new GenericContainer<>("docker.io/tzolov/mcp-everything-server:v2")
|
||||
.withCommand("node dist/index.js sse")
|
||||
.withLogConsumer(outputFrame -> System.out.println(outputFrame.getUtf8String()))
|
||||
.withExposedPorts(3001)
|
||||
.waitingFor(Wait.forHttp("/").forStatusCode(404));
|
||||
|
|
|
|||
|
|
@ -22,7 +22,8 @@ class HttpSseMcpSyncClientTests extends AbstractMcpSyncClientTests {
|
|||
|
||||
// Uses the https://github.com/tzolov/mcp-everything-server-docker-image
|
||||
@SuppressWarnings("resource")
|
||||
GenericContainer<?> container = new GenericContainer<>("docker.io/tzolov/mcp-everything-server:v1")
|
||||
GenericContainer<?> container = new GenericContainer<>("docker.io/tzolov/mcp-everything-server:v2")
|
||||
.withCommand("node dist/index.js sse")
|
||||
.withLogConsumer(outputFrame -> System.out.println(outputFrame.getUtf8String()))
|
||||
.withExposedPorts(3001)
|
||||
.waitingFor(Wait.forHttp("/").forStatusCode(404));
|
||||
|
|
|
|||
|
|
@ -68,7 +68,7 @@ class StdioMcpSyncClientTests extends AbstractMcpSyncClientTests {
|
|||
}
|
||||
|
||||
protected Duration getInitializationTimeout() {
|
||||
return Duration.ofSeconds(6);
|
||||
return Duration.ofSeconds(10);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -51,7 +51,8 @@ class HttpClientSseClientTransportTests {
|
|||
static String host = "http://localhost:3001";
|
||||
|
||||
@SuppressWarnings("resource")
|
||||
GenericContainer<?> container = new GenericContainer<>("docker.io/tzolov/mcp-everything-server:v1")
|
||||
GenericContainer<?> container = new GenericContainer<>("docker.io/tzolov/mcp-everything-server:v2")
|
||||
.withCommand("node dist/index.js sse")
|
||||
.withLogConsumer(outputFrame -> System.out.println(outputFrame.getUtf8String()))
|
||||
.withExposedPorts(3001)
|
||||
.waitingFor(Wait.forHttp("/").forStatusCode(404));
|
||||
|
|
|
|||
Loading…
Reference in a new issue