feat: add support for mcp http 400 #540
Conversation
WalkthroughThe changes introduce robust session error detection and automatic recovery logic to the MCP client, centralizing error handling and recovery in the codebase. A new documentation file explains the mechanism and its workflow. Additionally, a minor syntax update is made in the Gemini provider code, with no functional impact. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant McpClient
participant MCPServer
participant EventBus
Client->>McpClient: callTool / listTools / etc.
McpClient->>MCPServer: Send request
MCPServer-->>McpClient: Error (session expired/invalid)
McpClient->>McpClient: Detect session error
alt First session error
McpClient->>McpClient: Restart service, clear caches
McpClient-->>Client: Propagate error
Client->>McpClient: Retry operation
McpClient->>MCPServer: Send request
MCPServer-->>McpClient: Error (if still session error)
McpClient->>McpClient: Stop service
McpClient->>EventBus: Emit server status event
else Not a session error
McpClient-->>Client: Propagate error
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/mcp-session-recovery.md (3)
7-9: Specify language for code blocks to improve readability.The fenced code blocks should have language specifiers for proper syntax highlighting.
Apply this diff to add language specifiers:
-``` +```text Error POSTing to endpoint (HTTP 400): Bad Request: No valid session ID provided -``` +```
158-158: Minor language correction suggestion.The static analysis tool suggests a grammatical improvement for Chinese text flow.
Consider changing "新的连接" to "新地连接" for better Chinese grammar flow, though this is a very minor stylistic preference.
175-177: Add language specifiers to log output code blocks.The log output examples should specify the language for better formatting.
Apply this diff to improve the formatting:
-``` +```text Session error detected for server doris_server, restarting service... Service doris_server restarted due to session error -``` +``` -``` +```text Session error persists after restart for server doris_server, stopping service... MCP service doris_server has been stopped due to persistent session errors -``` +```Also applies to: 181-183
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/mcp-session-recovery.md(1 hunks)src/main/presenter/llmProviderPresenter/providers/geminiProvider.ts(1 hunks)src/main/presenter/mcpPresenter/mcpClient.ts(15 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/mcp-session-recovery.md
[uncategorized] ~158-~158: 动词的修饰一般为‘形容词（副词）+地+动词’。您的意思是否是：新"地"连接
Context: ...用者知道需要重试 5. 重新调用: 上层调用者重新发起请求，此时会建立新的连接和session 6. 持续错误检测: 如果重启后再次出现sessi...
(wb4)
🪛 markdownlint-cli2 (0.17.2)
docs/mcp-session-recovery.md
7-7: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
175-175: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
181-181: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (11)
src/main/presenter/llmProviderPresenter/providers/geminiProvider.ts (1)
445-445: LGTM: Minor syntax improvement for consistency.The addition of parentheses around the arrow function parameter is a harmless style improvement that enhances code consistency.
docs/mcp-session-recovery.md (1)
1-241: Excellent comprehensive documentation.This documentation provides a thorough explanation of the session recovery mechanism, covering:
- Problem background and MCP specification compliance
- Clear solution overview with error detection and recovery workflow
- Detailed code examples and usage patterns
- Distinction between different disconnect methods
- Practical usage recommendations
The documentation effectively complements the implementation changes in the mcpClient.ts file.
src/main/presenter/mcpPresenter/mcpClient.ts (9)
47-51: Well-designed session error interface.The SessionError interface properly extends Error with optional HTTP status and session expiry fields, providing structured error information for session management.
53-84: Robust session error detection with pattern matching.The
isSessionErrorfunction implements comprehensive error detection with:
- High-confidence session-specific patterns (session expired, invalid session, etc.)
- HTTP error patterns with transport context validation
- Proper defensive programming with type guards
This approach ensures accurate identification of session-related errors while avoiding false positives.
100-103: Appropriate state management for recovery control.The addition of
isRecoveringandhasRestartedflags provides proper concurrency control and prevents infinite retry loops, which is essential for robust session management.
667-706: Well-implemented session error recovery logic.The
checkAndHandleSessionErrormethod provides:
- Proper concurrency control with
isRecoveringflag- Single restart attempt with complete state cleanup
- Service shutdown on persistent errors to prevent infinite loops
- Clear logging for debugging and monitoring
The recovery strategy is simple yet effective, following the principle of fail-fast when recovery attempts don't succeed.
708-716: Clean service shutdown implementation.The
stopServicemethod properly delegates to the internal disconnect method with a specific reason, maintaining consistency in cleanup behavior.
718-734: Unified disconnect logic with proper event notification.The
internalDisconnectmethod provides:
- Centralized cleanup logic used by both normal disconnect and forced stop
- Customizable logging based on the disconnect reason
- Proper system notification via event bus
- Consistent resource cleanup
This design eliminates code duplication and ensures consistent behavior across different disconnect scenarios.
626-627: Proper refactoring of disconnect method.The public
disconnectmethod now correctly delegates to the internal method, maintaining the same public API while leveraging the shared cleanup logic.
737-776: Consistent session error handling across all public methods.All public async methods (
callTool,listTools,listPrompts,getPrompt,listResources,readResource) now include:
- Automatic connection establishment when needed
- Session error detection and recovery via
checkAndHandleSessionError- Reset of
hasRestartedflag on successful operations- Proper error propagation to callers
This consistent implementation ensures that session errors are handled uniformly across the entire MCP client API, allowing callers to retry operations after automatic recovery.
Also applies to: 785-826, 835-889, 892-934, 943-990, 993-1028
754-755: Smart recovery state reset on successful operations.The reset of
hasRestartedflag after successful operations is a clever design choice that allows the service to attempt recovery again if session errors occur later, while still preventing infinite loops during persistent issues.Also applies to: 797-798, 848-849, 908-909, 956-957, 1007-1008
Pull Request Description (中文)
你的功能请求是否与某个问题有关？请描述一下。
修复 #534 问题
支持http协议在遇到400等错误的时候自动重启服务，多次重启失败后关闭服务
Summary by CodeRabbit