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 1808de68 fix(search): tolerate blank optional arguments and return
safe errors (#195)
1808de68 is described below
commit 1808de68e6adcc4ba0dc6f395662f7d47c7bca18
Author: Aditya Parikh <[email protected]>
AuthorDate: Tue Oct 6 14:46:44 2026 -0400
fix(search): tolerate blank optional arguments and return safe errors (#195)
* fix(search): tolerate blank optional arguments and return safe errors
Signed-off-by: Aditya Parikh <[email protected]>
Co-authored-by: Junie <[email protected]>
* test(search): cover MCP regressions over HTTP and STDIO
Signed-off-by: Aditya Parikh <[email protected]>
Co-authored-by: Junie <[email protected]>
* fix(search): log backend failures at WARN and drop an off-topic test
The client only sees the generic message, so the log is the only record of
the cause; DEBUG hid it in production. The schemaless-facet integration test
exercised Solr's faceting rather than this change and was the sole reason
for the untyped platform fixture field.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CiUHyyXLTo9ATdgg8eRFZJ
Signed-off-by: Aditya Parikh <[email protected]>
* test(search): filter on the typed platform field
The untyped platform fixture field went away with the schemaless-facet test,
so the blank-options test now filters on platform_ss.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CiUHyyXLTo9ATdgg8eRFZJ
Signed-off-by: Aditya Parikh <[email protected]>
---------
Signed-off-by: Aditya Parikh <[email protected]>
Co-authored-by: Junie <[email protected]>
Co-authored-by: Claude Fable 5.1 <[email protected]>
---
.../solr/mcp/server/search/SearchService.java | 107 +++++++++++------
.../mcp/server/McpClientIntegrationTestBase.java | 26 +++++
.../search/SearchServiceIntegrationTest.java | 30 ++++-
.../solr/mcp/server/search/SearchServiceTest.java | 129 ++++++++++++++++++++-
4 files changed, 249 insertions(+), 43 deletions(-)
diff --git a/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
b/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
index ac1b2f37..356209b6 100644
--- a/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
+++ b/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
@@ -142,6 +142,12 @@ public class SearchService {
/** Remediation hint naming the {@code list-collections} tool. */
static final String LIST_COLLECTIONS_HINT = ". Hint: call
list-collections to see available collections.";
+ private static final String CONNECTION_ERROR = "Search failed: unable
to communicate with Solr. Retry later;"
+ + " if it persists, ask the operator to check Solr
availability and connection settings.";
+ private static final String RESPONSE_ERROR = "Search failed: unable to
process the Solr response."
+ + " If facetFields were supplied, call get-schema to
check those fields or retry without facetFields;"
+ + " if it persists, ask the operator to check server
logs and Solr compatibility.";
+
private final SolrClient solrClient;
/**
@@ -192,12 +198,13 @@ public class SearchService {
* The Solr query string (q parameter). Defaults to "*:*" if
not
* specified
* @param filterQueries
- * List of filter queries (fq parameter)
+ * List of filter queries (fq parameter); blank entries are
ignored
* @param facetFields
- * List of fields to facet on
+ * List of fields to facet on; blank entries are ignored
* @param sortClauses
* List of sort clauses for ordering results; each names a
field and
- * an optional {@code asc}/{@code desc} order (default
{@code asc})
+ * an optional {@code asc}/{@code desc} order (default
{@code asc});
+ * entries with blank fields are ignored
* @param start
* Starting offset for pagination
* @param rows
@@ -250,12 +257,14 @@ public class SearchService {
+ " {!edismax qf='name
author'}. If none specified defaults to \"*:*\"",
required = false) @Nullable String
query,
@McpToolParam(
- description = "Solr fq parameter: list
of filter queries, one filter per entry",
+ description = "Solr fq parameter: list
of filter queries, one filter per entry. Blank entries are ignored",
required = false) @Nullable
List<String> filterQueries,
- @McpToolParam(description = "Solr facet fields",
required = false) @Nullable List<String> facetFields,
+ @McpToolParam(
+ description = "Solr facet fields. Blank
entries are ignored",
+ required = false) @Nullable
List<String> facetFields,
@McpToolParam(
description = "Sort clauses applied in
order. Each has 'field' (field name to sort"
- + " on) and 'order'
('asc' or 'desc', default 'asc')",
+ + " on) and 'order'
('asc' or 'desc', default 'asc'). Entries with blank fields are ignored",
required = false) @Nullable
List<SortClause> sortClauses,
@McpToolParam(description = "Starting offset for
pagination", required = false) @Nullable Integer start,
@McpToolParam(description = "Number of rows to return",
required = false) @Nullable Integer rows)
@@ -272,20 +281,23 @@ public class SearchService {
// filter queries
if (!CollectionUtils.isEmpty(filterQueries)) {
- solrQuery.setFilterQueries(filterQueries.toArray(new
String[0]));
+
filterQueries.stream().filter(StringUtils::hasText).forEach(solrQuery::addFilterQuery);
}
// facets
if (!CollectionUtils.isEmpty(facetFields)) {
- solrQuery.setFacet(true);
- solrQuery.addFacetField(facetFields.toArray(new
String[0]));
+
facetFields.stream().filter(StringUtils::hasText).forEach(solrQuery::addFacetField);
+ }
+ // addFacetField sets facet=true; getFacetFields() is null
until the first one
+ if (solrQuery.getFacetFields() != null) {
solrQuery.setFacetMinCount(1);
solrQuery.setFacetSort(FacetParams.FACET_SORT_COUNT);
}
// sorting
if (!CollectionUtils.isEmpty(sortClauses)) {
-
solrQuery.setSorts(sortClauses.stream().map(SortClause::toSolrSortClause).toList());
+ sortClauses.stream().filter(clause -> clause != null &&
StringUtils.hasText(clause.field()))
+
.map(SortClause::toSolrSortClause).forEach(solrQuery::addSort);
}
// pagination
@@ -297,43 +309,51 @@ public class SearchService {
solrQuery.setRows(rows);
}
- final QueryResponse queryResponse;
try {
- queryResponse = solrClient.query(collection, solrQuery);
- } catch (SolrException e) {
- throw withRemediationHint(e, collection);
- }
+ final QueryResponse queryResponse =
solrClient.query(collection, solrQuery);
- // Add documents
- final SolrDocumentList documents = queryResponse.getResults();
+ // Add documents
+ final SolrDocumentList documents =
queryResponse.getResults();
- // Convert SolrDocuments to Maps
- // SolrDocument is a Map in Solr's field order, so no
per-document copy
- final List<Map<String, Object>> docs =
Collections.unmodifiableList(documents);
+ // Convert SolrDocuments to Maps
+ // SolrDocument is a Map in Solr's field order, so no
per-document copy
+ final List<Map<String, Object>> docs =
Collections.unmodifiableList(documents);
- // Add facets if present
- final var facets = getFacets(queryResponse);
+ // Add facets if present
+ final var facets = getFacets(queryResponse);
- return new SearchResponse(documents.getNumFound(),
documents.getStart(), documents.getMaxScore(), docs, facets);
+ return new SearchResponse(documents.getNumFound(),
documents.getStart(), documents.getMaxScore(), docs,
+ facets);
+ } catch (SolrException e) {
+ throw withRemediationHint(e, collection);
+ } catch (SolrServerException e) {
+ logger.warn("Solr query failed on collection {}",
collection, e);
+ throw new SolrServerException(CONNECTION_ERROR);
+ } catch (IOException e) {
+ logger.warn("Solr query failed on collection {}",
collection, e);
+ throw new IOException(CONNECTION_ERROR);
+ } catch (RuntimeException e) {
+ // The client only sees RESPONSE_ERROR; the log is the
sole record of the cause.
+ logger.warn("Solr query response failed on collection
{}", collection, e);
+ throw new IllegalStateException(RESPONSE_ERROR);
+ }
}
/**
- * Wraps common Solr query failures with a next-step hint. MCP clients
receive
- * the exception message as the tool error, so naming the follow-up
tool lets
- * them self-correct instead of retrying blind.
+ * Wraps common Solr query failures with a next-step hint. MCP unwraps
exception
+ * causes, so client-facing exceptions must omit the cause to preserve
safe
+ * guidance; the original failure is logged for server-side diagnostics.
*
* @param e
* the Solr exception raised by the query
* @param collection
* the collection that was queried
- * @return an exception carrying the original message plus a
remediation hint,
- * or the original exception when no hint applies
+ * @return an exception carrying safe guidance without a cause
*/
private static RuntimeException withRemediationHint(SolrException e,
String collection) {
final String message = String.valueOf(e.getMessage());
- // The MCP client only ever sees the exception message, so
without this the
- // server keeps no record of a failed query.
+ // Keep diagnostics in server logs, not in the client-facing
exception chain.
logger.debug("Solr query failed on collection {}", collection,
e);
// An unknown collection is a 404 whose body is Solr's HTML
"not found" page,
@@ -341,19 +361,34 @@ public class SearchService {
// The status code is the only signal that survives; match it
rather than the
// message text, which mentions neither the collection nor
"404".
if (e.code() == SolrException.ErrorCode.NOT_FOUND.code) {
- return new IllegalArgumentException(message +
LIST_COLLECTIONS_HINT, e);
+ return new IllegalArgumentException("Search failed: the
collection was not found" + LIST_COLLECTIONS_HINT);
}
// Everything below is a 400 carrying a generic SolrException,
indistinguishable
// except by Solr's message text.
final String lower = message.toLowerCase(Locale.ROOT);
- if (lower.contains(UNDEFINED_FIELD_TOKEN) ||
lower.contains(SORT_FIELD_NOT_FOUND_TOKEN)) {
- return new IllegalArgumentException(message +
GET_SCHEMA_HINT_FORMAT.formatted(collection), e);
+ if (e.code() == SolrException.ErrorCode.BAD_REQUEST.code) {
+ if (lower.contains(UNDEFINED_FIELD_TOKEN) ||
lower.contains(SORT_FIELD_NOT_FOUND_TOKEN)) {
+ return new IllegalArgumentException("Search
failed: a referenced field is not defined"
+ +
GET_SCHEMA_HINT_FORMAT.formatted(collection));
+ }
+ if (lower.contains(SYNTAX_ERROR_TOKEN) ||
lower.contains(CANNOT_PARSE_TOKEN)) {
+ return new IllegalArgumentException(
+ "Search failed: Solr could not
parse the query or filters" + LUCENE_SYNTAX_HINT);
+ }
+ return new
SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "Search failed: Solr rejected the
request. Check query syntax, filter queries, facet fields,"
+ + " sort clauses, and
pagination; call get-schema to verify the fields.");
}
- if (lower.contains(SYNTAX_ERROR_TOKEN) ||
lower.contains(CANNOT_PARSE_TOKEN)) {
- return new IllegalArgumentException(message +
LUCENE_SYNTAX_HINT, e);
+ if (e.code() == SolrException.ErrorCode.UNAUTHORIZED.code
+ || e.code() ==
SolrException.ErrorCode.FORBIDDEN.code) {
+ return new
SolrException(SolrException.ErrorCode.getErrorCode(e.code()),
+ "Search failed: Solr denied access. Ask
the operator to check the configured Solr credentials"
+ + " and collection
permissions.");
}
- return e;
+ return new
SolrException(SolrException.ErrorCode.getErrorCode(e.code()),
+ "Search failed: Solr could not complete the
request. Retry later; if it persists,"
+ + " ask the operator to check
Solr health and server logs.");
}
/**
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 0a2a4c8d..f50e4616 100644
--- a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
+++ b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java
@@ -828,6 +828,32 @@ public abstract class McpClientIntegrationTestBase {
"view-schema prompt completion should resolve
the created collection: " + result.completion().values());
}
+ @Test
+ @Order(39)
+ void blankOptionalSearchEntriesAreIgnoredThroughMcp() throws Exception {
+ var result = mcpClient.callTool(new CallToolRequest("search",
+ Map.of("collection", SHOWS_COLLECTION, "query",
"*:*", "rows", 0, "filterQueries", List.of(" "),
+ "facetFields", List.of("",
"platform"), "sortClauses",
+ List.of(Map.of("field", "",
"order", "")))));
+ assertNotError(result);
+ Map<String, Object> response =
OBJECT_MAPPER.readValue(extractText(result), new TypeReference<>() {
+ });
+ assertEquals(SHOWS_DOC_COUNT, getNumFound(response));
+ Map<?, ?> facets = (Map<?, ?>) response.get("facets");
+ Map<?, ?> platforms = (Map<?, ?>) facets.get("platform");
+ assertEquals(20, ((Number)
platforms.get("Netflix")).intValue());
+ }
+
+ @Test
+ @Order(40)
+ void searchFailureIsAnActionableMcpToolError() {
+ var result = mcpClient.callTool(new CallToolRequest("search",
+ Map.of("collection", SHOWS_COLLECTION,
"facetFields", List.of("nonexistent_field_xyz"))));
+ assertEquals(Boolean.TRUE, result.isError());
+ assertTrue(extractText(result).contains("get-schema"),
extractText(result));
+ assertFalse(extractText(result).contains("Exception"),
extractText(result));
+ }
+
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/search/SearchServiceIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
index 55c1aabf..3493113e 100644
---
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
@@ -70,6 +70,7 @@ class SearchServiceIntegrationTest {
[
{
"id": "book001",
+ "platform_ss": ["netflix"],
"name": ["A Game of Thrones"],
"author_ss": ["George R.R. Martin"],
"price": [7.99],
@@ -80,6 +81,7 @@ class SearchServiceIntegrationTest {
},
{
"id": "book002",
+ "platform_ss": ["netflix"],
"name": ["A Clash of Kings"],
"author_ss": ["George R.R. Martin"],
"price": [8.99],
@@ -90,6 +92,7 @@ class SearchServiceIntegrationTest {
},
{
"id": "book003",
+ "platform_ss": ["hulu"],
"name": ["A Storm of Swords"],
"author_ss": ["George R.R. Martin"],
"price": [9.99],
@@ -211,6 +214,15 @@ class SearchServiceIntegrationTest {
() -> "expected no facet buckets, got: " +
result.facets().get("genre_s"));
}
+ @Test
+ void searchWithBlankOptionsPreservesFiltersFacetsAndSort() throws
Exception {
+ SearchResponse result = searchService.search(COLLECTION_NAME, "
\t", List.of("", "platform_ss:netflix", " "),
+ List.of(" ", "platform_ss", ""), List.of(new
SortClause("", ""), new SortClause("id", "desc")), 0, 10);
+ assertEquals(2, result.numFound());
+ assertEquals(List.of("book002", "book001"),
getDocumentIds(result.documents()));
+ assertEquals(Map.of("netflix", 2L),
result.facets().get("platform_ss"));
+ }
+
/**
* Remediation hints classify Solr's error text, which this server
cannot see at
* compile time — the strings are produced by solr-core, and only
solr-solrj is
@@ -229,6 +241,7 @@ class SearchServiceIntegrationTest {
.search(COLLECTION_NAME,
"definitely_not_a_field:value", null, null, null, null, null));
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted(COLLECTION_NAME)),
() -> "expected get-schema hint, got: " +
e.getMessage());
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
@Test
@@ -237,6 +250,7 @@ class SearchServiceIntegrationTest {
.search(COLLECTION_NAME, "*:*",
List.of("definitely_not_a_field:value"), null, null, null, null));
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted(COLLECTION_NAME)),
() -> "expected get-schema hint, got: " +
e.getMessage());
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
/** Faceting words it differently: {@code undefined field: "name"}. */
@@ -246,6 +260,7 @@ class SearchServiceIntegrationTest {
.search(COLLECTION_NAME, "*:*", null,
List.of("definitely_not_a_field"), null, null, null));
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted(COLLECTION_NAME)),
() -> "expected get-schema hint, got: " +
e.getMessage());
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
/**
@@ -259,6 +274,7 @@ class SearchServiceIntegrationTest {
() -> searchService.search(COLLECTION_NAME,
"*:*", null, null, sort, null, null));
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted(COLLECTION_NAME)),
() -> "expected get-schema hint, got: " +
e.getMessage());
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
@Test
@@ -267,6 +283,7 @@ class SearchServiceIntegrationTest {
() -> searchService.search(COLLECTION_NAME,
"name:(", null, null, null, null, null));
assertTrue(e.getMessage().contains(SearchService.LUCENE_SYNTAX_HINT),
() -> "expected Lucene syntax hint, got: " +
e.getMessage());
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
/**
@@ -282,19 +299,24 @@ class SearchServiceIntegrationTest {
() ->
searchService.search("definitely_not_a_collection", "*:*", null, null, null,
null, null));
assertTrue(e.getMessage().contains(SearchService.LIST_COLLECTIONS_HINT),
() -> "expected list-collections hint, got: " +
e.getMessage());
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
/**
- * A Solr error we have no advice for must reach the client untouched.
Negative
- * {@code rows} is structurally identical to the undefined-field
failures — a
- * 400 carrying a generic {@code SolrException} — so it also pins that
the text
- * matching is not over-broad.
+ * An unclassified Solr error keeps its status but replaces backend
text with
+ * safe request guidance. Negative {@code rows} is structurally
identical to the
+ * undefined-field failures, so it also pins that text matching is not
+ * over-broad.
*/
@Test
void searchWithUnrecognizedSolrErrorPropagatesWithoutHint() {
SolrException e = assertThrows(SolrException.class,
() -> searchService.search(COLLECTION_NAME,
"*:*", null, null, null, null, -5));
assertFalse(e.getMessage().contains("Hint:"), () -> "expected
no hint, got: " + e.getMessage());
+ assertEquals(SolrException.ErrorCode.BAD_REQUEST.code,
e.code());
+ assertTrue(e.getMessage().contains("pagination"));
+ assertTrue(e.getMessage().contains("get-schema"));
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
/**
diff --git
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
index 28f10856..89cf1c91 100644
--- a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
@@ -23,6 +23,7 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.io.IOException;
+import java.util.Arrays;
import java.util.List;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrServerException;
@@ -47,6 +48,119 @@ class SearchServiceTest {
assertNotNull(localService);
}
+ @Test
+ void search_WithBlankFiltersAndFacets_ShouldOmitThem() throws Exception
{
+ SolrClient mockClient = mock(SolrClient.class);
+ QueryResponse mockResponse = mock(QueryResponse.class);
+
when(mockResponse.getResults()).thenReturn(createMockDocumentList());
+ when(mockClient.query(eq("test_collection"),
any(SolrQuery.class))).thenAnswer(invocation -> {
+ SolrQuery q = invocation.getArgument(1);
+ assertEquals("*:*", q.getQuery());
+ assertNull(q.getFilterQueries());
+ assertNull(q.getFacetFields());
+ assertNull(q.get("facet"));
+ return mockResponse;
+ });
+ SearchService localService = new SearchService(mockClient);
+ assertNotNull(localService.search("test_collection", " \t",
Arrays.asList("", " \t", null),
+ Arrays.asList(null, "", " \n"), null, null,
null));
+ }
+
+ @Test
+ void search_WithBlankSortFields_ShouldOmitSorting() throws Exception {
+ SolrClient mockClient = mock(SolrClient.class);
+ QueryResponse mockResponse = mock(QueryResponse.class);
+
when(mockResponse.getResults()).thenReturn(createMockDocumentList());
+ when(mockClient.query(eq("test_collection"),
any(SolrQuery.class))).thenAnswer(invocation -> {
+ SolrQuery q = invocation.getArgument(1);
+ assertNull(q.getSortField());
+ return mockResponse;
+ });
+ SearchService localService = new SearchService(mockClient);
+ assertNotNull(localService.search("test_collection", null,
null, null,
+ Arrays.asList(new SortClause("", ""), new
SortClause(" \t", "asc"), null), null, null));
+ }
+
+ @Test
+ void search_WithMixedBlankAndValidOptions_ShouldPreserveValidValues()
throws Exception {
+ SolrClient mockClient = mock(SolrClient.class);
+ QueryResponse mockResponse = mock(QueryResponse.class);
+
when(mockResponse.getResults()).thenReturn(createMockDocumentList());
+ String filter = "{!tag=genre}genre_s:fantasy";
+ String facet = "{!ex=genre}genre_s";
+ when(mockClient.query(eq("test_collection"),
any(SolrQuery.class))).thenAnswer(invocation -> {
+ SolrQuery q = invocation.getArgument(1);
+ assertEquals("{!edismax qf='name author_ss'}george
martin", q.getQuery());
+ assertArrayEquals(new String[]{filter, "price:[0 TO
10]"}, q.getFilterQueries());
+ assertArrayEquals(new String[]{facet, "author_ss"},
q.getFacetFields());
+ assertEquals("true", q.get("facet"));
+ assertEquals(1, q.getFacetMinCount());
+ assertEquals("count", q.getFacetSortString());
+ assertEquals("price desc,name asc", q.getSortField());
+ return mockResponse;
+ });
+ SearchService localService = new SearchService(mockClient);
+ assertNotNull(localService.search("test_collection", "{!edismax
qf='name author_ss'}george martin",
+ Arrays.asList("", filter, null, " \n",
"price:[0 TO 10]"),
+ Arrays.asList("\t", facet, "", null,
"author_ss"), Arrays.asList(null, new SortClause("", "asc"),
+ new SortClause("price",
"DESC"), new SortClause(" \n", ""), new SortClause("name", " \t")),
+ null, null));
+ }
+
+ @Test
+ void search_WithInvalidNonblankSortOrder_ShouldStillFail() {
+ SearchService localService = new
SearchService(mock(SolrClient.class));
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
+ () -> localService.search("test_collection",
null, null, null,
+ List.of(new SortClause("price",
"descending")), null, null));
+ assertTrue(e.getMessage().contains("'asc' or 'desc'"));
+ }
+
+ @Test
+ void search_BackendFailures_ShouldReturnSafeActionableErrors() throws
Exception {
+ String detail = "private-token http://internal-solr:8983
/private/config java.util.ArrayList";
+ for (Exception failure : List.of(new IOException(detail), new
SolrServerException(detail),
+ new
SolrException(SolrException.ErrorCode.SERVER_ERROR, detail),
+ new
SolrException(SolrException.ErrorCode.BAD_REQUEST, detail),
+ new
SolrException(SolrException.ErrorCode.UNAUTHORIZED, detail),
+ new
SolrException(SolrException.ErrorCode.FORBIDDEN, detail), new
ClassCastException(detail),
+ new IllegalStateException(detail))) {
+ SolrClient mockClient = mock(SolrClient.class);
+ when(mockClient.query(eq("test_collection"),
any(SolrQuery.class))).thenThrow(failure);
+ SearchService localService = new
SearchService(mockClient);
+ Exception e = assertThrows(Exception.class,
+ () ->
localService.search("test_collection", "*:*", null, List.of("platform"), null,
null, null));
+ for (String sensitive : List.of("private-token",
"internal-solr", "/private/config",
+ "java.util.ArrayList")) {
+ assertFalse(e.getMessage().contains(sensitive),
e::getMessage);
+ }
+ assertTrue(e.getMessage().contains("Search failed"),
e::getMessage);
+ assertTrue(e.getMessage().contains("operator") ||
e.getMessage().contains("get-schema"), e::getMessage);
+ assertNull(e.getCause(), "MCP must not unwrap a raw
backend failure");
+ if (failure instanceof SolrException solrFailure) {
+ assertEquals(solrFailure.code(),
assertInstanceOf(SolrException.class, e).code());
+ }
+ }
+ }
+
+ @Test
+ void search_FacetConversionFailure_ShouldNotReturnSuccessfulSearch()
throws Exception {
+ SolrClient mockClient = mock(SolrClient.class);
+ QueryResponse mockResponse = mock(QueryResponse.class);
+
when(mockResponse.getResults()).thenReturn(createMockDocumentListWithData());
+ ClassCastException failure = new
ClassCastException("java.util.ArrayList cannot be cast to NamedList");
+ when(mockResponse.getFacetFields()).thenThrow(failure);
+ when(mockClient.query(eq("test_collection"),
any(SolrQuery.class))).thenReturn(mockResponse);
+ SearchService localService = new SearchService(mockClient);
+ IllegalStateException e =
assertThrows(IllegalStateException.class,
+ () -> localService.search("test_collection",
"*:*", null, List.of("platform"), null, null, null));
+ assertFalse(e.getMessage().contains("ClassCastException"));
+ assertFalse(e.getMessage().contains("NamedList"));
+ assertTrue(e.getMessage().contains("facetFields"));
+ assertTrue(e.getMessage().contains("operator"));
+ assertNull(e.getCause(), "MCP must not unwrap a raw
response-conversion failure");
+ }
+
/*
* These stub Solr's error text rather than observe it, so they can
only show
* that a matching message produces a hint — never that Solr still
emits such a
@@ -66,8 +180,9 @@ class SearchServiceTest {
new
SolrException(SolrException.ErrorCode.BAD_REQUEST, "undefined field bogus"));
IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
() -> localService.search("test_collection",
"bogus:x", null, null, null, null, null));
- assertTrue(e.getMessage().contains("undefined field bogus"),
"original Solr message must be preserved");
+ assertFalse(e.getMessage().contains("undefined field bogus"),
"raw Solr message must not be exposed");
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted("test_collection")));
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
@Test
@@ -78,6 +193,7 @@ class SearchServiceTest {
IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
() -> localService.search("test_collection",
"*:*", null, null, sort, null, null));
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted("test_collection")));
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
@Test
@@ -87,6 +203,8 @@ class SearchServiceTest {
IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
() -> localService.search("test_collection",
"name:(", null, null, null, null, null));
assertTrue(e.getMessage().contains(SearchService.LUCENE_SYNTAX_HINT));
+
assertFalse(e.getMessage().contains("org.apache.solr.search.SyntaxError"));
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
/**
@@ -101,6 +219,8 @@ class SearchServiceTest {
IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
() -> localService.search("test_collection",
"*:*", null, null, null, null, null));
assertTrue(e.getMessage().contains(SearchService.LIST_COLLECTIONS_HINT));
+ assertFalse(e.getMessage().contains("mime type"));
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
@Test
@@ -109,8 +229,11 @@ class SearchServiceTest {
new
SolrException(SolrException.ErrorCode.SERVER_ERROR, "internal failure"));
SolrException e = assertThrows(SolrException.class,
() -> localService.search("test_collection",
null, null, null, null, null, null));
- assertTrue(e.getMessage().contains("internal failure"));
- assertFalse(e.getMessage().contains("Hint:"));
+ assertEquals(SolrException.ErrorCode.SERVER_ERROR.code,
e.code());
+ assertFalse(e.getMessage().contains("internal failure"));
+ assertTrue(e.getMessage().contains("operator"));
+ assertTrue(e.getMessage().contains("Solr health"));
+ assertNull(e.getCause(), "MCP must not unwrap a raw Solr
failure");
}
@Test
