modelcontextprotocol / modelcontextprotocol/java-sdk
Add null-safety validation and safe fallbacks for features / clientCapabilities in McpAsyncClient constructor and feature branches
まだ誰も着手していません。
- 主要言語
- Java
- スター
- 3.7k
- フォーク
- 1.1k
- 平均マージ
- 1日 15時間
- マージ済み PR(30日)
- 9
説明
Add null-safety validation and safe fallbacks for features / clientCapabilities in McpAsyncClient constructor and feature branches
Background
The current io.modelcontextprotocol.client.McpAsyncClient constructor performs null checks for transport, requestTimeout, and initializationTimeout, but does not validate the features object itself or its derived fields such as features.clientCapabilities() and features.roots().
Throughout the class, several branches directly dereference clientCapabilities (and its nested capabilities such as roots(), sampling(), and elicitation()), which can lead to NullPointerException with limited diagnostic information.
Example excerpt:
McpAsyncClient(McpClientTransport transport, Duration requestTimeout, Duration initializationTimeout,
McpClientFeatures.Async features) {
Assert.notNull(transport, "Transport must not be null");
Assert.notNull(requestTimeout, "Request timeout must not be null");
Assert.notNull(initializationTimeout, "Initialization timeout must not be null");
this.clientInfo = features.clientInfo(); // direct dereference
this.clientCapabilities = features.clientCapabilities(); // direct dereference
this.roots = new ConcurrentHashMap<>(features.roots()); // may NPE
if (this.clientCapabilities.roots() != null) { ... } // clientCapabilities could be null
if (this.clientCapabilities.sampling() != null) { ... }
if (this.clientCapabilities.elicitation() != null) { ... }
}
Reproduction Example
// features may be constructed dynamically, with some fields unset or null
McpAsyncClient client = new McpAsyncClient(
transport,
Duration.ofSeconds(5),
Duration.ofSeconds(10),
new McpClientFeatures.Async(/* some fields return null */)
);
// During initialization or feature checks, a NullPointerException occurs
Expected Behavior
- Perform explicit validation or safe fallback initialization during construction, providing clear and actionable error messages (e.g.,
IllegalArgumentExceptionorMcpError) instead of hidden NPEs. - Handle optional fields gracefully by using default values or null-safe checks, improving SDK robustness and developer experience.
Proposed Solution
-
Constructor Validation
- Add:
Assert.notNull(features, "Features must not be null"); - Depending on design intent:
- Strict mode:
Assert.notNull(features.clientCapabilities(), "ClientCapabilities must not be null"); - Lenient mode: allow
clientCapabilities == null, handle null safely later.
- Strict mode:
- Add:
-
Null-Safe Fallbacks
- Initialize
rootssafely:Map<String, Root> initialRoots = (features.roots() != null) ? features.roots() : Collections.emptyMap(); this.roots = new ConcurrentHashMap<>(initialRoots); - Add double null checks in all feature branches:
if (this.clientCapabilities != null && this.clientCapabilities.roots() != null) { ... } if (this.clientCapabilities != null && this.clientCapabilities.sampling() != null) { ... } if (this.clientCapabilities != null && this.clientCapabilities.elicitation() != null) { ... } - Retain current behavior for declared but unhandled capabilities (still throwing
McpError), but ensure the branch is entered safely.
- Initialize
-
Improve Error Messages
- Use consistent, developer-friendly error messages like:
"Features must not be null""ClientCapabilities is required when enabling sampling/elicitation"
- Use consistent, developer-friendly error messages like:
Compatibility
- No API changes — only internal validation and null-safety improvements.
- If “strict mode” is chosen (enforcing non-null
clientCapabilities), note this in release notes; otherwise, lenient handling remains fully backward compatible.
Test Recommendations
- Constructor tests
features == null→ expect meaningful exception (not NPE).features.roots() == null→ no NPE;rootsinitialized as empty map.features.clientCapabilities() == null→ no NPE during construction or branching.
- Feature branches
- Missing
roots/sampling/elicitation→ branches skipped safely, no NPE. - Declared capability but missing handler → same
McpErroras before, readable message.
- Missing
Impacted Areas
io.modelcontextprotocol.client.McpAsyncClientconstructor- Branches based on
clientCapabilities
Benefits
- Eliminates hidden NullPointerExceptions during initialization.
- Improves error traceability and developer experience.
- Small, low-risk change; easy to review and merge.
Environment (example)
- MCP Java SDK: current
mainbranch - JDK: 17 or 21
- OS: macOS / Linux / Windows
If maintainers agree, I’d be happy to open a PR implementing this improvement.
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
調査の方向性
io.modelcontextprotocol.client.McpAsyncClient コンストラクターから始め、clientCapabilities、roots、sampling、または elicitation を読み取るすべての分岐を調査します。issue の推奨事項に従い、null features と欠落しているオプションフィールドに対するコンストラクターおよび機能分岐のテストを追加します。完了条件は、既存の McpError の動作を変更せず、失敗時に隠れた NullPointerExceptions を発生させることなく、明確な検証または安全なフォールバック動作が提供されることです。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- java
- 領域
- api, backend
- issue の種類
- バグ
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 活発さ
- 停滞
- 明瞭さ
- おおむね明確
- 初心者へのやさしさ
- 35/100