feat(mcp-server): enhance SDK compatibility with reflection caching and error handling#6312
feat(mcp-server): enhance SDK compatibility with reflection caching and error handling#6312
Conversation
…nd error handling - Add reflection field caching in McpSessionHelper for better performance and reliability - Add SUPPORTED_SDK_VERSION constant for SDK compatibility tracking - Enhance error messages with SDK version information in ShenyuToolCallback - Add SDK compatibility notes in pom.xml documenting reflection usage - Update MCP_TOOL_EXAMPLES.md and MCP_TOOL_EXAMPLES_EN.md with SDK version compatibility table This change improves compatibility with MCP SDK 0.17.0 by: - Caching reflection fields at class load time - Adding graceful degradation when reflection fails - Documenting known limitations and supported SDK versions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves MCP server plugin robustness and diagnosability when interacting with MCP SDK internals by adding reflection field caching and surfacing SDK compatibility context in errors and documentation.
Changes:
- Cache and pre-resolve reflective access to MCP SDK internal fields in
McpSessionHelperand expose a supported SDK version constant. - Improve session-id extraction error reporting in
ShenyuToolCallbackby including SDK version context. - Add/refresh SDK compatibility notes in module POM and user-facing MCP tool examples docs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apache/shenyu/plugin/mcp/server/session/McpSessionHelper.java |
Adds reflection field caching, availability checks, and a supported SDK version constant for clearer compatibility failures. |
shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apache/shenyu/plugin/mcp/server/callback/ShenyuToolCallback.java |
Wraps session-id extraction failures with additional SDK compatibility context. |
shenyu-plugin/shenyu-plugin-mcp-server/pom.xml |
Documents reflection-based coupling to MCP SDK internal fields and version expectations. |
shenyu-plugin/shenyu-plugin-mcp-server/MCP_TOOL_EXAMPLES.md |
Adds a compatibility section describing tested SDK versions and limitations (CN). |
shenyu-plugin/shenyu-plugin-mcp-server/MCP_TOOL_EXAMPLES_EN.md |
Adds a compatibility section describing tested SDK versions and limitations (EN). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (SecurityException e) { | ||
| LOG.error("SDK COMPATIBILITY ERROR: Security manager blocked reflection access. " | ||
| + "Field resolution failed: {}", e.getMessage()); | ||
| fieldsResolved = false; |
There was a problem hiding this comment.
resolveReflectionFields() runs during class initialization and calls Field#setAccessible(true). On Java 9+ this can throw unchecked runtime exceptions (e.g., InaccessibleObjectException) in addition to SecurityException; currently those will escape the static initializer and can fail class loading with ExceptionInInitializerError. Consider catching RuntimeException around the reflective access, logging, and leaving fieldsResolved=false so the helper fails gracefully via checkReflectionAvailability() instead of crashing at class-load time.
| fieldsResolved = false; | |
| fieldsResolved = false; | |
| } catch (RuntimeException e) { | |
| LOG.error("SDK COMPATIBILITY ERROR: Unexpected runtime exception during reflection field resolution. " | |
| + "This may indicate module access restrictions or an incompatible SDK version (tested: {}). " | |
| + "Error: {}", SUPPORTED_SDK_VERSION, e.getMessage(), e); | |
| fieldsResolved = false; |
| public static String getSessionId(final McpSyncServerExchange mcpSyncServerExchange) { | ||
| McpServerSession session = getSession(mcpSyncServerExchange); | ||
| if (Objects.isNull(session)) { | ||
| throw new IllegalArgumentException("Session is required in McpAsyncServerExchange"); | ||
| } |
There was a problem hiding this comment.
getSessionId() checks Objects.isNull(session) after calling getSession(), but getSession() never returns null (it either returns a non-null session or throws). This branch is effectively dead code and can be removed to avoid misleading future readers about possible return values.
| // Re-throw SDK compatibility errors with additional context | ||
| throw new IllegalStateException( | ||
| "Failed to extract session ID from MCP exchange. " | ||
| + "This may indicate an SDK compatibility issue. " | ||
| + "Tested SDK version: " + McpSessionHelper.getSupportedSdkVersion() + ". " | ||
| + "Original error: " + e.getMessage(), e); |
There was a problem hiding this comment.
The try block throws an IllegalStateException when sessionId is empty, but the catch (IllegalStateException e) immediately wraps it into a new exception that suggests an SDK compatibility issue. This makes non-SDK failures (like an empty/missing session) look like compatibility problems; consider only wrapping exceptions that originate from McpSessionHelper reflection failures (e.g., messages starting with "SDK COMPATIBILITY ERROR"), and rethrow other IllegalStateExceptions unchanged.
| // Re-throw SDK compatibility errors with additional context | |
| throw new IllegalStateException( | |
| "Failed to extract session ID from MCP exchange. " | |
| + "This may indicate an SDK compatibility issue. " | |
| + "Tested SDK version: " + McpSessionHelper.getSupportedSdkVersion() + ". " | |
| + "Original error: " + e.getMessage(), e); | |
| // Only wrap SDK compatibility-related errors; rethrow others unchanged | |
| final String originalMessage = e.getMessage(); | |
| if (originalMessage != null && originalMessage.startsWith("SDK COMPATIBILITY ERROR")) { | |
| throw new IllegalStateException( | |
| "Failed to extract session ID from MCP exchange. " | |
| + "This may indicate an SDK compatibility issue. " | |
| + "Tested SDK version: " + McpSessionHelper.getSupportedSdkVersion() + ". " | |
| + "Original error: " + originalMessage, e); | |
| } | |
| throw e; |
| } catch (IllegalStateException e) { | ||
| // Re-throw SDK compatibility errors with additional context |
There was a problem hiding this comment.
extractSessionId() only catches IllegalStateException, but McpSessionHelper.getSessionId() can also throw IllegalArgumentException (e.g., missing async exchange/session). That will currently propagate without the added context and contradict the method’s Javadoc (@throws IllegalStateException). Consider catching/wrapping the helper’s IllegalArgumentException as well (or changing the helper to throw IllegalStateException consistently for extraction failures).
| } catch (IllegalStateException e) { | |
| // Re-throw SDK compatibility errors with additional context | |
| } catch (IllegalStateException | IllegalArgumentException e) { | |
| // Re-throw errors with additional context (including potential SDK compatibility issues) |
| McpServerSession.initialized and McpServerSession.state fields for session verification | ||
| - When upgrading SDK versions, verify that these internal fields still exist and | ||
| update McpSessionHelper.SUPPORTED_SDK_VERSION and | ||
| ShenyuStreamableHttpServerTransportProvider.SUPPORTED_SDK_VERSION accordingly |
There was a problem hiding this comment.
The comment says to update ShenyuStreamableHttpServerTransportProvider.SUPPORTED_SDK_VERSION, but that constant doesn’t exist in ShenyuStreamableHttpServerTransportProvider (the class currently has no SUPPORTED_SDK_VERSION). Either add the constant (if intended) or update the comment to reference the actual version source to avoid misleading future upgrades.
| ShenyuStreamableHttpServerTransportProvider.SUPPORTED_SDK_VERSION accordingly | |
| any related version constants in ShenyuStreamableHttpServerTransportProvider accordingly |
| - **SSE (Server-Sent Events)**: `/sse` endpoint with long-polling session support | ||
| - **Streamable HTTP**: Unified endpoint supporting GET (SSE stream) and POST (message) requests |
There was a problem hiding this comment.
SSE is a streaming (server-push) mechanism, not long-polling. The description "/sse endpoint with long-polling session support" is inaccurate; consider changing it to describe a long-lived streaming connection (and, if relevant, how session IDs are managed).
| - **SSE (Server-Sent Events)**: `/sse` endpoint with long-polling session support | |
| - **Streamable HTTP**: Unified endpoint supporting GET (SSE stream) and POST (message) requests | |
| - **SSE (Server-Sent Events)**: `/sse` endpoint providing a long-lived, one-way streaming (server-push) connection, with optional session ID–based correlation | |
| - **Streamable HTTP**: Unified endpoint where GET establishes an SSE stream and POST sends messages correlated to that stream |
| try { | ||
| sessionId = McpSessionHelper.getSessionId(mcpExchange); | ||
| } catch (NoSuchFieldException | IllegalAccessException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| if (StringUtils.hasText(sessionId)) { | ||
| LOG.debug("Extracted session ID: {}", sessionId); | ||
| return sessionId; | ||
| final String sessionId = McpSessionHelper.getSessionId(mcpExchange); | ||
| if (StringUtils.hasText(sessionId)) { | ||
| LOG.debug("Extracted session ID: {}", sessionId); | ||
| return sessionId; | ||
| } | ||
| throw new IllegalStateException("Session ID is empty – it should have been set earlier by handleMessageEndpoint"); | ||
| } catch (IllegalStateException e) { | ||
| // Re-throw SDK compatibility errors with additional context | ||
| throw new IllegalStateException( | ||
| "Failed to extract session ID from MCP exchange. " | ||
| + "This may indicate an SDK compatibility issue. " | ||
| + "Tested SDK version: " + McpSessionHelper.getSupportedSdkVersion() + ". " | ||
| + "Original error: " + e.getMessage(), e); | ||
| } |
There was a problem hiding this comment.
There are existing unit tests for ShenyuToolCallback, but none cover the new error-wrapping behavior that adds SDK version context. Consider adding a test where McpSessionHelper.getSessionId() throws an IllegalStateException (reflection failure) and asserting the propagated exception message includes the supported SDK version, and another where sessionId is empty to ensure it is not misclassified as a compatibility issue.
Summary
McpSessionHelperfor better performance and reliabilitySUPPORTED_SDK_VERSIONconstant for SDK compatibility trackingShenyuToolCallbackpom.xmldocumenting reflection usageMCP_TOOL_EXAMPLES.mdandMCP_TOOL_EXAMPLES_EN.mdwith SDK version compatibility tableChanges
McpSessionHelper.java
exchangeField,sessionField)checkReflectionAvailability()with re-resolve fallbackisReflectionAvailable()for proactive checkingSUPPORTED_SDK_VERSIONconstant for compatibility trackingShenyuToolCallback.java
pom.xml
Documentation (MCP_TOOL_EXAMPLES.md, MCP_TOOL_EXAMPLES_EN.md)
Test Results
SDK Compatibility
🤖 Generated with Claude Code