mcp: add DisableListening option to StreamableClientTransport (#729)
mcp: add DisableStandaloneSSE option to control standalone SSE stream Currently, StreamableClientTransport always establishes a standalone SSE connection after initialization to receive server-initiated messages. While this is useful for receiving notifications like ToolListChangedNotification, it causes problems in several real-world scenarios: 1. **Unnecessary resource usage**: Many use cases (for me), especially in scientific computing scenarios, only require simple request-response communication. Clients don't need server-initiated notifications, making the persistent SSE connection wasteful. 2. **Server compatibility issues**: Some third-party MCP servers don't properly handle GET requests for SSE streams. When the client automatically tries to establish the SSE connection, it fails or hangs, blocking the entire connection process. This is particularly problematic when using third-party servers that cannot be modified. This issue is similar to what was discussed in issue #634. 3. **Lack of user control**: The MCP specification states that the standalone SSE stream is optional ("The client MAY issue an HTTP GET"), but the current SDK implementation doesn't provide a way to opt out. This implementation is also inconsistent with other MCP SDKs like github.com/mark3labs/mcp-go, which provides getListeningEnabled control (defaults to false, requires explicit enablement). Fixes #728 Co-authored-by: zhaoxin <zhaoxin@pjlab.org.cn>
This commit is contained in:
parent
a225d4dc7d
commit
b4f957ff3c
1 changed files with 158 additions and 13 deletions
|
|
@ -696,3 +696,121 @@ func TestStreamableClientTransientErrors(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestStreamableClientDisableStandaloneSSE(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
disableStandaloneSSE bool
|
||||
expectGETRequest bool
|
||||
}{
|
||||
{
|
||||
name: "default behavior (standalone SSE enabled)",
|
||||
disableStandaloneSSE: false,
|
||||
expectGETRequest: true,
|
||||
},
|
||||
{
|
||||
name: "standalone SSE disabled",
|
||||
disableStandaloneSSE: true,
|
||||
expectGETRequest: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
getRequestKey := streamableRequestKey{"GET", "123", "", ""}
|
||||
|
||||
fake := &fakeStreamableServer{
|
||||
t: t,
|
||||
responses: fakeResponses{
|
||||
{"POST", "", methodInitialize, ""}: {
|
||||
header: header{
|
||||
"Content-Type": "application/json",
|
||||
sessionIDHeader: "123",
|
||||
},
|
||||
body: jsonBody(t, initResp),
|
||||
},
|
||||
{"POST", "123", notificationInitialized, ""}: {
|
||||
status: http.StatusAccepted,
|
||||
wantProtocolVersion: latestProtocolVersion,
|
||||
},
|
||||
getRequestKey: {
|
||||
header: header{
|
||||
"Content-Type": "text/event-stream",
|
||||
},
|
||||
wantProtocolVersion: latestProtocolVersion,
|
||||
optional: !test.expectGETRequest,
|
||||
},
|
||||
{"DELETE", "123", "", ""}: {
|
||||
optional: true,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
httpServer := httptest.NewServer(fake)
|
||||
defer httpServer.Close()
|
||||
|
||||
transport := &StreamableClientTransport{
|
||||
Endpoint: httpServer.URL,
|
||||
DisableStandaloneSSE: test.disableStandaloneSSE,
|
||||
}
|
||||
client := NewClient(testImpl, nil)
|
||||
session, err := client.Connect(ctx, transport, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("client.Connect() failed: %v", err)
|
||||
}
|
||||
|
||||
// Give some time for the standalone SSE connection to be established (if enabled)
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
|
||||
// Verify the connection state
|
||||
streamableConn, ok := session.mcpConn.(*streamableClientConn)
|
||||
if !ok {
|
||||
t.Fatalf("Expected *streamableClientConn, got %T", session.mcpConn)
|
||||
}
|
||||
|
||||
if got, want := streamableConn.disableStandaloneSSE, test.disableStandaloneSSE; got != want {
|
||||
t.Errorf("disableStandaloneSSE field: got %v, want %v", got, want)
|
||||
}
|
||||
|
||||
// Clean up
|
||||
if err := session.Close(); err != nil {
|
||||
t.Errorf("closing session: %v", err)
|
||||
}
|
||||
|
||||
// Check if GET request was received
|
||||
fake.calledMu.Lock()
|
||||
getRequestReceived := false
|
||||
if fake.called != nil {
|
||||
getRequestReceived = fake.called[getRequestKey]
|
||||
}
|
||||
fake.calledMu.Unlock()
|
||||
|
||||
if got, want := getRequestReceived, test.expectGETRequest; got != want {
|
||||
t.Errorf("GET request received: got %v, want %v", got, want)
|
||||
}
|
||||
|
||||
// If we expected a GET request, verify it was actually received
|
||||
if test.expectGETRequest {
|
||||
if missing := fake.missingRequests(); len(missing) > 0 {
|
||||
// Filter out optional requests
|
||||
var requiredMissing []streamableRequestKey
|
||||
for _, key := range missing {
|
||||
if resp, ok := fake.responses[key]; ok && !resp.optional {
|
||||
requiredMissing = append(requiredMissing, key)
|
||||
}
|
||||
}
|
||||
if len(requiredMissing) > 0 {
|
||||
t.Errorf("did not receive expected requests: %v", requiredMissing)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// If we didn't expect a GET request, verify it wasn't sent
|
||||
if getRequestReceived {
|
||||
t.Error("GET request was sent unexpectedly when DisableStandaloneSSE is true")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue