modelcontextprotocol / modelcontextprotocol/java-sdk

Add null-safety validation and safe fallbacks for features / clientCapabilities in McpAsyncClient constructor and feature branches

Ouverte
#634 0 commentaires 1 réaction 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

enhancement P3
Langage dominant
Java
Étoiles
3.7k
Forks
1.1k
Merge moyen
1 j 15 h
PR mergées (30 j)
9

Description

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., IllegalArgumentException or McpError) instead of hidden NPEs.
  • Handle optional fields gracefully by using default values or null-safe checks, improving SDK robustness and developer experience.

Proposed Solution

  1. 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.
  2. Null-Safe Fallbacks

    • Initialize roots safely:
      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.
  3. Improve Error Messages

    • Use consistent, developer-friendly error messages like:
      • "Features must not be null"
      • "ClientCapabilities is required when enabling sampling/elicitation"

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; roots initialized 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 McpError as before, readable message.

Impacted Areas

  • io.modelcontextprotocol.client.McpAsyncClient constructor
  • 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 main branch
  • JDK: 17 or 21
  • OS: macOS / Linux / Windows

If maintainers agree, I’d be happy to open a PR implementing this improvement.

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Commencez dans le constructeur de io.modelcontextprotocol.client.McpAsyncClient et inspectez chaque branche qui lit clientCapabilities, roots, sampling ou elicitation. Ajoutez des tests du constructeur et des branches de fonctionnalités pour les fonctionnalités null et les champs optionnels manquants, en suivant les recommandations de l'issue. Le travail est considéré comme terminé lorsque les échecs produisent une validation claire ou un comportement de repli sûr, sans NullPointerExceptions masquées, tandis que le comportement existant de McpError reste inchangé.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
java
Domaine
api, backend
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
À l'abandon
Clarté
Plutôt claire
Accessibilité débutants
35/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.