diff --git a/api/controllers/console/datasets/datasets.py b/api/controllers/console/datasets/datasets.py index ea0fdef0a7..d001dfba64 100644 --- a/api/controllers/console/datasets/datasets.py +++ b/api/controllers/console/datasets/datasets.py @@ -50,6 +50,7 @@ from fields.dataset_fields import ( from fields.document_fields import document_status_fields from graphon.model_runtime.entities.model_entities import ModelType from libs.login import current_account_with_tenant, login_required +from libs.url_utils import normalize_api_base_url from models import ApiToken, Dataset, Document, DocumentSegment, UploadFile from models.dataset import DatasetPermission, DatasetPermissionEnum from models.enums import ApiTokenType, SegmentStatus @@ -889,7 +890,8 @@ class DatasetApiBaseUrlApi(Resource): @login_required @account_initialization_required def get(self): - return {"api_base_url": (dify_config.SERVICE_API_URL or request.host_url.rstrip("/")) + "/v1"} + base = dify_config.SERVICE_API_URL or request.host_url.rstrip("/") + return {"api_base_url": normalize_api_base_url(base)} @console_ns.route("/datasets/retrieval-setting") diff --git a/api/controllers/console/workspace/tool_providers.py b/api/controllers/console/workspace/tool_providers.py index 471594f349..34c9534de8 100644 --- a/api/controllers/console/workspace/tool_providers.py +++ b/api/controllers/console/workspace/tool_providers.py @@ -1131,6 +1131,14 @@ class ToolMCPAuthApi(Resource): with sessionmaker(db.engine).begin() as session: service = MCPToolManageService(session=session) service.clear_provider_credentials(provider_id=provider_id, tenant_id=tenant_id) + parsed = urlparse(server_url) + sanitized_url = f"{parsed.scheme}://{parsed.hostname}{parsed.path}" + logger.warning( + "MCP authorization failed for provider %s (url=%s)", + provider_id, + sanitized_url, + exc_info=True, + ) raise ValueError(f"Failed to connect to MCP server: {e}") from e diff --git a/api/core/mcp/client/streamable_client.py b/api/core/mcp/client/streamable_client.py index 5c3cd0d8f8..acba3e666b 100644 --- a/api/core/mcp/client/streamable_client.py +++ b/api/core/mcp/client/streamable_client.py @@ -303,9 +303,16 @@ class StreamableHTTPTransport: if response.status_code == 404: if isinstance(message.root, JSONRPCRequest): + error_msg = ( + f"MCP server URL returned 404 Not Found: {self.url} " + "— verify the server URL is correct and the server is running" + if is_initialization + else "Session terminated by server" + ) self._send_session_terminated_error( ctx.server_to_client_queue, message.root.id, + message=error_msg, ) return @@ -381,12 +388,13 @@ class StreamableHTTPTransport: self, server_to_client_queue: ServerToClientQueue, request_id: RequestId, + message: str = "Session terminated by server", ): """Send a session terminated error response.""" jsonrpc_error = JSONRPCError( jsonrpc="2.0", id=request_id, - error=ErrorData(code=32600, message="Session terminated by server"), + error=ErrorData(code=32600, message=message), ) session_message = SessionMessage(JSONRPCMessage(jsonrpc_error)) server_to_client_queue.put(session_message) diff --git a/api/libs/url_utils.py b/api/libs/url_utils.py new file mode 100644 index 0000000000..adcac3add0 --- /dev/null +++ b/api/libs/url_utils.py @@ -0,0 +1,3 @@ +def normalize_api_base_url(base_url: str) -> str: + """Normalize a base URL to always end with /v1, avoiding double /v1 suffixes.""" + return base_url.rstrip("/").removesuffix("/v1").rstrip("/") + "/v1" diff --git a/api/models/model.py b/api/models/model.py index 7fe0731098..a1117fc43a 100644 --- a/api/models/model.py +++ b/api/models/model.py @@ -25,6 +25,7 @@ from graphon.enums import WorkflowExecutionStatus from graphon.file import FILE_MODEL_IDENTITY, File, FileTransferMethod, FileType from graphon.file import helpers as file_helpers from libs.helper import generate_string # type: ignore[import-not-found] +from libs.url_utils import normalize_api_base_url from libs.uuid_utils import uuidv7 from models.utils.file_input_compat import build_file_from_input_mapping @@ -446,7 +447,8 @@ class App(Base): @property def api_base_url(self) -> str: - return (dify_config.SERVICE_API_URL or request.host_url.rstrip("/")) + "/v1" + base = dify_config.SERVICE_API_URL or request.host_url.rstrip("/") + return normalize_api_base_url(base) @property def tenant(self) -> Tenant | None: diff --git a/api/tests/unit_tests/controllers/console/datasets/test_datasets.py b/api/tests/unit_tests/controllers/console/datasets/test_datasets.py index 94d6c17915..9465936f28 100644 --- a/api/tests/unit_tests/controllers/console/datasets/test_datasets.py +++ b/api/tests/unit_tests/controllers/console/datasets/test_datasets.py @@ -1772,6 +1772,21 @@ class TestDatasetApiBaseUrlApi: assert response["api_base_url"] == "http://localhost:5000/v1" + def test_get_api_base_url_no_double_v1(self, app): + api = DatasetApiBaseUrlApi() + method = unwrap(api.get) + + with ( + app.test_request_context("/"), + patch( + "controllers.console.datasets.datasets.dify_config.SERVICE_API_URL", + "https://example.com/v1", + ), + ): + response = method(api) + + assert response["api_base_url"] == "https://example.com/v1" + class TestDatasetRetrievalSettingApi: def test_get_success(self, app): diff --git a/api/tests/unit_tests/core/mcp/client/test_streamable_http.py b/api/tests/unit_tests/core/mcp/client/test_streamable_http.py index 81f8da9a62..bbbffa2e69 100644 --- a/api/tests/unit_tests/core/mcp/client/test_streamable_http.py +++ b/api/tests/unit_tests/core/mcp/client/test_streamable_http.py @@ -971,6 +971,23 @@ class TestHandlePostRequestNew: assert isinstance(item, SessionMessage) assert isinstance(item.message.root, JSONRPCError) assert item.message.root.id == 77 + assert item.message.root.error.message == "Session terminated by server" + + def test_404_on_initialization_includes_url_in_error(self): + t = _new_transport(url="http://example.com/mcp/server/abc123/mcp") + q: queue.Queue = queue.Queue() + msg = _make_request_msg("initialize", 1) + ctx = self._make_ctx(t, q, message=msg) + mock_resp = MagicMock() + mock_resp.status_code = 404 + ctx.client.stream = self._stream_ctx(mock_resp) + t._handle_post_request(ctx) + item = q.get_nowait() + assert isinstance(item, SessionMessage) + assert isinstance(item.message.root, JSONRPCError) + assert item.message.root.error.code == 32600 + assert "404 Not Found" in item.message.root.error.message + assert "http://example.com/mcp/server/abc123/mcp" in item.message.root.error.message def test_404_for_notification_no_error_sent(self): t = _new_transport() diff --git a/web/app/components/tools/mcp/hooks/use-mcp-service-card.ts b/web/app/components/tools/mcp/hooks/use-mcp-service-card.ts index 8547b7f7ba..5ea6a237fc 100644 --- a/web/app/components/tools/mcp/hooks/use-mcp-service-card.ts +++ b/web/app/components/tools/mcp/hooks/use-mcp-service-card.ts @@ -69,7 +69,7 @@ export const useMCPServiceCardState = ( const serverPublished = !!id const serverActivated = status === 'active' const serverURL = serverPublished - ? `${appInfo.api_base_url.replace('/v1', '')}/mcp/server/${server_code}/mcp` + ? `${appInfo.api_base_url.replace(/\/v1$/, '')}/mcp/server/${server_code}/mcp` : '***********' // App state checks