Skip to content

feat(mcp-server): enhance SDK compatibility with reflection caching and error handling#6312

Merged
moremind merged 4 commits intomasterfrom
feat/mcp-server-sdk-upgrade
Apr 2, 2026
Merged

feat(mcp-server): enhance SDK compatibility with reflection caching and error handling#6312
moremind merged 4 commits intomasterfrom
feat/mcp-server-sdk-upgrade

Conversation

@Aias00
Copy link
Copy Markdown
Contributor

@Aias00 Aias00 commented Mar 31, 2026

Summary

  • 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

Changes

McpSessionHelper.java

  • Static field caching for reflection (exchangeField, sessionField)
  • Field resolution at class load time with logging
  • checkReflectionAvailability() with re-resolve fallback
  • isReflectionAvailable() for proactive checking
  • Clear error messages with SDK version info
  • SUPPORTED_SDK_VERSION constant for compatibility tracking

ShenyuToolCallback.java

  • Enhanced error messages including SDK version information
  • Better error context for debugging reflection failures

pom.xml

  • Added SDK compatibility documentation comments
  • Documented reflection usage on internal SDK fields
  • Cleaned up commented-out dependencies

Documentation (MCP_TOOL_EXAMPLES.md, MCP_TOOL_EXAMPLES_EN.md)

  • Added SDK Version Compatibility section
  • Documented supported protocols (SSE, Streamable HTTP)
  • Documented known limitations (reflection, timeout, CORS)

Test Results

Tests run: 76, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

SDK Compatibility

Dependency Version
MCP SDK 0.17.0
Spring AI 1.1.2
Spring Boot 3.3.1

🤖 Generated with Claude Code

…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>
Copilot AI review requested due to automatic review settings March 31, 2026 05:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 McpSessionHelper and expose a supported SDK version constant.
  • Improve session-id extraction error reporting in ShenyuToolCallback by 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;
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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;

Copilot uses AI. Check for mistakes.
Comment on lines 144 to 148
public static String getSessionId(final McpSyncServerExchange mcpSyncServerExchange) {
McpServerSession session = getSession(mcpSyncServerExchange);
if (Objects.isNull(session)) {
throw new IllegalArgumentException("Session is required in McpAsyncServerExchange");
}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +775 to +780
// 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);
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
// 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;

Copilot uses AI. Check for mistakes.
Comment on lines +774 to +775
} catch (IllegalStateException e) {
// Re-throw SDK compatibility errors with additional context
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
} 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)

Copilot uses AI. Check for mistakes.
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
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
ShenyuStreamableHttpServerTransportProvider.SUPPORTED_SDK_VERSION accordingly
any related version constants in ShenyuStreamableHttpServerTransportProvider accordingly

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +18
- **SSE (Server-Sent Events)**: `/sse` endpoint with long-polling session support
- **Streamable HTTP**: Unified endpoint supporting GET (SSE stream) and POST (message) requests
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
- **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

Copilot uses AI. Check for mistakes.
Comment on lines 767 to 781
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);
}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@moremind moremind added this to the 2.7.1 milestone Apr 2, 2026
@moremind moremind added the mcp label Apr 2, 2026
@moremind moremind merged commit 2c44e7e into master Apr 2, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants