This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-mcp.git
The following commit(s) were added to refs/heads/main by this push:
new 83ab54b7 fix: keep Solr failure text out of MCP resource responses;
drop JsonUtils (#275)
83ab54b7 is described below
commit 83ab54b7a42cbe1de1c958b0e6f3fe6156da7253
Author: Aditya Parikh <[email protected]>
AuthorDate: Mon Oct 26 15:46:28 2026 -0400
fix: keep Solr failure text out of MCP resource responses; drop JsonUtils
(#275)
* refactor: drop JsonUtils and call ObjectMapper.writeValueAsString directly
JsonUtils.toJson existed to wrap Jackson 2's checked
JsonProcessingException. Jackson 3's JacksonException is unchecked, so the
helper now only swallows serialization failures into an error JSON body,
and every caller already holds an injected ObjectMapper.
Signed-off-by: Aditya Parikh <[email protected]>
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix: keep Solr failure text out of MCP resource responses
getSchemaResource returned the exception message in its error JSON, and
getCollectionsResource let SolrServerException, IOException and
JacksonException escape. Spring AI's resource callback reports the root
cause's message to the client, and Solr and SolrJ failure text carries the
internal Solr URL ("Error from server at <url>: ...").
Both resources now log the full failure and throw an IllegalStateException
with a fixed message, a next-step hint and no cause, following
SearchService. The schema resource no longer returns {"error": ...} as a
successful read, which a client cannot tell apart from a schema.
McpClientIntegrationTestBase reads solr://no_such_collection_xyz/schema
over HTTP and stdio and asserts the MCP error names the failure without
"Error from server" or a Solr URL.
Signed-off-by: Aditya Parikh <[email protected]>
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
---------
Signed-off-by: Aditya Parikh <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
---
.../mcp/server/collection/CollectionService.java | 21 ++++++--
.../solr/mcp/server/schema/SchemaService.java | 17 ++++---
.../org/apache/solr/mcp/server/util/JsonUtils.java | 59 ----------------------
.../mcp/server/McpClientIntegrationTestBase.java | 19 +++++++
.../server/collection/CollectionServiceTest.java | 23 +++++++++
.../solr/mcp/server/schema/SchemaServiceTest.java | 19 +++++++
6 files changed, 88 insertions(+), 70 deletions(-)
diff --git
a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
index 438839dd..fa5a62f0 100644
--- a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
+++ b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
@@ -19,7 +19,6 @@ package org.apache.solr.mcp.server.collection;
import static org.apache.solr.mcp.server.collection.CollectionUtils.getFloat;
import static org.apache.solr.mcp.server.collection.CollectionUtils.getInteger;
import static org.apache.solr.mcp.server.collection.CollectionUtils.getLong;
-import static org.apache.solr.mcp.server.util.JsonUtils.toJson;
import io.micrometer.observation.annotation.Observed;
import io.modelcontextprotocol.spec.McpSchema.CompleteRequest;
@@ -128,6 +127,10 @@ public class CollectionService {
private static final Logger logger =
LoggerFactory.getLogger(CollectionService.class);
+ static final String COLLECTIONS_RESOURCE_ERROR =
+ "Failed to list Solr collections. Retry later; if it persists,"
+ + " ask the operator to check Solr availability and the server
logs.";
+
// ========================================
// Constants for API Parameters and Paths
// ========================================
@@ -304,8 +307,8 @@ public class CollectionService {
* available in the Solr cluster. The returned JSON contains an array of
collection names.
*
* @return JSON string containing the list of collections
- * @throws SolrServerException if there are errors communicating with Solr
- * @throws IOException if there are I/O errors during communication
+ * @throws IllegalStateException with {@link #COLLECTIONS_RESOURCE_ERROR} if
the collections
+ * cannot be listed or serialized
*/
@PreAuthorize("isAuthenticated()")
@McpResource(
@@ -313,8 +316,16 @@ public class CollectionService {
name = "solr-collections",
description = "List of all Solr collections available in the cluster",
mimeType = "application/json")
- public String getCollectionsResource() throws SolrServerException,
IOException {
- return toJson(objectMapper, listCollections());
+ public String getCollectionsResource() {
+ try {
+ return objectMapper.writeValueAsString(listCollections());
+ } catch (Exception e) {
+ // Deliberately no cause: MCP reports the root cause's message to the
client,
+ // and Solr and SolrJ failure text can carry internal host names, URLs
and
+ // paths. The log is the sole record of the cause.
+ logger.error("Failed to list collections for the collections resource",
e);
+ throw new IllegalStateException(COLLECTIONS_RESOURCE_ERROR);
+ }
}
/** Maximum number of completion suggestions returned per request. */
diff --git a/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java
b/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java
index c7bb39d8..3480205b 100644
--- a/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java
+++ b/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java
@@ -16,7 +16,6 @@
*/
package org.apache.solr.mcp.server.schema;
-import static org.apache.solr.mcp.server.util.JsonUtils.toJson;
import static org.apache.solr.mcp.server.util.PromptText.optionalCodeBlock;
import io.micrometer.observation.annotation.Observed;
@@ -119,6 +118,10 @@ public class SchemaService {
private static final Logger logger =
LoggerFactory.getLogger(SchemaService.class);
+ static final String SCHEMA_RESOURCE_ERROR =
+ "Failed to retrieve the schema. Hint: call list-collections to check"
+ + " the collection name; if it persists, ask the operator to check
the server logs.";
+
/** SolrJ client for communicating with Solr server */
private final SolrClient solrClient;
@@ -149,6 +152,8 @@ public class SchemaService {
*
* @param collection the name of the collection to retrieve schema for
* @return JSON string containing the schema representation
+ * @throws IllegalStateException with {@link #SCHEMA_RESOURCE_ERROR} if the
schema cannot be
+ * retrieved or serialized
*/
@PreAuthorize("isAuthenticated()")
@McpResource(
@@ -159,13 +164,13 @@ public class SchemaService {
mimeType = "application/json")
public String getSchemaResource(String collection) {
try {
- return toJson(objectMapper, getSchema(collection));
+ return objectMapper.writeValueAsString(getSchema(collection));
} catch (Exception e) {
+ // Deliberately no cause: MCP reports the root cause's message to the
client,
+ // and Solr and SolrJ failure text can carry internal host names, URLs
and
+ // paths. The log is the sole record of the cause.
logger.error("Failed to get schema for collection: {}", collection, e);
- // Serialise via Jackson rather than concatenating: an exception message
- // containing a quote, backslash or newline would otherwise emit invalid
- // JSON to the MCP client.
- return toJson(objectMapper, Map.of("error",
String.valueOf(e.getMessage())));
+ throw new IllegalStateException(SCHEMA_RESOURCE_ERROR);
}
}
diff --git a/src/main/java/org/apache/solr/mcp/server/util/JsonUtils.java
b/src/main/java/org/apache/solr/mcp/server/util/JsonUtils.java
deleted file mode 100644
index 624e2cb2..00000000
--- a/src/main/java/org/apache/solr/mcp/server/util/JsonUtils.java
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.solr.mcp.server.util;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import tools.jackson.core.JacksonException;
-import tools.jackson.databind.ObjectMapper;
-
-/**
- * Utility class for JSON serialization operations.
- *
- * <p>Provides common JSON conversion methods used throughout the MCP server,
particularly for MCP
- * Resource responses that need to return JSON strings.
- *
- * @version 0.0.1
- * @since 0.0.1
- */
-public final class JsonUtils {
-
- private static final Logger logger =
LoggerFactory.getLogger(JsonUtils.class);
-
- private JsonUtils() {
- // Utility class - prevent instantiation
- }
-
- /**
- * Converts an object to its JSON string representation.
- *
- * <p>Used by MCP Resource methods that need to return serialized JSON
responses. On serialization
- * failure, returns an error JSON object.
- *
- * @param objectMapper the Jackson ObjectMapper for serialization
- * @param obj the object to serialize
- * @return JSON string representation, or error JSON on failure
- */
- public static String toJson(ObjectMapper objectMapper, Object obj) {
- try {
- return objectMapper.writeValueAsString(obj);
- } catch (JacksonException e) {
- logger.error("Failed to serialize response", e);
- return "{\"error\": \"Failed to serialize response\"}";
- }
- }
-}
diff --git
a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
index 7d153d2a..f4cbe19f 100644
--- a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
+++ b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
@@ -19,6 +19,7 @@ package org.apache.solr.mcp.server;
import static org.junit.jupiter.api.Assertions.*;
import io.modelcontextprotocol.client.McpSyncClient;
+import io.modelcontextprotocol.spec.McpError;
import io.modelcontextprotocol.spec.McpSchema.CallToolRequest;
import io.modelcontextprotocol.spec.McpSchema.CallToolResult;
import io.modelcontextprotocol.spec.McpSchema.CompleteRequest;
@@ -28,6 +29,7 @@ import
io.modelcontextprotocol.spec.McpSchema.GetPromptRequest;
import io.modelcontextprotocol.spec.McpSchema.GetPromptResult;
import io.modelcontextprotocol.spec.McpSchema.PromptMessage;
import io.modelcontextprotocol.spec.McpSchema.PromptReference;
+import io.modelcontextprotocol.spec.McpSchema.ReadResourceRequest;
import io.modelcontextprotocol.spec.McpSchema.ResourceReference;
import io.modelcontextprotocol.spec.McpSchema.TextContent;
import io.modelcontextprotocol.spec.McpSchema.Tool;
@@ -1227,6 +1229,23 @@ public abstract class McpClientIntegrationTestBase {
assertFalse(extractText(result).contains("Exception"),
extractText(result));
}
+ @Test
+ @Order(45)
+ void schemaResourceFailureIsAnMcpErrorWithoutTheSolrUrl() {
+ // An unknown collection makes SolrJ raise "Error from server at <solr
url>:
+ // ...".
+ McpError e =
+ assertThrows(
+ McpError.class,
+ () ->
+ mcpClient.readResource(
+ new
ReadResourceRequest("solr://no_such_collection_xyz/schema")));
+ String text = e.getMessage() + " " + e.getJsonRpcError();
+ assertTrue(text.contains("Failed to retrieve the schema"), text);
+ assertFalse(text.contains("Error from server"), text);
+ assertFalse(text.contains("http://";) || text.contains("https://";), text);
+ }
+
private static String extractFirstMessageText(GetPromptResult result) {
List<PromptMessage> messages = result.messages();
assertFalse(messages.isEmpty(), "messages must not be empty");
diff --git
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceTest.java
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceTest.java
index cbb22a1e..9c7f4176 100644
---
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceTest.java
@@ -747,6 +747,29 @@ class CollectionServiceTest {
assertThrows(IOException.class, () -> collectionService.listCollections());
}
+ @Test
+ void getCollectionsResource_Success() throws Exception {
+ NamedList<Object> response = new NamedList<>();
+ response.add("collections", Arrays.asList("collection1", "collection2"));
+ when(solrClient.request(any(), any())).thenReturn(response);
+
+ assertEquals("[\"collection1\",\"collection2\"]",
collectionService.getCollectionsResource());
+ }
+
+ @Test
+ void getCollectionsResource_SolrFailure_DoesNotLeakCause() throws Exception {
+ when(solrClient.request(any(), any()))
+ .thenThrow(
+ new SolrServerException(
+ "Server refused connection at:
http://solr-internal.corp:8983/solr";));
+
+ IllegalStateException e =
+ assertThrows(IllegalStateException.class, () ->
collectionService.getCollectionsResource());
+
+ assertEquals(CollectionService.COLLECTIONS_RESOURCE_ERROR, e.getMessage());
+ assertNull(e.getCause());
+ }
+
// Helper methods — mock the Solr Metrics API response format:
// response -> "metrics" -> "solr.core.<name>" -> "CACHE.searcher.xxx" /
// "HANDLER./xxx"
diff --git
a/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceTest.java
b/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceTest.java
index a4b5ba3e..dd59e4b5 100644
--- a/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/schema/SchemaServiceTest.java
@@ -119,6 +119,25 @@ class SchemaServiceTest {
});
}
+ @Test
+ void testGetSchemaResource_SolrFailure_DoesNotLeakCause() throws Exception {
+ // Given
+ final String collectionName = "test_collection";
+ when(solrClient.request(any(SchemaRequest.class), eq(collectionName)))
+ .thenThrow(
+ new SolrServerException(
+ "Server refused connection at:
http://solr-internal.corp:8983/solr";));
+
+ // When
+ IllegalStateException e =
+ assertThrows(
+ IllegalStateException.class, () ->
schemaService.getSchemaResource(collectionName));
+
+ // Then the client gets the fixed message, never the Solr failure text
+ assertEquals(SchemaService.SCHEMA_RESOURCE_ERROR, e.getMessage());
+ assertNull(e.getCause());
+ }
+
@Test
void testGetSchema_WithEmptyCollection() {
// Given an empty collection name
