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 c7b2529 fix: propagate errors from listCollections() instead of
returning empty list (#112)
c7b2529 is described below
commit c7b252908497ee39d06ace1de5905994cec2414d
Author: Aditya Parikh <[email protected]>
AuthorDate: Fri May 8 17:10:38 2026 -0400
fix: propagate errors from listCollections() instead of returning empty
list (#112)
When Solr is unreachable, listCollections() was catching
SolrServerException/IOException and returning an empty list. This caused
the MCP client to incorrectly tell users "there are no collections"
instead of reporting a connection error.
Remove the try-catch and let checked exceptions propagate through
listCollections(), validateCollectionExists(), and their callers so the
MCP framework can surface a proper error to the client.
Closes #3
Signed-off-by: adityamparikh <[email protected]>
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
---
.../mcp/server/collection/CollectionService.java | 86 +++++++++-------------
.../CollectionServiceIntegrationTest.java | 10 +--
.../server/collection/CollectionServiceTest.java | 52 +++++++------
.../ConferenceEndToEndIntegrationTest.java | 2 +-
4 files changed, 67 insertions(+), 83 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 f20ed2b..7cbb7a2 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
@@ -102,10 +102,11 @@ import org.springframework.stereotype.Service;
* <strong>Error Handling:</strong>
*
* <p>
- * The service implements robust error handling with graceful degradation.
- * Failed operations return null values rather than throwing exceptions (except
- * where validation requires it), allowing partial metrics collection when some
- * endpoints are unavailable.
+ * Collection-level operations (listing, validation) propagate
+ * {@link SolrServerException} and {@link IOException} so that callers
+ * (including the MCP framework) receive a clear error when Solr is
unreachable.
+ * Sub-metric operations (cache, handler) degrade gracefully and return
+ * {@code null} when their specific endpoints are unavailable.
*
* <p>
* <strong>Example Usage:</strong>
@@ -282,7 +283,7 @@ public class CollectionService {
* @return JSON string containing the list of collections
*/
@McpResource(uri = "solr://collections", name = "solr-collections",
description = "List of all Solr collections available in the cluster", mimeType
= "application/json")
- public String getCollectionsResource() {
+ public String getCollectionsResource() throws SolrServerException,
IOException {
return toJson(objectMapper, listCollections());
}
@@ -297,7 +298,7 @@ public class CollectionService {
* @return list of available collection names for autocompletion
*/
@McpComplete(uri = "solr://{collection}/schema")
- public List<String> completeCollectionForSchema() {
+ public List<String> completeCollectionForSchema() throws
SolrServerException, IOException {
return listCollections();
}
@@ -314,14 +315,6 @@ public class CollectionService {
* the base collection name if needed.
*
* <p>
- * <strong>Error Handling:</strong>
- *
- * <p>
- * If the operation fails due to connectivity issues or API errors, an
empty
- * list is returned rather than throwing an exception, allowing the
application
- * to continue functioning with degraded capabilities.
- *
- * <p>
* <strong>MCP Tool Usage:</strong>
*
* <p>
@@ -329,23 +322,23 @@ public class CollectionService {
* natural language requests like "list all collections" or "show me
available
* databases".
*
- * @return a list of collection names, or an empty list if unable to
retrieve
- * them
+ * @return a list of collection names; never null (returns an empty
list when
+ * Solr reports no collections)
+ * @throws SolrServerException
+ * if there are errors communicating with Solr
+ * @throws IOException
+ * if there are I/O errors during communication
* @see CollectionAdminRequest.List
*/
@PreAuthorize("isAuthenticated()")
@McpTool(name = "list-collections", description = "List solr
collections")
- public List<String> listCollections() {
- try {
- CollectionAdminRequest.List request = new
CollectionAdminRequest.List();
- CollectionAdminResponse response =
request.process(solrClient);
-
- @SuppressWarnings("unchecked")
- List<String> collections = (List<String>)
response.getResponse().get(COLLECTIONS_KEY);
- return collections != null ? collections : new
ArrayList<>();
- } catch (SolrServerException | IOException _) {
- return new ArrayList<>();
- }
+ public List<String> listCollections() throws SolrServerException,
IOException {
+ CollectionAdminRequest.List request = new
CollectionAdminRequest.List();
+ CollectionAdminResponse response = request.process(solrClient);
+
+ @SuppressWarnings("unchecked")
+ List<String> collections = (List<String>)
response.getResponse().get(COLLECTIONS_KEY);
+ return collections != null ? collections : new ArrayList<>();
}
/**
@@ -558,7 +551,7 @@ public class CollectionService {
* @see #extractCacheStats(NamedList)
* @see #isCacheStatsEmpty(CacheStats)
*/
- public CacheStats getCacheMetrics(String collection) {
+ public CacheStats getCacheMetrics(String collection) throws
SolrServerException, IOException {
String actualCollection = extractCollectionName(collection);
if (!validateCollectionExists(actualCollection)) {
@@ -670,7 +663,7 @@ public class CollectionService {
* @see #fetchFlatHandlerInfo(String, String, String)
* @see #isHandlerStatsEmpty(HandlerStats)
*/
- public HandlerStats getHandlerMetrics(String collection) {
+ public HandlerStats getHandlerMetrics(String collection) throws
SolrServerException, IOException {
String actualCollection = extractCollectionName(collection);
if (!validateCollectionExists(actualCollection)) {
@@ -876,36 +869,29 @@ public class CollectionService {
* This dual approach ensures compatibility with SolrCloud environments
where
* shard names may be returned alongside collection names.
*
- * <p>
- * <strong>Error Handling:</strong>
- *
- * <p>
- * Returns {@code false} if validation fails due to communication
errors,
- * allowing calling methods to handle missing collections appropriately.
- *
* @param collection
* the collection name to validate
* @return true if the collection exists (either exact or shard match),
false
* otherwise
+ * @throws SolrServerException
+ * if there are errors communicating with Solr
+ * @throws IOException
+ * if there are I/O errors during communication
* @see #listCollections()
* @see #extractCollectionName(String)
*/
- private boolean validateCollectionExists(String collection) {
- try {
- List<String> collections = listCollections();
-
- // Check for exact match first
- if (collections.contains(collection)) {
- return true;
- }
+ private boolean validateCollectionExists(String collection) throws
SolrServerException, IOException {
+ List<String> collections = listCollections();
- // Check if any of the returned collections start with
the collection name (for
- // shard
- // names)
- return collections.stream().anyMatch(c ->
c.startsWith(collection + SHARD_SUFFIX));
- } catch (Exception e) {
- return false;
+ // Check for exact match first
+ if (collections.contains(collection)) {
+ return true;
}
+
+ // Check if any of the returned collections start with the
collection name (for
+ // shard
+ // names)
+ return collections.stream().anyMatch(c ->
c.startsWith(collection + SHARD_SUFFIX));
}
/**
diff --git
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
index 788b952..e915012 100644
---
a/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/collection/CollectionServiceIntegrationTest.java
@@ -99,7 +99,7 @@ class CollectionServiceIntegrationTest {
}
@Test
- void testListCollections() {
+ void testListCollections() throws Exception {
List<String> collections = collectionService.listCollections();
log.debug("Collections: {}", collections);
@@ -200,7 +200,7 @@ class CollectionServiceIntegrationTest {
}
@Test
- void testGetCacheMetrics_afterQueries() {
+ void testGetCacheMetrics_afterQueries() throws Exception {
CacheStats cacheStats =
collectionService.getCacheMetrics(TEST_COLLECTION);
assertNotNull(cacheStats, "Cache stats should not be null after
warm-up queries");
@@ -224,7 +224,7 @@ class CollectionServiceIntegrationTest {
}
@Test
- void testGetHandlerMetrics_afterQueriesAndIndexing() {
+ void testGetHandlerMetrics_afterQueriesAndIndexing() throws Exception {
HandlerStats handlerStats =
collectionService.getHandlerMetrics(TEST_COLLECTION);
assertNotNull(handlerStats, "Handler stats should not be null
after activity");
@@ -243,12 +243,12 @@ class CollectionServiceIntegrationTest {
}
@Test
- void testGetCacheMetrics_nonExistentCollection() {
+ void testGetCacheMetrics_nonExistentCollection() throws Exception {
assertNull(collectionService.getCacheMetrics("non_existent_collection"));
}
@Test
- void testGetHandlerMetrics_nonExistentCollection() {
+ void testGetHandlerMetrics_nonExistentCollection() throws Exception {
assertNull(collectionService.getHandlerMetrics("non_existent_collection"));
}
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 51d31a3..482c290 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
@@ -74,16 +74,14 @@ class CollectionServiceTest {
}
@Test
- void listCollections_WhenExceptionOccurs_ShouldReturnEmptyList() throws
Exception {
+ void listCollections_WhenExceptionOccurs_ShouldPropagate() throws
Exception {
// Given - mock throws exception
when(solrClient.request(any(), any())).thenThrow(new
SolrServerException("Connection error"));
- // When
- List<String> result = collectionService.listCollections();
-
- // Then
- assertNotNull(result);
- assertTrue(result.isEmpty());
+ // When / Then - exception propagates to caller
+ SolrServerException exception =
assertThrows(SolrServerException.class,
+ () -> collectionService.listCollections());
+ assertTrue(exception.getMessage().contains("Connection error"));
}
// Collection name extraction tests
@@ -365,7 +363,7 @@ class CollectionServiceTest {
// Collection validation tests
@Test
- void getCollectionStats_NotFound() {
+ void getCollectionStats_NotFound() throws Exception {
CollectionService spyService = spy(collectionService);
doReturn(Collections.emptyList()).when(spyService).listCollections();
@@ -390,7 +388,7 @@ class CollectionServiceTest {
}
@Test
- void validateCollectionExists_WithException() throws Exception {
+ void validateCollectionExists_WithEmptyList() throws Exception {
CollectionService spyService = spy(collectionService);
doReturn(Collections.emptyList()).when(spyService).listCollections();
@@ -402,9 +400,12 @@ class CollectionServiceTest {
// Cache metrics tests
@Test
- void getCacheMetrics_WithNonExistentCollection_ShouldReturnNull() {
- // When - Mock will not have collection configured
- CacheStats result =
collectionService.getCacheMetrics("nonexistent");
+ void getCacheMetrics_WithNonExistentCollection_ShouldReturnNull()
throws Exception {
+ CollectionService spyService = spy(collectionService);
+
doReturn(Collections.emptyList()).when(spyService).listCollections();
+
+ // When - collection not found in empty list
+ CacheStats result = spyService.getCacheMetrics("nonexistent");
// Then
assertNull(result);
@@ -426,7 +427,7 @@ class CollectionServiceTest {
}
@Test
- void getCacheMetrics_CollectionNotFound() {
+ void getCacheMetrics_CollectionNotFound() throws Exception {
CollectionService spyService = spy(collectionService);
doReturn(Collections.emptyList()).when(spyService).listCollections();
@@ -543,9 +544,12 @@ class CollectionServiceTest {
// Handler metrics tests
@Test
- void getHandlerMetrics_WithNonExistentCollection_ShouldReturnNull() {
- // When - Mock will not have collection configured
- HandlerStats result =
collectionService.getHandlerMetrics("nonexistent");
+ void getHandlerMetrics_WithNonExistentCollection_ShouldReturnNull()
throws Exception {
+ CollectionService spyService = spy(collectionService);
+
doReturn(Collections.emptyList()).when(spyService).listCollections();
+
+ // When - collection not found in empty list
+ HandlerStats result =
spyService.getHandlerMetrics("nonexistent");
// Then
assertNull(result);
@@ -568,7 +572,7 @@ class CollectionServiceTest {
}
@Test
- void getHandlerMetrics_CollectionNotFound() {
+ void getHandlerMetrics_CollectionNotFound() throws Exception {
CollectionService spyService = spy(collectionService);
doReturn(Collections.emptyList()).when(spyService).listCollections();
@@ -714,23 +718,17 @@ class CollectionServiceTest {
}
@Test
- void listCollections_Error() throws Exception {
+ void listCollections_SolrServerException_Propagates() throws Exception {
when(solrClient.request(any(), any())).thenThrow(new
SolrServerException("Connection error"));
- List<String> result = collectionService.listCollections();
-
- assertNotNull(result);
- assertTrue(result.isEmpty());
+ assertThrows(SolrServerException.class, () ->
collectionService.listCollections());
}
@Test
- void listCollections_IOError() throws Exception {
+ void listCollections_IOException_Propagates() throws Exception {
when(solrClient.request(any(), any())).thenThrow(new
IOException("IO error"));
- List<String> result = collectionService.listCollections();
-
- assertNotNull(result);
- assertTrue(result.isEmpty());
+ assertThrows(IOException.class, () ->
collectionService.listCollections());
}
// Helper methods — mock the Solr Metrics API response format:
diff --git
a/src/test/java/org/apache/solr/mcp/server/collection/ConferenceEndToEndIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/collection/ConferenceEndToEndIntegrationTest.java
index baa0305..ca9cb7d 100644
---
a/src/test/java/org/apache/solr/mcp/server/collection/ConferenceEndToEndIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/collection/ConferenceEndToEndIntegrationTest.java
@@ -74,7 +74,7 @@ class ConferenceEndToEndIntegrationTest {
@Test
@Order(1)
- void collectionAppearsInList() {
+ void collectionAppearsInList() throws Exception {
List<String> collections = collectionService.listCollections();
boolean found = collections.contains(COLLECTION)
|| collections.stream().anyMatch(c ->
c.startsWith(COLLECTION + "_shard"));
