Commit 62ae4f16 by Serhii Zghama Committed by GitHub

fix(mcp): only fall back to legacy SSE transport on a 4xx Streamable HTTP error (#7154)

* fix(mcp): only fall back to legacy SSE transport on a 4xx Streamable HTTP error

The MCP client caught any Streamable HTTP connection error and immediately
retried with the legacy HTTP+SSE transport. For a Streamable-HTTP-only server,
a failure unrelated to protocol support (network error, 5xx, or an error after
a successful initialize) was masked by a misleading SSE error such as
"Non-200 status code (405)", hiding the real root cause.

Follow the MCP spec's backwards-compatibility flow: fall back to SSE only when
the server rejects the Streamable HTTP request with a 4xx (e.g. 404/405),
which signals it does not speak Streamable HTTP. Other errors now surface
as-is, and when the SSE fallback also fails both errors are reported.

* test(mcp): cover conditional SSE fallback behavior

Update the fallback test to use a 4xx StreamableHTTPError and add cases for:
non-HTTP errors and 5xx not triggering fallback, and both errors being
surfaced when the SSE fallback also fails.

* perf: code

* perf: code

---------

Co-authored-by: archer <545436317@qq.com>
parent 1d3557ca
import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'; import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; import {
StreamableHTTPClientTransport,
StreamableHTTPError
} from '@modelcontextprotocol/sdk/client/streamableHttp.js';
import type { AppSchemaType } from '@fastgpt/global/core/app/type'; import type { AppSchemaType } from '@fastgpt/global/core/app/type';
import { type McpToolConfigType } from '@fastgpt/global/core/app/tool/mcpTool/type'; import { type McpToolConfigType } from '@fastgpt/global/core/app/tool/mcpTool/type';
import { retryFn } from '@fastgpt/global/common/system/utils'; import { retryFn } from '@fastgpt/global/common/system/utils';
...@@ -20,6 +23,23 @@ export const assertMCPUrlNotInternal = async (url: string) => { ...@@ -20,6 +23,23 @@ export const assertMCPUrlNotInternal = async (url: string) => {
} }
}; };
const shouldFallbackToSSE = (error: unknown): boolean => {
return (
error instanceof StreamableHTTPError &&
typeof error.code === 'number' &&
error.code >= 400 &&
error.code < 500
);
};
const getErrorMessage = (error: unknown) => {
if (error instanceof Error) {
return error.message;
}
return String(error);
};
export class MCPClient { export class MCPClient {
private client: Client; private client: Client;
private url: string; private url: string;
...@@ -63,7 +83,13 @@ export class MCPClient { ...@@ -63,7 +83,13 @@ export class MCPClient {
} }
}); });
await this.client.connect(transport); await this.client.connect(transport);
} catch (error) { } catch (streamableError: any) {
if (!shouldFallbackToSSE(streamableError)) {
logger.info('Streamable HTTP error', streamableError);
throw streamableError;
}
try {
await this.client.connect( await this.client.connect(
new SSEClientTransport(new URL(this.url), { new SSEClientTransport(new URL(this.url), {
requestInit: { requestInit: {
...@@ -93,6 +119,14 @@ export class MCPClient { ...@@ -93,6 +119,14 @@ export class MCPClient {
} }
}) })
); );
} catch (sseError: any) {
logger.info('SSE error', sseError);
throw new Error(
`MCP connection failed. Streamable HTTP: ${getErrorMessage(
streamableError
)}; SSE: ${getErrorMessage(sseError)}`
);
}
} }
this.client.onerror = (error) => { this.client.onerror = (error) => {
......
...@@ -18,6 +18,7 @@ vi.mock('@fastgpt/service/core/app/schema', () => ({ ...@@ -18,6 +18,7 @@ vi.mock('@fastgpt/service/core/app/schema', () => ({
} }
})); }));
import { StreamableHTTPError } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
import { MCPClient, assertMCPUrlNotInternal, getMCPChildren } from '@fastgpt/service/core/app/mcp'; import { MCPClient, assertMCPUrlNotInternal, getMCPChildren } from '@fastgpt/service/core/app/mcp';
import type { AppSchemaType } from '@fastgpt/global/core/app/type'; import type { AppSchemaType } from '@fastgpt/global/core/app/type';
...@@ -423,13 +424,13 @@ describe('MCPClient', () => { ...@@ -423,13 +424,13 @@ describe('MCPClient', () => {
}); });
describe('getConnection', () => { describe('getConnection', () => {
it('should fallback to SSE when StreamableHTTP fails', async () => { it('should fallback to SSE when server rejects Streamable HTTP with a 4xx', async () => {
const mcpClient = new MCPClient(config); const mcpClient = new MCPClient(config);
const client = getPrivateClient(mcpClient); const client = getPrivateClient(mcpClient);
// First connect (StreamableHTTP) fails, second (SSE) succeeds // StreamableHTTP rejected with 405 (server speaks legacy SSE), SSE succeeds
client.connect = vi client.connect = vi
.fn() .fn()
.mockRejectedValueOnce(new Error('streamable failed')) .mockRejectedValueOnce(new StreamableHTTPError(405, 'Method Not Allowed'))
.mockResolvedValueOnce(undefined); .mockResolvedValueOnce(undefined);
const result = await (mcpClient as any).getConnection(); const result = await (mcpClient as any).getConnection();
...@@ -437,12 +438,39 @@ describe('MCPClient', () => { ...@@ -437,12 +438,39 @@ describe('MCPClient', () => {
expect(result).toBe(client); expect(result).toBe(client);
}); });
it('should reject when both transports fail', async () => { it('should not fallback to SSE on a non-HTTP (e.g. network) error', async () => {
const mcpClient = new MCPClient(config); const mcpClient = new MCPClient(config);
const client = getPrivateClient(mcpClient); const client = getPrivateClient(mcpClient);
client.connect = vi.fn().mockRejectedValue(new Error('all failed')); client.connect = vi.fn().mockRejectedValue(new Error('network unreachable'));
await expect((mcpClient as any).getConnection()).rejects.toThrow('all failed'); await expect((mcpClient as any).getConnection()).rejects.toThrow('network unreachable');
// Original error surfaces as-is, SSE transport is not attempted
expect(client.connect).toHaveBeenCalledTimes(1);
});
it('should not fallback to SSE when Streamable HTTP fails with a 5xx', async () => {
const mcpClient = new MCPClient(config);
const client = getPrivateClient(mcpClient);
client.connect = vi
.fn()
.mockRejectedValue(new StreamableHTTPError(500, 'Internal Server Error'));
await expect((mcpClient as any).getConnection()).rejects.toThrow('Internal Server Error');
expect(client.connect).toHaveBeenCalledTimes(1);
});
it('should surface both errors when the SSE fallback also fails', async () => {
const mcpClient = new MCPClient(config);
const client = getPrivateClient(mcpClient);
client.connect = vi
.fn()
.mockRejectedValueOnce(new StreamableHTTPError(404, 'Not Found'))
.mockRejectedValueOnce(new Error('SSE handshake failed'));
await expect((mcpClient as any).getConnection()).rejects.toThrow(
/Streamable HTTP:.*Not Found.*SSE:.*SSE handshake failed/s
);
expect(client.connect).toHaveBeenCalledTimes(2);
}); });
it('should return client on StreamableHTTP success', async () => { it('should return client on StreamableHTTP success', async () => {
......
Subproject commit 465869779095ca30d14a2852c220dd2d896a6c67 Subproject commit 722e7f70a59c2614e05143f62f50230d6efb641b
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment