fix(search): tolerate blank optional arguments and return safe errors

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 @@ private static Map<String, Map<String, Long>> getFacets(QueryResponse queryRespo
 	 *            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 SearchResponse search(@McpToolParam(description = "Solr collection to que
 							+ " {!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 SearchResponse search(@McpToolParam(description = "Solr collection to que
 
 		// 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,63 +309,86 @@ public SearchResponse search(@McpToolParam(description = "Solr collection to que
 			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,
 		// so SolrJ reports it as a mime-type mismatch and leaves getMetadata() null.
 		// 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.");
 	}
 
 	/**

src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java

@@ -828,6 +828,32 @@ void completePromptArg_ViewSchema_ReturnsCreatedCollection() {
 				"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");

src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java

@@ -70,6 +70,7 @@ void setUp() throws Exception {
 					[
 					  {
 					    "id": "book001",
+					    "platform_ss": ["netflix"],
 					    "name": ["A Game of Thrones"],
 					    "author_ss": ["George R.R. Martin"],
 					    "price": [7.99],
@@ -80,6 +81,7 @@ void setUp() throws Exception {
 					  },
 					  {
 					    "id": "book002",
+					    "platform_ss": ["netflix"],
 					    "name": ["A Clash of Kings"],
 					    "author_ss": ["George R.R. Martin"],
 					    "price": [8.99],
@@ -90,6 +92,7 @@ void setUp() throws Exception {
 					  },
 					  {
 					    "id": "book003",
+					    "platform_ss": ["hulu"],
 					    "name": ["A Storm of Swords"],
 					    "author_ss": ["George R.R. Martin"],
 					    "price": [9.99],
@@ -211,6 +214,15 @@ void facetingAQueryThatMatchesNothingReturnsEmptyFacets() throws SolrServerExcep
 				() -> "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 @@ void searchWithUndefinedFieldInQueryReturnsGetSchemaHint() {
 				.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 @@ void searchWithUndefinedFieldInFilterQueryReturnsGetSchemaHint() {
 				.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 @@ void searchWithUndefinedFacetFieldReturnsGetSchemaHint() {
 				.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 @@ void searchWithUndefinedSortFieldReturnsGetSchemaHint() {
 				() -> 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 @@ void searchWithUnparseableQueryReturnsLuceneSyntaxHint() {
 				() -> 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 @@ void searchOnMissingCollectionReturnsListCollectionsHint() {
 				() -> 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");
 	}
 
 	/**